[v2,09/15] dt-bindings: display: Add max-res property for mxsfb
diff mbox series

Message ID 1565779731-1300-10-git-send-email-robert.chiras@nxp.com
State Needs Review / ACK
Headers show
Series
  • Improvements and fixes for mxsfb DRM driver
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Robert Chiras Aug. 14, 2019, 10:48 a.m. UTC
Add new optional property 'max-res', to limit the maximum supported
resolution by the MXSFB_DRM driver.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Stefan Agner Aug. 14, 2019, 11:03 a.m. UTC | #1
On 2019-08-14 12:48, Robert Chiras wrote:
> Add new optional property 'max-res', to limit the maximum supported
> resolution by the MXSFB_DRM driver.

I would also mention the reason why we need this.

I guess this needs a vendor prefix as well (fsl,max-res). I also would
like to have the ack of the device tree folks here.

--
Stefan

> 
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> ---
>  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
> b/Documentation/devicetree/bindings/display/mxsfb.txt
> index 472e1ea..55e22ed 100644
> --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> @@ -17,6 +17,12 @@ Required properties:
>  Required sub-nodes:
>    - port: The connection to an encoder chip.
>  
> +Optional properties:
> +- max-res:	an array with a maximum of two integers, representing the
> +		maximum supported resolution, in the form of
> +		<maxX>, <maxY>; if one of the item is <0>, the default
> +		driver-defined maximum resolution for that axis is used
> +
>  Example:
>  
>  	lcdif1: display-controller@2220000 {
Robert Chiras Aug. 14, 2019, 11:14 a.m. UTC | #2
Hi Stefan,
On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> > 
> > Add new optional property 'max-res', to limit the maximum supported
> > resolution by the MXSFB_DRM driver.
> I would also mention the reason why we need this.
> 
> I guess this needs a vendor prefix as well (fsl,max-res). I also
> would
> like to have the ack of the device tree folks here.
Rob Herring also aked be about this, and I'll copy here the reply, with
explanations:

Indeed, this limitation is actually due to bandwidth limitation, but
the problem is that this limitation comes on i.MX8M (known as mScale
850D), where the memory bandwidth cannot support: GPU/VPU workload in
the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p@60.
Since eLCDIF is a secondary display we though to add the posibility to
limit it's bandwidth by limiting the resolution.
If you say that more details are needed, I can add them in the
description.
> 
> --
> Stefan
> 
> > 
> > 
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
> > b/Documentation/devicetree/bindings/display/mxsfb.txt
> > index 472e1ea..55e22ed 100644
> > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> > @@ -17,6 +17,12 @@ Required properties:
> >  Required sub-nodes:
> >    - port: The connection to an encoder chip.
> > 
> > +Optional properties:
> > +- max-res:   an array with a maximum of two integers, representing
> > the
> > +             maximum supported resolution, in the form of
> > +             <maxX>, <maxY>; if one of the item is <0>, the
> > default
> > +             driver-defined maximum resolution for that axis is
> > used
> > +
> >  Example:
> > 
> >       lcdif1: display-controller@2220000 {

Thanks,
Robert
Stefan Agner Aug. 14, 2019, 11:25 a.m. UTC | #3
On 2019-08-14 13:14, Robert Chiras wrote:
> Hi Stefan,
> On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
>> On 2019-08-14 12:48, Robert Chiras wrote:
>> >
>> > Add new optional property 'max-res', to limit the maximum supported
>> > resolution by the MXSFB_DRM driver.
>> I would also mention the reason why we need this.
>>
>> I guess this needs a vendor prefix as well (fsl,max-res). I also
>> would
>> like to have the ack of the device tree folks here.
> Rob Herring also aked be about this, and I'll copy here the reply, with
> explanations:
> 
> Indeed, this limitation is actually due to bandwidth limitation, but
> the problem is that this limitation comes on i.MX8M (known as mScale
> 850D), where the memory bandwidth cannot support: GPU/VPU workload in
> the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p@60.
> Since eLCDIF is a secondary display we though to add the posibility to
> limit it's bandwidth by limiting the resolution.
> If you say that more details are needed, I can add them in the
> description.

Oh sorry I missed that.

Rob Herring also wrote:
> I suppose what you are after is bandwidth limits? IIRC, there's already 
> some bindings expressing such limits. Also, wouldn't you need to account 
> for bpp and using the 2nd plane (IIRC that there is one).

I guess the binding he refers to is max-memory-bandwidth, which is used
in multiple driver already. It makes sense to reuse this property
instead of inventing a new set of property which is also not taking bpp
into account...

The pl111 driver implements this property, it should be fairly easy to
adopt that code.

--
Stefan



>>
>> --
>> Stefan
>>
>> >
>> >
>> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
>> > ---
>> >  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
>> > b/Documentation/devicetree/bindings/display/mxsfb.txt
>> > index 472e1ea..55e22ed 100644
>> > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
>> > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
>> > @@ -17,6 +17,12 @@ Required properties:
>> >  Required sub-nodes:
>> >    - port: The connection to an encoder chip.
>> >
>> > +Optional properties:
>> > +- max-res:   an array with a maximum of two integers, representing
>> > the
>> > +             maximum supported resolution, in the form of
>> > +             <maxX>, <maxY>; if one of the item is <0>, the
>> > default
>> > +             driver-defined maximum resolution for that axis is
>> > used
>> > +
>> >  Example:
>> >
>> >       lcdif1: display-controller@2220000 {
> 
> Thanks,
> Robert
Robert Chiras Aug. 14, 2019, 12:03 p.m. UTC | #4
Hi Stefann,
On Mi, 2019-08-14 at 13:25 +0200, Stefan Agner wrote:
> On 2019-08-14 13:14, Robert Chiras wrote:
> > 
> > Hi Stefan,
> > On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
> > > 
> > > On 2019-08-14 12:48, Robert Chiras wrote:
> > > > 
> > > > 
> > > > Add new optional property 'max-res', to limit the maximum
> > > > supported
> > > > resolution by the MXSFB_DRM driver.
> > > I would also mention the reason why we need this.
> > > 
> > > I guess this needs a vendor prefix as well (fsl,max-res). I also
> > > would
> > > like to have the ack of the device tree folks here.
> > Rob Herring also aked be about this, and I'll copy here the reply,
> > with
> > explanations:
> > 
> > Indeed, this limitation is actually due to bandwidth limitation,
> > but
> > the problem is that this limitation comes on i.MX8M (known as
> > mScale
> > 850D), where the memory bandwidth cannot support: GPU/VPU workload
> > in
> > the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p
> > @60.
> > Since eLCDIF is a secondary display we though to add the posibility
> > to
> > limit it's bandwidth by limiting the resolution.
> > If you say that more details are needed, I can add them in the
> > description.
> Oh sorry I missed that.
> 
> Rob Herring also wrote:
> > 
> > I suppose what you are after is bandwidth limits? IIRC, there's
> > already
> > some bindings expressing such limits. Also, wouldn't you need to
> > account
> > for bpp and using the 2nd plane (IIRC that there is one).
> I guess the binding he refers to is max-memory-bandwidth, which is
> used
> in multiple driver already. It makes sense to reuse this property
> instead of inventing a new set of property which is also not taking
> bpp
> into account...
Actually I saw this binding, but I thought it is strictly related to
that driver, where it is documented. Now, that you say this one is used
in many drivers (and recalling that you suggested for my 'max-res' to
add a prefix), I get it now.
> 
> The pl111 driver implements this property, it should be fairly easy
> to
> adopt that code.
Sure, I will do something similar here, too.
> 
> --
> Stefan
> 
> 
> 
> > 
> > > 
> > > 
> > > --
> > > Stefan
> > > 
> > > > 
> > > > 
> > > > 
> > > > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > b/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > index 472e1ea..55e22ed 100644
> > > > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > @@ -17,6 +17,12 @@ Required properties:
> > > >  Required sub-nodes:
> > > >    - port: The connection to an encoder chip.
> > > > 
> > > > +Optional properties:
> > > > +- max-res:   an array with a maximum of two integers,
> > > > representing
> > > > the
> > > > +             maximum supported resolution, in the form of
> > > > +             <maxX>, <maxY>; if one of the item is <0>, the
> > > > default
> > > > +             driver-defined maximum resolution for that axis
> > > > is
> > > > used
> > > > +
> > > >  Example:
> > > > 
> > > >       lcdif1: display-controller@2220000 {
> > Thanks,
> > Robert

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt
index 472e1ea..55e22ed 100644
--- a/Documentation/devicetree/bindings/display/mxsfb.txt
+++ b/Documentation/devicetree/bindings/display/mxsfb.txt
@@ -17,6 +17,12 @@  Required properties:
 Required sub-nodes:
   - port: The connection to an encoder chip.
 
+Optional properties:
+- max-res:	an array with a maximum of two integers, representing the
+		maximum supported resolution, in the form of
+		<maxX>, <maxY>; if one of the item is <0>, the default
+		driver-defined maximum resolution for that axis is used
+
 Example:
 
 	lcdif1: display-controller@2220000 {