Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb

From: Stefan Agner <stefan@agner.ch>
To: Robert Chiras <robert.chiras@nxp.com>
Cc: mark.rutland@arm.com, robh+dt@kernel.org,
	dl-linux-imx <linux-imx@nxp.com>,
	marex@denx.de, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, agx@sigxcpu.org, festevam@gmail.com,
	daniel@ffwll.ch, dri-devel@lists.freedesktop.org,
	shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org,
	airlied@linux.ie, kernel@pengutronix.de, s.hauer@pengutronix.de
Subject: Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
Date: Wed, 14 Aug 2019 13:25:05 +0200	[thread overview]
Message-ID: <33feedd20e0fc154c5b736f882d24569@agner.ch> (raw)
In-Reply-To: <1565781243.3209.55.camel@nxp.com>

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