| Message ID | 1565779731-1300-10-git-send-email-robert.chiras@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Improvements and fixes for mxsfb DRM driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
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 {
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
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
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
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 {
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(+)