cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Oct 16 05:00:10 CEST 2018 media-tree git hash:490d84f6d73c12f4204241cff8651eed60aae914 media_build git hash: 9f419c414672676f63e85a61ea99df0ddcd6e9a7 v4l-utils git hash: 9d7d01f24b5e8ac73fbed783cffd5c0f5f6e8a87 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.11-marune linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.123-i686: OK linux-3.18.123-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.159-i686: OK linux-4.4.159-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.131-i686: OK linux-4.9.131-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19-rc6-i686: OK linux-4.19-rc6-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH 1/2] media: docs-rst: Document memory-to-memory video decoder interface
Le lundi 15 octobre 2018 à 19:13 +0900, Tomasz Figa a écrit : > On Wed, Aug 8, 2018 at 11:55 AM Tomasz Figa wrote: > > > > On Tue, Aug 7, 2018 at 4:37 PM Hans Verkuil wrote: > > > > > > On 08/07/2018 09:05 AM, Tomasz Figa wrote: > > > > On Thu, Jul 26, 2018 at 7:57 PM Hans Verkuil wrote: > > > > > > > I wonder if we should make these min buffer controls required. It > > > > > > > might be easier > > > > > > > that way. > > > > > > > > > > > > Agreed. Although userspace is still free to ignore it, because > > > > > > REQBUFS > > > > > > would do the right thing anyway. > > > > > > > > > > It's never been entirely clear to me what the purpose of those min > > > > > buffers controls > > > > > is. REQBUFS ensures that the number of buffers is at least the > > > > > minimum needed to > > > > > make the HW work. So why would you need these controls? It only makes > > > > > sense if they > > > > > return something different from REQBUFS. > > > > > > > > > > > > > The purpose of those controls is to let the client allocate a number > > > > of buffers bigger than minimum, without the need to allocate the > > > > minimum number of buffers first (to just learn the number), free them > > > > and then allocate a bigger number again. > > > > > > I don't feel this is particularly useful. One problem with the minimum > > > number > > > of buffers as used in the kernel is that it is often the minimum number of > > > buffers required to make the hardware work, but it may not be optimal. > > > E.g. > > > quite a few capture drivers set the minimum to 2, which is enough for the > > > hardware, but it will likely lead to dropped frames. You really need 3 > > > (one is being DMAed, one is queued and linked into the DMA engine and one > > > is > > > being processed by userspace). > > > > > > I would actually prefer this to be the recommended minimum number of > > > buffers, > > > which is >= the minimum REQBUFS uses. > > > > > > I.e., if you use this number and you have no special requirements, then > > > you'll > > > get good performance. > > > > I guess we could make it so. It would make existing user space request > > more buffers than it used to with the original meaning, but I guess it > > shouldn't be a big problem. > > I gave it a bit more thought and I feel like kernel is not the right > place to put any assumptions on what the userspace expects "good > performance" to be. Actually, having these controls return the minimum > number of buffers as REQBUFS would allocate makes it very well > specified - with this number you can only process frame by frame and > the number of buffers added by userspace defines exactly the queue > depth. It leaves no space for driver-specific quirks, because the > driver doesn't decide what's "good performance" anymore. I agree on that and I would add that the driver making any assumption would lead to memory waste in context where less buffer will still work (think of fence based operation as an example). > > Best regards, > Tomasz
Working test 5
Did you get my email from last week? Let me know if you have photos for cutting out or retouching? We are an image team who can do editing for your the web store photos, industry photos or portrait photos. Send photos, we will do testing for you to check quality. Waiting for your reply soon. Thanks, Judy
Working test 3
Did you get my email from last week? Let me know if you have photos for cutting out or retouching? We are an image team who can do editing for your the web store photos, industry photos or portrait photos. Send photos, we will do testing for you to check quality. Waiting for your reply soon. Thanks, Judy
Working test 3
Did you get my email from last week? Let me know if you have photos for cutting out or retouching? We are an image team who can do editing for your the web store photos, industry photos or portrait photos. Send photos, we will do testing for you to check quality. Waiting for your reply soon. Thanks, Judy
Re: [PATCH v3 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
Hello, On Monday, 15 October 2018 22:01:21 EEST Niklas Söderlund wrote: > On 2018-10-15 18:37:40 +0100, Kieran Bingham wrote: > >>> diff --git > >>> a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >>> b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >>> new file mode 100644 > >>> index ..a73e3c0dc31b > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >>> @@ -0,0 +1,182 @@ > >>> +Maxim Integrated Quad GMSL Deserializer > >>> +--- > >>> + > >>> +The MAX9286 deserializer receives video data on up to 4 Gigabit > >>> Multimedia > >>> +Serial Links (GMSL) and outputs them on a CSI-2 port using up to 4 > >>> data lanes. > >> > >> CSI-2 D-PHY I presume? > > > > Yes, that's how I've adapted the driver based on the latest bus changes. > > > > Niklas - Could you confirm that everything in VIN/CSI2 is configured to > > use D-PHY and not C-PHY at all ? > > Yes it's only D-PHY. > > >>> + > >>> +- remote-endpoint: phandle to the remote GMSL source endpoint subnode > >>> in the > >>> + remote node port. > >>> + > >>> +Required Endpoint Properties for CSI-2 Output Port (Port 4): > >>> + > >>> +- data-lanes: array of physical CSI-2 data lane indexes. > >>> +- clock-lanes: index of CSI-2 clock lane. > >> > >> Is any number of lanes supported? How about lane remapping? If you do > >> not have lane remapping, the clock-lanes property is redundant. > > > > Uhm ... Niklas? > > The MAX9286 documentation contains information on lane remapping and > support for any number (1-4) of enabled data-lanes. I have not tested if > this works in practice but the registers are there and documented :-) That's my understanding too. Clock lane remapping doesn't seem to be supported though. We could thus omit the clock-lanes property. -- Regards, Laurent Pinchart
Working test 4
Did you get my email from last week? Let me know if you have photos for cutting out or retouching? We are an image team who can do editing for your the web store photos, industry photos or portrait photos. Send photos, we will do testing for you to check quality. Waiting for your reply soon. Thanks, Judy
Re: [PATCH v3 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
Hi Kieran, On Mon, Oct 15, 2018 at 06:37:40PM +0100, Kieran Bingham wrote: > Hi Sakari, > > Thank you for the review, > > On 15/10/18 17:45, Sakari Ailus wrote: > > Hi Kieran, > > > > Could you cc the devicetree list for the next version, please? > > Argh - looks like I've missed the DT list on all three postings. > > No wonder the responses have been quiet :-) > > I'll do a v4 post after I've gone through some of your comments, and > make sure I remember the DT guys this time :) > > > > On Tue, Oct 09, 2018 at 09:57:23PM +0100, Kieran Bingham wrote: > >> From: Laurent Pinchart > >> > >> The MAX9286 deserializes video data received on up to 4 Gigabit > >> Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up > >> to 4 data lanes. > >> > >> Signed-off-by: Laurent Pinchart > >> Signed-off-by: Jacopo Mondi > >> Signed-off-by: Kieran Bingham > >> > >> --- > >> v3: > >> - Update binding descriptions > >> --- > >> .../bindings/media/i2c/maxim,max9286.txt | 182 ++ > >> 1 file changed, 182 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> > >> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> new file mode 100644 > >> index ..a73e3c0dc31b > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> @@ -0,0 +1,182 @@ > >> +Maxim Integrated Quad GMSL Deserializer > >> +--- > >> + > >> +The MAX9286 deserializer receives video data on up to 4 Gigabit Multimedia > >> +Serial Links (GMSL) and outputs them on a CSI-2 port using up to 4 data > >> lanes. > > > > CSI-2 D-PHY I presume? > > Yes, that's how I've adapted the driver based on the latest bus changes. > > Niklas - Could you confirm that everything in VIN/CSI2 is configured to > use D-PHY and not C-PHY at all ? > > > >> + > >> +In addition to video data, the GMSL links carry a bidirectional control > >> +channel that encapsulates I2C messages. The MAX9286 forwards all I2C > >> traffic > >> +not addressed to itself to the other side of the links, where a GMSL > >> +serializer will output it on a local I2C bus. In the other direction all > >> I2C > >> +traffic received over GMSL by the MAX9286 is output on the local I2C bus. > >> + > >> +Required Properties: > >> + > >> +- compatible: Shall be "maxim,max9286" > >> +- reg: I2C device address > >> + > >> +Optional Properties: > >> + > >> +- poc-supply: Regulator providing Power over Coax to the cameras > >> +- pwdn-gpios: GPIO connected to the #PWDN pin > > > > If this is basically a hardware reset pin, then you could call it e.g. > > enable-gpios or reset-gpios. There was recently a similar discussion > > related to ad5820 DT bindings. Techinically is a powerdown control, it shuts the current to the chip, not reset it. > > Ah yes ... now which polarity ;-) The signal is active low (when is at physical level 0, the chip is off). According to the gpio bindings documentation "The gpio-specifier's polarity flag should represent the physical level at the GPIO controller that achieves (or represents, for inputs) a logically asserted value at the device." Sakari's argument, which I understand and has been debated before, is to use generic names (ie. don't use the pin name as specified by the HW manual, but name it after its function. It doesn't matter if your pin is called "#RST", just call it "reset-gpios' and state the pin name in the documentation if you like to.) I count much more 'enable-gpios' compared to 'powerdown-gpios', so that seems the obvious choice, as generic names have not yet been documented anywhere as far as I know, but the most common ones should be used. Using generic names is good because in the power up/down routines, you don't have to care about the signals polarities, but only about their logical levels. At power-up if you have an "enable-gpio" just set it to logical 1, the gpio framework translates it to the appropriate physical level. Easier to write and to review. To comply with GPIO bindings we would have to enable-gpios: <&gpio 13 GPIO_ACTIVE_LOW>; And at power up we would have to use the logical value, and for an enable signal, setting it to "1" at power up would be the natural choice. However to have our line set to physical 1 and have the chip powered we would have to: gpiod_set_value(&enable, 0); Which makes any reason to use generic names vanish. All of this to say that, even if less popular, I would call it "powerdown-gpios", which is anyway quite generic, and describe it as: powerdown-gpios: Power down GPIO signal, pin name "PWDN". Active low. So that at power_up: gpiod_set_value(&pwdn, 0); And at power down: gpiod_set_value(&pwdn, 1); > > > >> + > >> +Required endpoint nodes: > >> +--- > >> + > >> +The connections to the MAX928
Re: [PATCH v3 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
Hi Kieran, On 2018-10-15 18:37:40 +0100, Kieran Bingham wrote: > >> diff --git > >> a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> new file mode 100644 > >> index ..a73e3c0dc31b > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt > >> @@ -0,0 +1,182 @@ > >> +Maxim Integrated Quad GMSL Deserializer > >> +--- > >> + > >> +The MAX9286 deserializer receives video data on up to 4 Gigabit Multimedia > >> +Serial Links (GMSL) and outputs them on a CSI-2 port using up to 4 data > >> lanes. > > > > CSI-2 D-PHY I presume? > > Yes, that's how I've adapted the driver based on the latest bus changes. > > Niklas - Could you confirm that everything in VIN/CSI2 is configured to > use D-PHY and not C-PHY at all ? Yes it's only D-PHY. > >> + > >> +- remote-endpoint: phandle to the remote GMSL source endpoint subnode in > >> the > >> + remote node port. > >> + > >> +Required Endpoint Properties for CSI-2 Output Port (Port 4): > >> + > >> +- data-lanes: array of physical CSI-2 data lane indexes. > >> +- clock-lanes: index of CSI-2 clock lane. > > > > Is any number of lanes supported? How about lane remapping? If you do not > > have lane remapping, the clock-lanes property is redundant. > > > Uhm ... Niklas? The MAX9286 documentation contains information on lane remapping and support for any number (1-4) of enabled data-lanes. I have not tested if this works in practice but the registers are there and documented :-) -- Regards, Niklas Söderlund
Re: [ANN] Agenda for the media summit on Thursday Oct 25th in Edinburgh
Em Mon, 15 Oct 2018 13:36:08 +0200 Hans Verkuil escreveu: > Hi all, > > We are organizing a media mini-summit on Thursday October 25th in > Edinburgh, Edinburgh International Conference Centre. > > If you plan to attend, please register on the ELCE/OSS site since we're > using there tracking system: > > https://events.linuxfoundation.org/events/elc-openiot-europe-2018/register/ > > Name of the room for the summit: Tinto, Level 0 of the EICC > > We had 75 people sign up for the summit as of a week ago, which is quite > amazing. I'm not listing all of them here, just those that I know are active > media developers: ... > Mauro Carvalho Chehab On a side note, please always use my @kernel.org e-mail, e. g: Mauro Carvalho Chehab (with is the canonical e-mail it is set at Kernel tree) or Mauro Carvalho Chehab (with also has my employer's name on it [1] that sponsors my work upstream) I stopped using the s-opensource a while ago, and the server handling it was even decommissioned those days. Thanks, Mauro [1] Also, please notice that, except if I say otherwise, all opinions, comments, etc. I post with such e-mail don't represent my employer's opinion. It is just my way to thank Samsung for sponsoring my upstream's work.
Re: [PATCH v3 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
Hi Sakari,
Thank you for the review,
On 15/10/18 17:45, Sakari Ailus wrote:
> Hi Kieran,
>
> Could you cc the devicetree list for the next version, please?
Argh - looks like I've missed the DT list on all three postings.
No wonder the responses have been quiet :-)
I'll do a v4 post after I've gone through some of your comments, and
make sure I remember the DT guys this time :)
> On Tue, Oct 09, 2018 at 09:57:23PM +0100, Kieran Bingham wrote:
>> From: Laurent Pinchart
>>
>> The MAX9286 deserializes video data received on up to 4 Gigabit
>> Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
>> to 4 data lanes.
>>
>> Signed-off-by: Laurent Pinchart
>> Signed-off-by: Jacopo Mondi
>> Signed-off-by: Kieran Bingham
>>
>> ---
>> v3:
>> - Update binding descriptions
>> ---
>> .../bindings/media/i2c/maxim,max9286.txt | 182 ++
>> 1 file changed, 182 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
>> b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
>> new file mode 100644
>> index ..a73e3c0dc31b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
>> @@ -0,0 +1,182 @@
>> +Maxim Integrated Quad GMSL Deserializer
>> +---
>> +
>> +The MAX9286 deserializer receives video data on up to 4 Gigabit Multimedia
>> +Serial Links (GMSL) and outputs them on a CSI-2 port using up to 4 data
>> lanes.
>
> CSI-2 D-PHY I presume?
Yes, that's how I've adapted the driver based on the latest bus changes.
Niklas - Could you confirm that everything in VIN/CSI2 is configured to
use D-PHY and not C-PHY at all ?
>> +
>> +In addition to video data, the GMSL links carry a bidirectional control
>> +channel that encapsulates I2C messages. The MAX9286 forwards all I2C traffic
>> +not addressed to itself to the other side of the links, where a GMSL
>> +serializer will output it on a local I2C bus. In the other direction all I2C
>> +traffic received over GMSL by the MAX9286 is output on the local I2C bus.
>> +
>> +Required Properties:
>> +
>> +- compatible: Shall be "maxim,max9286"
>> +- reg: I2C device address
>> +
>> +Optional Properties:
>> +
>> +- poc-supply: Regulator providing Power over Coax to the cameras
>> +- pwdn-gpios: GPIO connected to the #PWDN pin
>
> If this is basically a hardware reset pin, then you could call it e.g.
> enable-gpios or reset-gpios. There was recently a similar discussion
> related to ad5820 DT bindings.
Ah yes ... now which polarity ;-)
>> +
>> +Required endpoint nodes:
>> +---
>> +
>> +The connections to the MAX9286 GMSL and its endpoint nodes are modeled using
>> +the OF graph bindings in accordance with the video interface bindings
>> defined
>> +in Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +The following table lists the port number corresponding to each device port.
>> +
>> +PortDescription
>> +
>> +Port 0 GMSL Input 0
>> +Port 1 GMSL Input 1
>> +Port 2 GMSL Input 2
>> +Port 3 GMSL Input 3
>> +Port 4 CSI-2 Output
>> +
>> +Optional Endpoint Properties for GSML Input Ports (Port [0-3]):
>
> Isn't port 4 included?
Hrm ... yes well I guess these are mandatory for port 4. I'll look at
the wording here.
>
>> +
>> +- remote-endpoint: phandle to the remote GMSL source endpoint subnode in the
>> + remote node port.
>> +
>> +Required Endpoint Properties for CSI-2 Output Port (Port 4):
>> +
>> +- data-lanes: array of physical CSI-2 data lane indexes.
>> +- clock-lanes: index of CSI-2 clock lane.
>
> Is any number of lanes supported? How about lane remapping? If you do not
> have lane remapping, the clock-lanes property is redundant.
Uhm ... Niklas?
>
>> +
>> +Required i2c-mux nodes:
>> +--
>> +
>> +Each GMSL link is modeled as a child bus of an i2c bus multiplexer/switch,
>> in
>> +accordance with bindings described in
>> +Documentation/devicetree/bindings/i2c/i2c-mux.txt. The serializer device on
>> +the remote end of the GMSL link shall be modelled as a child node of the
>> +corresponding I2C bus.
>> +
>> +Required i2c child bus properties:
>> +- all properties described as required i2c child bus nodes properties in
>> + Documentation/devicetree/bindings/i2c/i2c-mux.txt.
>> +
>> +Example:
>> +---
>> +
>> +gmsl-deserializer@2c {
>> +compatible = "maxim,max9286";
>> +reg = <0x2c>;
>> +poc-supply = <&camera_poc_12v>;
>> +pwdn-gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
>> +
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +ports {
>> +#address-cells = <1>;
>> +#size-
Re: [PATCH v3 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286
Hi Kieran,
Could you cc the devicetree list for the next version, please?
On Tue, Oct 09, 2018 at 09:57:23PM +0100, Kieran Bingham wrote:
> From: Laurent Pinchart
>
> The MAX9286 deserializes video data received on up to 4 Gigabit
> Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> to 4 data lanes.
>
> Signed-off-by: Laurent Pinchart
> Signed-off-by: Jacopo Mondi
> Signed-off-by: Kieran Bingham
>
> ---
> v3:
> - Update binding descriptions
> ---
> .../bindings/media/i2c/maxim,max9286.txt | 182 ++
> 1 file changed, 182 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
> b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
> new file mode 100644
> index ..a73e3c0dc31b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.txt
> @@ -0,0 +1,182 @@
> +Maxim Integrated Quad GMSL Deserializer
> +---
> +
> +The MAX9286 deserializer receives video data on up to 4 Gigabit Multimedia
> +Serial Links (GMSL) and outputs them on a CSI-2 port using up to 4 data
> lanes.
CSI-2 D-PHY I presume?
> +
> +In addition to video data, the GMSL links carry a bidirectional control
> +channel that encapsulates I2C messages. The MAX9286 forwards all I2C traffic
> +not addressed to itself to the other side of the links, where a GMSL
> +serializer will output it on a local I2C bus. In the other direction all I2C
> +traffic received over GMSL by the MAX9286 is output on the local I2C bus.
> +
> +Required Properties:
> +
> +- compatible: Shall be "maxim,max9286"
> +- reg: I2C device address
> +
> +Optional Properties:
> +
> +- poc-supply: Regulator providing Power over Coax to the cameras
> +- pwdn-gpios: GPIO connected to the #PWDN pin
If this is basically a hardware reset pin, then you could call it e.g.
enable-gpios or reset-gpios. There was recently a similar discussion
related to ad5820 DT bindings.
> +
> +Required endpoint nodes:
> +---
> +
> +The connections to the MAX9286 GMSL and its endpoint nodes are modeled using
> +the OF graph bindings in accordance with the video interface bindings defined
> +in Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +The following table lists the port number corresponding to each device port.
> +
> +PortDescription
> +
> +Port 0 GMSL Input 0
> +Port 1 GMSL Input 1
> +Port 2 GMSL Input 2
> +Port 3 GMSL Input 3
> +Port 4 CSI-2 Output
> +
> +Optional Endpoint Properties for GSML Input Ports (Port [0-3]):
Isn't port 4 included?
> +
> +- remote-endpoint: phandle to the remote GMSL source endpoint subnode in the
> + remote node port.
> +
> +Required Endpoint Properties for CSI-2 Output Port (Port 4):
> +
> +- data-lanes: array of physical CSI-2 data lane indexes.
> +- clock-lanes: index of CSI-2 clock lane.
Is any number of lanes supported? How about lane remapping? If you do not
have lane remapping, the clock-lanes property is redundant.
> +
> +Required i2c-mux nodes:
> +--
> +
> +Each GMSL link is modeled as a child bus of an i2c bus multiplexer/switch, in
> +accordance with bindings described in
> +Documentation/devicetree/bindings/i2c/i2c-mux.txt. The serializer device on
> +the remote end of the GMSL link shall be modelled as a child node of the
> +corresponding I2C bus.
> +
> +Required i2c child bus properties:
> +- all properties described as required i2c child bus nodes properties in
> + Documentation/devicetree/bindings/i2c/i2c-mux.txt.
> +
> +Example:
> +---
> +
> + gmsl-deserializer@2c {
> + compatible = "maxim,max9286";
> + reg = <0x2c>;
> + poc-supply = <&camera_poc_12v>;
> + pwdn-gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + max9286_in0: endpoint {
> + remote-endpoint = <&rdacm20_out0>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + max9286_in1: endpoint {
> + remote-endpoint = <&rdacm20_out1>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + max9286_in2: endpoint {
> + remote-endpoint = <&rdacm20_ou
Re: [V2, 2/5] Documentation: dt-bindings: Document the Synopsys MIPI DPHY Rx bindings
Hi Rob,
On 12-Oct-18 17:45, Rob Herring wrote:
> On Thu, Sep 20, 2018 at 01:16:40PM +0200, Luis Oliveira wrote:
>> Add device-tree bindings documentation for SNPS DesignWare MIPI D-PHY in
>> RX mode.
>
> "dt-bindings: phy: ..." for the subject.
>
Yes, you are right.
>>
>> Signed-off-by: Luis Oliveira
>> ---
>> Changelog
>> v2:
>> - no changes
>>
>> .../devicetree/bindings/phy/snps,dphy-rx.txt | 36
>> ++
>> 1 file changed, 36 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/snps,dphy-rx.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/snps,dphy-rx.txt
>> b/Documentation/devicetree/bindings/phy/snps,dphy-rx.txt
>> new file mode 100644
>> index 000..9079f4a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/snps,dphy-rx.txt
>> @@ -0,0 +1,36 @@
>> +Synopsys DesignWare MIPI Rx D-PHY block details
>> +
>> +Description
>> +---
>> +
>> +The Synopsys MIPI D-PHY controller supports MIPI-DPHY in receiver mode.
>> +Please refer to phy-bindings.txt for more information.
>> +
>> +Required properties:
>> +- compatible: Shall be "snps,dphy-rx".
>> +- #phy-cells: Must be 1.
>> +- snps,dphy-frequency : Output frequency of the D-PHY.
>> +- snps,dphy-te-len : Size of the communication interface (8 bits->8 or
>> 12bits->12).
>> +- reg : Physical base address and size of the device
>> memory mapped
>> + registers;
>> +
>> +Optional properties:
>> +- snps,compat-mode : Compatibility mode control
>
> type? values?
>
I will remove this in V3.
>> +
>> +The per-board settings:
>> +- gpios : Synopsys testchip used as reference uses this to
>> change setup
>> + configurations.
>
> Preferred to be named (e.g. foo-gpios). How many? What are their
> functions?
>
Ok, thanks for reviewing this.
>> +
>> +Example:
>> +
>> +mipi_dphy_rx1: dphy@3040 {
>> +compatible = "snps,dphy-rx";
>> +#phy-cells = <1>;
>> +snps,dphy-frequency = <30>;
>> +snps,dphy-te-len = <12>;
>> +snps,compat-mode = <1>;
>> +reg = < 0x03040 0x20
>> +0x08000 0x100
>> +0x09000 0x100>;
>> +};
>> +
>> --
>> 2.9.3
>>
Re: [PATCH v5 5/6] media: Add controls for JPEG quantization tables
On Fri, 2018-10-12 at 22:00 +0200, Paul Kocialkowski wrote: > Hi, > > Le mercredi 19 septembre 2018 à 13:28 +0900, Tomasz Figa a écrit : > > On Thu, Sep 13, 2018 at 9:15 PM Paul Kocialkowski wrote: > > > Hi, > > > > > > On Wed, 2018-09-05 at 19:00 -0300, Ezequiel Garcia wrote: > > > > From: Shunqian Zheng > > > > > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > > > configure the JPEG quantization tables. > > > > > > > > Signed-off-by: Shunqian Zheng > > > > Signed-off-by: Ezequiel Garcia > > > > --- > > > > .../media/uapi/v4l/extended-controls.rst | 31 +++ > > > > .../media/videodev2.h.rst.exceptions | 1 + > > > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++ > > > > include/uapi/linux/v4l2-controls.h| 12 +++ > > > > include/uapi/linux/videodev2.h| 1 + > > > > 5 files changed, 55 insertions(+) > > > > > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst > > > > b/Documentation/media/uapi/v4l/extended-controls.rst > > > > index 9f7312bf3365..1335d27d30f3 100644 > > > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > > > @@ -3354,7 +3354,38 @@ JPEG Control IDs > > > > Specify which JPEG markers are included in compressed stream. This > > > > control is valid only for encoders. > > > > > > > > +.. _jpeg-quant-tables-control: > > > > > > I just had a look at how the Allwinner VPU handles JPEG decoding and it > > > seems to require the following information (in addition to > > > quantization): > > > > I assume the hardware doesn't have the ability to parse those from the > > stream and so they need to be parsed by user space and given to the > > driver? > > That's correct, we are also dealing with a stateless decoder here. It's > actually the same hardware engine that's used for MPEG2 decoding, only > configured differently. > > So we will need to introduce a pixfmt for compressed JPEG data without > headers, reuse JPEG controls that apply and perhaps introduce new ones > too if needed. > > I am also wondering about how MJPEG support should fit into this. As > far as I understood, it shouldn't be very different from JPEG so we > might want to have common controls for both. > We've recently discussed this and we were proposing to just drop MJPEG and stick to JPEG. The reason is that MJPEG is not clearly defined. Note that others treat MJPEG and JPEG as aliases. See "Re: [RFC] V4L2_PIX_FMT_MJPEG vs V4L2_PIX_FMT_JPEG". Also, I'll be adding support for spec-compliant JPEG frames in rockchip JPEG encoder. This will allow to use the driver with already available userspace. IOW, we don't absolutely need a new pixelformat for encoders. Decoders, on the other side, would be a different story, as parsing headers in the kernel can raise some safety concerns. Regards, Ezequiel
Re: [PATCH v3 6/6] media: video-i2c: support runtime PM
Hi Mita-san,
On Sun, Oct 14, 2018 at 03:02:39AM +0900, Akinobu Mita wrote:
> AMG88xx has a register for setting operating mode. This adds support
> runtime PM by changing the operating mode.
>
> The instruction for changing sleep mode to normal mode is from the
> reference specifications.
>
> https://docid81hrs3j1.cloudfront.net/medialibrary/2017/11/PANA-S-A0002141979-1.pdf
>
> Cc: Matt Ranostay
> Cc: Sakari Ailus
> Cc: Hans Verkuil
> Cc: Mauro Carvalho Chehab
> Reviewed-by: Matt Ranostay
> Acked-by: Sakari Ailus
> Signed-off-by: Akinobu Mita
> ---
> * v3
> - Move chip->set_power() call to the video device release() callback.
> - Add Acked-by line
>
> drivers/media/i2c/video-i2c.c | 141
> +-
> 1 file changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> index 3334fc2..22fdc43 100644
> --- a/drivers/media/i2c/video-i2c.c
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -94,10 +95,23 @@ struct video_i2c_chip {
> /* xfer function */
> int (*xfer)(struct video_i2c_data *data, char *buf);
>
> + /* power control function */
> + int (*set_power)(struct video_i2c_data *data, bool on);
> +
> /* hwmon init function */
> int (*hwmon_init)(struct video_i2c_data *data);
> };
>
> +/* Power control register */
> +#define AMG88XX_REG_PCTL 0x00
> +#define AMG88XX_PCTL_NORMAL 0x00
> +#define AMG88XX_PCTL_SLEEP 0x10
> +
> +/* Reset register */
> +#define AMG88XX_REG_RST 0x01
> +#define AMG88XX_RST_FLAG 0x30
> +#define AMG88XX_RST_INIT 0x3f
> +
> /* Frame rate register */
> #define AMG88XX_REG_FPSC 0x02
> #define AMG88XX_FPSC_1FPSBIT(0)
> @@ -127,6 +141,59 @@ static int amg88xx_setup(struct video_i2c_data *data)
> return regmap_update_bits(data->regmap, AMG88XX_REG_FPSC, mask, val);
> }
>
> +static int amg88xx_set_power_on(struct video_i2c_data *data)
> +{
> + int ret;
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL, AMG88XX_PCTL_NORMAL);
> + if (ret)
> + return ret;
> +
> + msleep(50);
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_INIT);
> + if (ret)
> + return ret;
> +
> + msleep(2);
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_FLAG);
> + if (ret)
> + return ret;
> +
> + /*
> + * Wait two frames before reading thermistor and temperature registers
> + */
> + msleep(200);
> +
> + return 0;
> +}
> +
> +static int amg88xx_set_power_off(struct video_i2c_data *data)
> +{
> + int ret;
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL, AMG88XX_PCTL_SLEEP);
> + if (ret)
> + return ret;
> + /*
> + * Wait for a while to avoid resuming normal mode immediately after
> + * entering sleep mode, otherwise the device occasionally goes wrong
> + * (thermistor and temperature registers are not updated at all)
> + */
> + msleep(100);
> +
> + return 0;
> +}
> +
> +static int amg88xx_set_power(struct video_i2c_data *data, bool on)
> +{
> + if (on)
> + return amg88xx_set_power_on(data);
> +
> + return amg88xx_set_power_off(data);
> +}
> +
> #if IS_ENABLED(CONFIG_HWMON)
>
> static const u32 amg88xx_temp_config[] = {
> @@ -158,7 +225,15 @@ static int amg88xx_read(struct device *dev, enum
> hwmon_sensor_types type,
> __le16 buf;
> int tmp;
>
> + tmp = pm_runtime_get_sync(regmap_get_device(data->regmap));
> + if (tmp < 0) {
> + pm_runtime_put_noidle(regmap_get_device(data->regmap));
> + return tmp;
> + }
> +
> tmp = regmap_bulk_read(data->regmap, AMG88XX_REG_TTHL, &buf, 2);
> + pm_runtime_mark_last_busy(regmap_get_device(data->regmap));
> + pm_runtime_put_autosuspend(regmap_get_device(data->regmap));
> if (tmp)
> return tmp;
>
> @@ -217,6 +292,7 @@ static const struct video_i2c_chip video_i2c_chip[] = {
> .regmap_config = &amg88xx_regmap_config,
> .setup = &amg88xx_setup,
> .xfer = &amg88xx_xfer,
> + .set_power = amg88xx_set_power,
> .hwmon_init = amg88xx_hwmon_init,
> },
> };
> @@ -343,14 +419,21 @@ static void video_i2c_del_list(struct vb2_queue *vq,
> enum vb2_buffer_state state
> static int start_streaming(struct vb2_queue *vq, unsigned int count)
> {
> struct video_i2c_data *data = vb2_get_drv_priv(vq);
> + struct device *dev = regmap_get_device(data->regmap);
> int ret;
>
> if (data->kthread_vid_cap)
> return 0;
>
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(dev);
> +
Re: [PATCH 1/4] media: ov5640: fix resolution update
Hi Hugues,
On Mon, Oct 15, 2018 at 03:13:12PM +, Hugues FRUCHET wrote:
> Hi Laurent, Jacopo, Sam,
>
> I'm also OK to change to a simpler alternative;
> - drop the "restore" step
Do you mean the restore step at the end of 'ov5640_restore_mode()' ?
I agree, I've been carrying this one [1] in my tree for some time now.
I just didn't send it because of the too many issues in flight on this
driver.
> - send the whole init register sequence + mode changes + format changes
> at streamon
>
This might be a first step in my opinion too, yes.
> is this what you have in mind Laurent ?
I know Laurent does not fully agree with me on this, but I would like
to have Maxime's series on clock tree handling merged and tested on
CSI-2 first before adding more patches to the pile of pending items on
ov5640. I hope to have time to test them on CSI-2 this week before
ELC-E.
Steve, you're the driver maintainer do you have preferences here?
Also, if this might be useful, I would like to help co-maintaining the
driver (with possibily other people, possibly with the sensor wired in
DVP mode), and help establishing priorities, as this driver needs some
love, but one item at the time to avoid getting lost in too many
pending changes as it happened recently :)
Thanks
j
[1]
media: ov5640: Do not restore format at power up
Do not force restoring the last applied capture format during sensor power
up
as it will be re-set at s_stream time.
Signed-off-by: Jacopo Mondi
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index b226946..17ee55b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1737,12 +1737,10 @@ static int ov5640_restore_mode(struct ov5640_dev
*sensor)
if (ret)
return ret;
- /* now restore the last capture mode */
- ret = ov5640_set_mode(sensor, &ov5640_mode_init_data);
- if (ret < 0)
- return ret;
+ sensor->pending_mode_change = true;
+ sensor->pending_fmt_change = true;
- return ov5640_set_framefmt(sensor, &sensor->fmt);
+ return 0;
}
static void ov5640_power(struct ov5640_dev *sensor, bool enable)
>
> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
> >> Hi Sam,
> >> thanks for the patch, I see the same issue you reported, but I
> >> think this patch can be improved.
> >>
> >> (expanding the Cc list to all people involved in recent ov5640
> >> developemts, not just for this patch, but for the whole series to look
> >> at. Copying names from another series cover letter, hope it is
> >> complete.)
> >>
> >> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> >>> set_fmt was not properly triggering a mode change when
> >>> a new mode was set that happened to have the same format
> >>> as the previous mode (for example, when only changing the
> >>> frame dimensions). Fix this.
> >>>
> >>> Signed-off-by: Sam Bobrowicz
> >>> ---
> >>>
> >>> drivers/media/i2c/ov5640.c | 8
> >>> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >>> index eaefdb5..5031aab 100644
> >>> --- a/drivers/media/i2c/ov5640.c
> >>> +++ b/drivers/media/i2c/ov5640.c
> >>> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> >>>
> >>> goto out;
> >>>
> >>> }
> >>>
> >>> - if (new_mode != sensor->current_mode) {
> >>> +
> >>> + if (new_mode != sensor->current_mode ||
> >>> + mbus_fmt->code != sensor->fmt.code) {
> >>> + sensor->fmt = *mbus_fmt;
> >>>
> >>> sensor->current_mode = new_mode;
> >>> sensor->pending_mode_change = true;
> >>>
> >>> - }
> >>> - if (mbus_fmt->code != sensor->fmt.code) {
> >>> - sensor->fmt = *mbus_fmt;
> >>>
> >>> sensor->pending_fmt_change = true;
> >>>
> >>> }
> >>
> >> How I did reproduce the issue:
> >>
> >> # Set 1024x768 on ov5640 without changing the image format
> >> # (default image size at startup is 640x480)
> >> $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768 field:none]"
> >>sensor->pending_mode_change = true; //verified this flag gets set
> >>
> >> # Start streaming, after having configured the whole pipeline to work
> >> # with 1024x768
> >> $ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
> >> Unable to start streaming: Broken pipe (32).
> >>
> >> # Inspect which part of pipeline validation went wrong
> >> # Turns out the sensor->fmt field is not updated, and when get_fmt()
> >> # is called, the old one is returned.
> >> $ media-ctl -e "ov5640 2-003c" -p
> >>...
> >>[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
> >> ycbcr:601
> >> quantization:full-range] ^^^ ^^^
> >>
> >> So yes, sensor->fmt is not udapted as it should be when only image
> >> resolution is changed.
> >>
> >> Al
Re: [PATCH] adv7604: add CEC support for adv7611/adv7612
On 10/15/2018 05:15 PM, Niklas Söderlund wrote: > Hi Hans, > > Thanks for your patch. > > On 2018-10-12 13:30:02 +0200, Hans Verkuil wrote: >> The CEC IP is very similar between the three HDMI receivers, but >> not identical. Add support for all three variants. >> >> Tested with an adv7604 and an adv7612. >> >> Signed-off-by: Hans Verkuil > > This fixes CEC on my Koelsch board using the adv7604. > > Reviewed-by: Niklas Söderlund > > Side note do you know of a way to simulate a cycling of the physical HDMI > cable? My current test-case for CEC is: > > v4l2-ctl -d $(grep -l "adv7612" /sys/class/video4linux/*/name | sed > 's#.*video4linux\(.*\)/name#/dev\1#g') --set-edid=type=hdmi > cec-ctl -d 0 --playback > cec-ctl -d 1 --tv > # Here I need to attach or if it already is connected disconnect and > # reconnect the HDMI cable > cec-ctl -d 0 -S > cec-ctl -d 1 -S > > If that step could be done in software I can add this test to my > automatic test scripts which would be nice. You can clear the EDID of the receiver: v4l2-ctl --clear-edid This will invalidate the physical address and pull the HPD low. Note: you'll need this recent patch: "adv7604: when the EDID is cleared, unconfigure CEC as well" Regards, Hans
Re: [PATCH] adv7604: add CEC support for adv7611/adv7612
Hi Hans,
Thanks for your patch.
On 2018-10-12 13:30:02 +0200, Hans Verkuil wrote:
> The CEC IP is very similar between the three HDMI receivers, but
> not identical. Add support for all three variants.
>
> Tested with an adv7604 and an adv7612.
>
> Signed-off-by: Hans Verkuil
This fixes CEC on my Koelsch board using the adv7604.
Reviewed-by: Niklas Söderlund
Side note do you know of a way to simulate a cycling of the physical HDMI
cable? My current test-case for CEC is:
v4l2-ctl -d $(grep -l "adv7612" /sys/class/video4linux/*/name | sed
's#.*video4linux\(.*\)/name#/dev\1#g') --set-edid=type=hdmi
cec-ctl -d 0 --playback
cec-ctl -d 1 --tv
# Here I need to attach or if it already is connected disconnect and
# reconnect the HDMI cable
cec-ctl -d 0 -S
cec-ctl -d 1 -S
If that step could be done in software I can add this test to my
automatic test scripts which would be nice.
> ---
> drivers/media/i2c/adv7604.c | 63 +++--
> 1 file changed, 53 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 9eb7c70a7712..88786276dbe4 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -114,6 +114,11 @@ struct adv76xx_chip_info {
> unsigned int fmt_change_digital_mask;
> unsigned int cp_csc;
>
> + unsigned int cec_irq_status;
> + unsigned int cec_rx_enable;
> + unsigned int cec_rx_enable_mask;
> + bool cec_irq_swap;
> +
> const struct adv76xx_format_info *formats;
> unsigned int nformats;
>
> @@ -2003,10 +2008,11 @@ static void adv76xx_cec_tx_raw_status(struct
> v4l2_subdev *sd, u8 tx_raw_status)
> static void adv76xx_cec_isr(struct v4l2_subdev *sd, bool *handled)
> {
> struct adv76xx_state *state = to_state(sd);
> + const struct adv76xx_chip_info *info = state->info;
> u8 cec_irq;
>
> /* cec controller */
> - cec_irq = io_read(sd, 0x4d) & 0x0f;
> + cec_irq = io_read(sd, info->cec_irq_status) & 0x0f;
> if (!cec_irq)
> return;
>
> @@ -2024,15 +2030,21 @@ static void adv76xx_cec_isr(struct v4l2_subdev *sd,
> bool *handled)
>
> for (i = 0; i < msg.len; i++)
> msg.msg[i] = cec_read(sd, i + 0x15);
> - cec_write(sd, 0x26, 0x01); /* re-enable rx */
> + cec_write(sd, info->cec_rx_enable,
> + info->cec_rx_enable_mask); /* re-enable rx */
> cec_received_msg(state->cec_adap, &msg);
> }
> }
>
> - /* note: the bit order is swapped between 0x4d and 0x4e */
> - cec_irq = ((cec_irq & 0x08) >> 3) | ((cec_irq & 0x04) >> 1) |
> - ((cec_irq & 0x02) << 1) | ((cec_irq & 0x01) << 3);
> - io_write(sd, 0x4e, cec_irq);
> + if (info->cec_irq_swap) {
> + /*
> + * Note: the bit order is swapped between 0x4d and 0x4e
> + * on adv7604
> + */
> + cec_irq = ((cec_irq & 0x08) >> 3) | ((cec_irq & 0x04) >> 1) |
> + ((cec_irq & 0x02) << 1) | ((cec_irq & 0x01) << 3);
> + }
> + io_write(sd, info->cec_irq_status + 1, cec_irq);
>
> if (handled)
> *handled = true;
> @@ -2041,6 +2053,7 @@ static void adv76xx_cec_isr(struct v4l2_subdev *sd,
> bool *handled)
> static int adv76xx_cec_adap_enable(struct cec_adapter *adap, bool enable)
> {
> struct adv76xx_state *state = cec_get_drvdata(adap);
> + const struct adv76xx_chip_info *info = state->info;
> struct v4l2_subdev *sd = &state->sd;
>
> if (!state->cec_enabled_adap && enable) {
> @@ -2052,11 +2065,11 @@ static int adv76xx_cec_adap_enable(struct cec_adapter
> *adap, bool enable)
> /* tx: arbitration lost */
> /* tx: retry timeout */
> /* rx: ready */
> - io_write_clr_set(sd, 0x50, 0x0f, 0x0f);
> - cec_write(sd, 0x26, 0x01);/* enable rx */
> + io_write_clr_set(sd, info->cec_irq_status + 3, 0x0f, 0x0f);
> + cec_write(sd, info->cec_rx_enable, info->cec_rx_enable_mask);
> } else if (state->cec_enabled_adap && !enable) {
> /* disable cec interrupts */
> - io_write_clr_set(sd, 0x50, 0x0f, 0x00);
> + io_write_clr_set(sd, info->cec_irq_status + 3, 0x0f, 0x00);
> /* disable address mask 1-3 */
> cec_write_clr_set(sd, 0x27, 0x70, 0x00);
> /* power down cec section */
> @@ -2221,6 +2234,16 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32
> status, bool *handled)
> return 0;
> }
>
> +static irqreturn_t adv76xx_irq_handler(int irq, void *dev_id)
> +{
> + struct adv76xx_state *state = dev_id;
> + bool handled = false;
> +
> + adv76xx_isr(&state->sd, 0, &handled);
> +
> + return handled ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> st
Re: [PATCH 1/4] media: ov5640: fix resolution update
Hi Laurent, Jacopo, Sam,
I'm also OK to change to a simpler alternative;
- drop the "restore" step
- send the whole init register sequence + mode changes + format changes
at streamon
is this what you have in mind Laurent ?
On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
>> Hi Sam,
>> thanks for the patch, I see the same issue you reported, but I
>> think this patch can be improved.
>>
>> (expanding the Cc list to all people involved in recent ov5640
>> developemts, not just for this patch, but for the whole series to look
>> at. Copying names from another series cover letter, hope it is
>> complete.)
>>
>> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
>>> set_fmt was not properly triggering a mode change when
>>> a new mode was set that happened to have the same format
>>> as the previous mode (for example, when only changing the
>>> frame dimensions). Fix this.
>>>
>>> Signed-off-by: Sam Bobrowicz
>>> ---
>>>
>>> drivers/media/i2c/ov5640.c | 8
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>>> index eaefdb5..5031aab 100644
>>> --- a/drivers/media/i2c/ov5640.c
>>> +++ b/drivers/media/i2c/ov5640.c
>>> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>>>
>>> goto out;
>>>
>>> }
>>>
>>> - if (new_mode != sensor->current_mode) {
>>> +
>>> + if (new_mode != sensor->current_mode ||
>>> + mbus_fmt->code != sensor->fmt.code) {
>>> + sensor->fmt = *mbus_fmt;
>>>
>>> sensor->current_mode = new_mode;
>>> sensor->pending_mode_change = true;
>>>
>>> - }
>>> - if (mbus_fmt->code != sensor->fmt.code) {
>>> - sensor->fmt = *mbus_fmt;
>>>
>>> sensor->pending_fmt_change = true;
>>>
>>> }
>>
>> How I did reproduce the issue:
>>
>> # Set 1024x768 on ov5640 without changing the image format
>> # (default image size at startup is 640x480)
>> $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768 field:none]"
>>sensor->pending_mode_change = true; //verified this flag gets set
>>
>> # Start streaming, after having configured the whole pipeline to work
>> # with 1024x768
>> $ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
>> Unable to start streaming: Broken pipe (32).
>>
>> # Inspect which part of pipeline validation went wrong
>> # Turns out the sensor->fmt field is not updated, and when get_fmt()
>> # is called, the old one is returned.
>> $ media-ctl -e "ov5640 2-003c" -p
>>...
>>[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ycbcr:601
>> quantization:full-range] ^^^ ^^^
>>
>> So yes, sensor->fmt is not udapted as it should be when only image
>> resolution is changed.
>>
>> Although I still see value in having two separate flags for the
>> 'mode_change' (which in ov5640 lingo is resolution) and 'fmt_change' (which
>> in ov5640 lingo is the image format), and write their configuration to
>> registers only when they get actually changed.
>>
>> For this reasons I would like to propse the following patch which I
>> have tested by:
>> 1) changing resolution only
>> 2) changing format only
>> 3) change both
>>
>> What do you and others think?
>
> I think that the format setting code should be completely rewritten, it's
> pretty much unmaintainable as-is.
>
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index eaefdb5..e392b9d 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -2020,6 +2020,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>> struct ov5640_dev *sensor = to_ov5640_dev(sd);
>> const struct ov5640_mode_info *new_mode;
>> struct v4l2_mbus_framefmt *mbus_fmt = &format->format;
>> + struct v4l2_mbus_framefmt *fmt;
>> int ret;
>>
>> if (format->pad != 0)
>> @@ -2037,22 +2038,19 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>> if (ret)
>> goto out;
>>
>> - if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> - struct v4l2_mbus_framefmt *fmt =
>> - v4l2_subdev_get_try_format(sd, cfg, 0);
>> + if (format->which == V4L2_SUBDEV_FORMAT_TRY)
>> + fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
>> + else
>> + fmt = &sensor->fmt;
>>
>> - *fmt = *mbus_fmt;
>> - goto out;
>> - }
>> + *fmt = *mbus_fmt;
>>
>> if (new_mode != sensor->current_mode) {
>> sensor->current_mode = new_mode;
>> sensor->pending_mode_change = true;
>> }
>> - if (mbus_fmt->code != sensor->fmt.code) {
>> - sensor->fmt = *mbus_fmt;
>> + if (mbus_fmt->code != sensor->fmt.code)
>> sensor->pending_fmt_change = true;
>> - }
>> out:
>>
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
Hi Maxime,
I've recently found a problem around JPEG framerate, see below:
On 10/11/2018 11:20 AM, Maxime Ripard wrote:
> The clock structure for the PCLK is quite obscure in the documentation, and
> was hardcoded through the bytes array of each and every mode.
>
> This is troublesome, since we cannot adjust it at runtime based on other
> parameters (such as the number of bytes per pixel), and we can't support
> either framerates that have not been used by the various vendors, since we
> don't have the needed initialization sequence.
>
> We can however understand how the clock tree works, and then implement some
> functions to derive the various parameters from a given rate. And now that
> those parameters are calculated at runtime, we can remove them from the
> initialization sequence.
>
> The modes also gained a new parameter which is the clock that they are
> running at, from the register writes they were doing, so for now the switch
> to the new algorithm should be transparent.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/media/i2c/ov5640.c | 289 -
> 1 file changed, 288 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 30b15e91d8be..88fb16341466 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -175,6 +175,7 @@ struct ov5640_mode_info {
> u32 htot;
> u32 vact;
> u32 vtot;
> + u32 pixel_clock;
> const struct reg_value *reg_data;
> u32 reg_data_size;
> };
> @@ -700,6 +701,7 @@ static const struct reg_value
> ov5640_setting_15fps_QSXGA_2592_1944[] = {
> /* power-on sensor init reg table */
> static const struct ov5640_mode_info ov5640_mode_init_data = {
> 0, SUBSAMPLING, 640, 1896, 480, 984,
> + 5600,
> ov5640_init_setting_30fps_VGA,
> ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
> };
> @@ -709,74 +711,91 @@
> ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>176, 1896, 144, 984,
> + 2800,
>ov5640_setting_15fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> + 2800,
>ov5640_setting_15fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> + 2800,
>ov5640_setting_15fps_VGA_640_480,
>ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>720, 1896, 480, 984,
> + 2800,
>ov5640_setting_15fps_NTSC_720_480,
>ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> {OV5640_MODE_PAL_720_576, SUBSAMPLING,
>720, 1896, 576, 984,
> + 2800,
>ov5640_setting_15fps_PAL_720_576,
>ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>1024, 1896, 768, 1080,
> + 2800,
>ov5640_setting_15fps_XGA_1024_768,
>ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>1280, 1892, 720, 740,
> + 2100,
>ov5640_setting_15fps_720P_1280_720,
>ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> {OV5640_MODE_1080P_1920_1080, SCALING,
>1920, 2500, 1080, 1120,
> + 4200,
>ov5640_setting_15fps_1080P_1920_1080,
>ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> {OV5640_MODE_QSXGA_2592_1944, SCALING,
>2592, 2844, 1944, 1968,
> + 8400,
>ov5640_setting_15fps_QSXGA_2592_1944,
>ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
> }, {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>176, 1896, 144, 984,
> + 5600,
>ov5640_setting_30fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> + 5600,
>ov5640_setting_30fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
> {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> + 5600,
>ov5640_setting_30fps_VGA_640_480,
>ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
> {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>720, 1896, 480, 9
Re: [PATCH v4 12/12] ov5640: Enforce a mode change when changing the framerate
Hi Maxime,
This is already fixed in media tree:
0929983e49c81c1d413702cd9b83bb06c4a2555c media: ov5640: fix framerate update
On 10/11/2018 11:21 AM, Maxime Ripard wrote:
> The current logic only requires to call ov5640_set_mode, which will in turn
> change the clock rates according to the mode and frame interval, when a new
> mode is set up.
>
> However, when only the frame interval is changed but the mode isn't,
> ov5640_set_mode is never called and the resulting frame rate will be old or
> default one. Fix this by requiring that ov5640_set_mode is called when the
> frame interval is changed as well.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/media/i2c/ov5640.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 818411400ef6..e01d2cb93c67 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2638,8 +2638,12 @@ static int ov5640_s_frame_interval(struct v4l2_subdev
> *sd,
> goto out;
> }
>
> - sensor->current_fr = frame_rate;
> - sensor->frame_interval = fi->interval;
> + if (frame_rate != sensor->current_fr) {
> + sensor->current_fr = frame_rate;
> + sensor->frame_interval = fi->interval;
> + sensor->pending_mode_change = true;
> + }
> +
> mode = ov5640_find_mode(sensor, frame_rate, mode->hact,
> mode->vact, true);
> if (!mode) {
>
BR,
Hugues.
[ANN] Agenda for the media summit on Thursday Oct 25th in Edinburgh
Hi all, We are organizing a media mini-summit on Thursday October 25th in Edinburgh, Edinburgh International Conference Centre. If you plan to attend, please register on the ELCE/OSS site since we're using there tracking system: https://events.linuxfoundation.org/events/elc-openiot-europe-2018/register/ Name of the room for the summit: Tinto, Level 0 of the EICC We had 75 people sign up for the summit as of a week ago, which is quite amazing. I'm not listing all of them here, just those that I know are active media developers: Sakari Ailus Neil Armstrong Kieran Bingham Mauro Carvalho Chehab Nicolas Dufresne (Collabora) Ezequiel Garcia Helen Koike Michael Ira Krufky (Vimeo/Livestream) Brad Love Jacopo Mondi Gustavo Padovan Laurent Pinchart Ricardo Ribalda Delgado (Qtechnology A/S) Maxime Ripard Niklas Söderlund Hans Verkuil (Cisco) Sean Young (Monax) Agenda == General remarks: the given start/end times for the various topics are approximate since it is always hard to predict how long a discussion will take. If people are attending other summits and those conflict with specific topics they want to be part of, then let me know and we can rearrange the schedule to (hopefully) accomodate that. Let me know asap if there are problems with this schedule, or if new topics are requested. 9:00-9:25: Introduction (Hans Verkuil) Settling in, hooking everything up, getting wifi/projector/etc. to work, drinking coffee/tea/water and a short intro :-) 9:25-9:35: Status of the HDMI CEC kernel support (Hans Verkuil) Give a quick overview of the status: what has been merged, what is still pending, what is under development. 9:35-9:45: Status of the RC kernel support (Sean Young) A 10 minute status update on rc-core, present and future. I'll give a brief presentation and leave some time for discussion. 9:45-10:00: Save/restore controls from MTD (Ricardo Ribalda Delgado) Industrial/Scientific sensors usually come with very extensive calibration information such as: per column gain, list of dead pixels, temperature sensor offset... etc We are saving that information on an flash device that is located by the sensor. Show how we are integrating that calibration flash with v4l2-ctrl. And if this feature is useful for someone else and upstream it. 10:00-10:10: dri-devel and 'dim' (Laurent Pinchart) Experiences (good and bad) with the dri-devel 'dim' utility. 10:10-11:00: Automated Testing (Ezequiel Garcia) There is a lot of discussion going on around testing, so it's a good opportunity for us to talk about our current testing infrastructure. We are already doing a good job with v4l2-compliance. Can we do more? 11:00-11:15: Break 11:15-12:00: Stateless Codec userspace (Hans Verkuil) Support for stateless codecs and Request API should be merged for 4.20, and the next step is to discuss how to organize the userspace support. Hopefully by the time the media summit starts we'll have some better ideas of what we want in this area. 12:00-13:30: Lunch 13:30-14:30: Which ioctls should be replaced with better versions? (Hans Verkuil) Some parts of the V4L2 API are awkward to use and I think it would be a good idea to look at possible candidates for that. Examples are the ioctls that use struct v4l2_buffer: the multiplanar support is really horrible, and writing code to support both single and multiplanar is hard. We are also running out of fields and the timeval isn't y2038 compliant. A proof-of-concept is here: https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=v4l2-buffer&id=a95549df06d9900f3559afdbb9da06bd4b22d1f3 It's a bit old, but it gives a good impression of what I have in mind. Another candidate is VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL/VIDIOC_ENUM_FRAMEINTERVALS: expressing frame intervals as a fraction is really awkward and so is the fact that the subdev and 'normal' ioctls are not the same. Discuss what possible other ioctls are candidates for a refresh. 14:30-15:00: Fault tolerant V4L2 (Kieran Bingham) In other words, how should we handle complex devices which do not 'fully probe' since one or more subdevices (e.g. sensors) are broken (or break while in use!). 15:00-15:15: Break 15:15-16:00: Tentative: Complex Cameras (Mauro Carvalho Chehab) The idea is to discuss about the undergoing work with complex camera development is happening. As we're working to merge request API, another topic for discussion is how to add support for requests on it (or on a separate but related library). Note: to be confirmed, this topic might be dropped. 16:00-16:30: Discuss the media development process Since we are all here, d
[GIT FIXES FOR v4.20] cec: forgot to cancel delayed work
Urgent bug fix for kernel oops in CEC corner case. Also Cc-ed for stable to 4.18 since it fixes a bug in commit 7ec2b3b941a6, which was also slated for the stable release. Regards, Hans The following changes since commit 8caec72e8cbff65afa38928197bea5a393b67975: media: vivid: Support 480p for webcam capture (2018-10-09 10:37:54 -0400) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git tags/br-v4.20i for you to fetch changes up to 6ec72c987e0584cf72c7eda5a1fc7a8e1b409198: cec: forgot to cancel delayed work (2018-10-15 12:16:14 +0200) Tag branch Hans Verkuil (1): cec: forgot to cancel delayed work drivers/media/cec/cec-adap.c | 2 ++ 1 file changed, 2 insertions(+)
[PATCH for 4.20] cec: forgot to cancel delayed work
If the wait for completion was interrupted, then make sure to cancel
any delayed work.
This can only happen if a transmit is waiting for a reply, and you press
Ctrl-C or reboot/poweroff or something like that which interrupts the
thread waiting for the reply and then proceeds to delete the CEC message.
Since the delayed work wasn't canceled, once it would trigger it referred
to stale data and resulted in a kernel oops.
Signed-off-by: Hans Verkuil
Fixes: 7ec2b3b941a6 ("cec: add new tx/rx status bits to detect aborts/timeouts")
---
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 0c0d9107383e..31d1f4ab915e 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -844,6 +844,8 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct
cec_msg *msg,
*/
mutex_unlock(&adap->lock);
wait_for_completion_killable(&data->c);
+ if (!data->completed)
+ cancel_delayed_work_sync(&data->work);
mutex_lock(&adap->lock);
/* Cancel the transmit if it was interrupted */
Re: [PATCH 1/2] media: docs-rst: Document memory-to-memory video decoder interface
On Wed, Aug 8, 2018 at 11:55 AM Tomasz Figa wrote: > > On Tue, Aug 7, 2018 at 4:37 PM Hans Verkuil wrote: > > > > On 08/07/2018 09:05 AM, Tomasz Figa wrote: > > > On Thu, Jul 26, 2018 at 7:57 PM Hans Verkuil wrote: > > I wonder if we should make these min buffer controls required. It > > might be easier > > that way. > > >>> > > >>> Agreed. Although userspace is still free to ignore it, because REQBUFS > > >>> would do the right thing anyway. > > >> > > >> It's never been entirely clear to me what the purpose of those min > > >> buffers controls > > >> is. REQBUFS ensures that the number of buffers is at least the minimum > > >> needed to > > >> make the HW work. So why would you need these controls? It only makes > > >> sense if they > > >> return something different from REQBUFS. > > >> > > > > > > The purpose of those controls is to let the client allocate a number > > > of buffers bigger than minimum, without the need to allocate the > > > minimum number of buffers first (to just learn the number), free them > > > and then allocate a bigger number again. > > > > I don't feel this is particularly useful. One problem with the minimum > > number > > of buffers as used in the kernel is that it is often the minimum number of > > buffers required to make the hardware work, but it may not be optimal. E.g. > > quite a few capture drivers set the minimum to 2, which is enough for the > > hardware, but it will likely lead to dropped frames. You really need 3 > > (one is being DMAed, one is queued and linked into the DMA engine and one is > > being processed by userspace). > > > > I would actually prefer this to be the recommended minimum number of > > buffers, > > which is >= the minimum REQBUFS uses. > > > > I.e., if you use this number and you have no special requirements, then > > you'll > > get good performance. > > I guess we could make it so. It would make existing user space request > more buffers than it used to with the original meaning, but I guess it > shouldn't be a big problem. I gave it a bit more thought and I feel like kernel is not the right place to put any assumptions on what the userspace expects "good performance" to be. Actually, having these controls return the minimum number of buffers as REQBUFS would allocate makes it very well specified - with this number you can only process frame by frame and the number of buffers added by userspace defines exactly the queue depth. It leaves no space for driver-specific quirks, because the driver doesn't decide what's "good performance" anymore. Best regards, Tomasz
Re: [PATCH 6/8] drm: sti: don't pass GFP_DMA32 to dma_alloc_wc
Le sam. 13 oct. 2018 à 17:17, Christoph Hellwig a écrit :
>
> The DMA API does its own zone decisions based on the coherent_dma_mask.
>
> Signed-off-by: Christoph Hellwig
Reviewed-by: Benjamin Gaignard
> ---
> drivers/gpu/drm/sti/sti_gdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> index c32de6cbf061..cdf0a1396e00 100644
> --- a/drivers/gpu/drm/sti/sti_gdp.c
> +++ b/drivers/gpu/drm/sti/sti_gdp.c
> @@ -517,7 +517,7 @@ static void sti_gdp_init(struct sti_gdp *gdp)
> /* Allocate all the nodes within a single memory page */
> size = sizeof(struct sti_gdp_node) *
> GDP_NODE_PER_FIELD * GDP_NODE_NB_BANK;
> - base = dma_alloc_wc(gdp->dev, size, &dma_addr, GFP_KERNEL | GFP_DMA);
> + base = dma_alloc_wc(gdp->dev, size, &dma_addr, GFP_KERNEL);
>
> if (!base) {
> DRM_ERROR("Failed to allocate memory for GDP node\n");
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 7/8] media: sti/bdisp: don't pass GFP_DMA32 to dma_alloc_attrs
Le sam. 13 oct. 2018 à 17:18, Christoph Hellwig a écrit : > > The DMA API does its own zone decisions based on the coherent_dma_mask. > > Signed-off-by: Christoph Hellwig Reviewed-by: Benjamin Gaignard > --- > drivers/media/platform/sti/bdisp/bdisp-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c > b/drivers/media/platform/sti/bdisp/bdisp-hw.c > index 26d9fa7aeb5f..4372abbb5950 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c > @@ -510,7 +510,7 @@ int bdisp_hw_alloc_filters(struct device *dev) > > /* Allocate all the filters within a single memory page */ > size = (BDISP_HF_NB * NB_H_FILTER) + (BDISP_VF_NB * NB_V_FILTER); > - base = dma_alloc_attrs(dev, size, &paddr, GFP_KERNEL | GFP_DMA, > + base = dma_alloc_attrs(dev, size, &paddr, GFP_KERNEL, >DMA_ATTR_WRITE_COMBINE); > if (!base) > return -ENOMEM; > -- > 2.19.1 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 1/2] ipu3-cio2: Unregister device nodes first, then release resources
On 10/15/2018 04:39 PM, Sakari Ailus wrote: > Hi Bingbu, > > On Mon, Oct 15, 2018 at 03:15:05PM +0800, Bing Bu Cao wrote: >> On 10/10/2018 04:32 PM, Sakari Ailus wrote: >>> While there are issues related to object lifetime management, unregister >>> the media device first, followed immediately by other device nodes when >>> the driver is being unbound. Only then the resources needed by the driver >>> may be released. This is slightly safer. >>> >>> Signed-off-by: Sakari Ailus >>> --- >>> drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c >>> b/drivers/media/pci/intel/ipu3/ipu3-cio2.c >>> index 452eb9b42140..723022ef3662 100644 >>> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c >>> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c >>> @@ -1846,12 +1846,12 @@ static void cio2_pci_remove(struct pci_dev *pci_dev) >>> struct cio2_device *cio2 = pci_get_drvdata(pci_dev); >>> unsigned int i; >>> >>> + media_device_unregister(&cio2->media_dev); >>> cio2_notifier_exit(cio2); >>> - cio2_fbpt_exit_dummy(cio2); >>> for (i = 0; i < CIO2_QUEUES; i++) >>> cio2_queue_exit(cio2, &cio2->queue[i]); >>> + cio2_fbpt_exit_dummy(cio2); >> Hi, Sakari, >> The fbpt dummy pages cleanup does not matter much before/after queues >> exit, right? > cio2_queue_exit() will unregister the video device and the video buffer > queue. Up to this point it's possible to open the video device and start > streaming on it. While this patch does not fully address the issue it makes > it a slightly lesser issue. Okay, thanks for your explanation. >>> v4l2_device_unregister(&cio2->v4l2_dev); >>> - media_device_unregister(&cio2->media_dev); >>> media_device_cleanup(&cio2->media_dev); >>> mutex_destroy(&cio2->lock); >>> }
Re: [PATCH 1/2] ipu3-cio2: Unregister device nodes first, then release resources
Hi Bingbu, On Mon, Oct 15, 2018 at 03:15:05PM +0800, Bing Bu Cao wrote: > > On 10/10/2018 04:32 PM, Sakari Ailus wrote: > > While there are issues related to object lifetime management, unregister > > the media device first, followed immediately by other device nodes when > > the driver is being unbound. Only then the resources needed by the driver > > may be released. This is slightly safer. > > > > Signed-off-by: Sakari Ailus > > --- > > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > index 452eb9b42140..723022ef3662 100644 > > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > @@ -1846,12 +1846,12 @@ static void cio2_pci_remove(struct pci_dev *pci_dev) > > struct cio2_device *cio2 = pci_get_drvdata(pci_dev); > > unsigned int i; > > > > + media_device_unregister(&cio2->media_dev); > > cio2_notifier_exit(cio2); > > - cio2_fbpt_exit_dummy(cio2); > > for (i = 0; i < CIO2_QUEUES; i++) > > cio2_queue_exit(cio2, &cio2->queue[i]); > > + cio2_fbpt_exit_dummy(cio2); > Hi, Sakari, > The fbpt dummy pages cleanup does not matter much before/after queues > exit, right? cio2_queue_exit() will unregister the video device and the video buffer queue. Up to this point it's possible to open the video device and start streaming on it. While this patch does not fully address the issue it makes it a slightly lesser issue. > > v4l2_device_unregister(&cio2->v4l2_dev); > > - media_device_unregister(&cio2->media_dev); > > media_device_cleanup(&cio2->media_dev); > > mutex_destroy(&cio2->lock); > > } > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [RFC] Informal meeting during ELCE to discuss userspace support for stateless codecs
+Alexandre Courbot On Wed, Oct 10, 2018 at 3:55 PM Hans Verkuil wrote: > > On 10/08/2018 01:53 PM, Hans Verkuil wrote: > > Hi all, > > > > I would like to meet up somewhere during the ELCE to discuss userspace > > support > > for stateless (and perhaps stateful as well?) codecs. > > > > It is also planned as a topic during the summit, but I would prefer to > > prepare > > for that in advance, esp. since I myself do not have any experience writing > > userspace SW for such devices. > > > > Nicolas, it would be really great if you can participate in this meeting > > since you probably have the most experience with this by far. > > > > Looking through the ELCE program I found two timeslots that are likely to > > work > > for most of us (because the topics in the program appear to be boring for us > > media types!): > > > > Tuesday from 10:50-15:50 > > Let's do this Tuesday. Let's meet at the Linux Foundation Registration > Desk at 11:00. I'll try to figure out where we can sit the day before. > Please check your email Tuesday morning for any last minute changes. > > Tomasz, it would be nice indeed if we can get you and Paul in as well > using Hangouts on my laptop. That would be great. Please let me know if you need any help with setup. Best regards, Tomasz > > I would very much appreciate it if those who have experience with the > userspace support think about this beforehand and make some requirements > list of what you would like to see. > > Regards, > > Hans > > > > > or: > > > > Monday from 15:45 onward > > > > My guess is that we need 2-3 hours or so. Hard to predict. > > > > The basic question that I would like to have answered is what the userspace > > component should look like? libv4l-like plugin or a library that userspace > > can > > link with? Do we want more general support for stateful codecs as well that > > deals > > with resolution changes and the more complex parts of the codec API? > > > > I've mailed this directly to those that I expect are most interested in > > this, > > but if someone want to join in let me know. > > > > I want to keep the group small though, so you need to bring relevant > > experience > > to the table. > > > > Regards, > > > > Hans > > >
Re: [PATCH 1/8] cpufreq: tegra186: don't pass GFP_DMA32 to dma_alloc_coherent
On Sat, Oct 13, 2018 at 5:17 PM Christoph Hellwig wrote: > > The DMA API does its own zone decisions based on the coherent_dma_mask. > > Signed-off-by: Christoph Hellwig Acked-by: Rafael J. Wysocki > --- > drivers/cpufreq/tegra186-cpufreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/tegra186-cpufreq.c > b/drivers/cpufreq/tegra186-cpufreq.c > index 1f59966573aa..f1e09022b819 100644 > --- a/drivers/cpufreq/tegra186-cpufreq.c > +++ b/drivers/cpufreq/tegra186-cpufreq.c > @@ -121,7 +121,7 @@ static struct cpufreq_frequency_table *init_vhint_table( > void *virt; > > virt = dma_alloc_coherent(bpmp->dev, sizeof(*data), &phys, > - GFP_KERNEL | GFP_DMA32); > + GFP_KERNEL); > if (!virt) > return ERR_PTR(-ENOMEM); > > -- > 2.19.1 >
Re: [PATCH 1/2] ipu3-cio2: Unregister device nodes first, then release resources
On 10/10/2018 04:32 PM, Sakari Ailus wrote: > While there are issues related to object lifetime management, unregister > the media device first, followed immediately by other device nodes when > the driver is being unbound. Only then the resources needed by the driver > may be released. This is slightly safer. > > Signed-off-by: Sakari Ailus > --- > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > index 452eb9b42140..723022ef3662 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > @@ -1846,12 +1846,12 @@ static void cio2_pci_remove(struct pci_dev *pci_dev) > struct cio2_device *cio2 = pci_get_drvdata(pci_dev); > unsigned int i; > > + media_device_unregister(&cio2->media_dev); > cio2_notifier_exit(cio2); > - cio2_fbpt_exit_dummy(cio2); > for (i = 0; i < CIO2_QUEUES; i++) > cio2_queue_exit(cio2, &cio2->queue[i]); > + cio2_fbpt_exit_dummy(cio2); Hi, Sakari, The fbpt dummy pages cleanup does not matter much before/after queues exit, right? > v4l2_device_unregister(&cio2->v4l2_dev); > - media_device_unregister(&cio2->media_dev); > media_device_cleanup(&cio2->media_dev); > mutex_destroy(&cio2->lock); > }
