Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling

From: Marek Vasut <marex@denx.de>
To: Stefan Agner <stefan@agner.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	David Airlie <airlied@linux.ie>,
	Fabio Estevam <festevam@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	dri-devel@lists.freedesktop.org,
	NXP Linux Team <linux-imx@nxp.com>,
	Daniel Vetter <daniel@ffwll.ch>, Shawn Guo <shawnguo@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
Date: Tue, 21 Jan 2020 22:22:18 +0100	[thread overview]
Message-ID: <d52d2145-d260-eaf1-de3b-a140198dc0cc@denx.de> (raw)
In-Reply-To: <6a2e711d186e216f34c6cbd5b66f2a19@agner.ch>

On 11/14/19 2:50 PM, Stefan Agner wrote:
> On 2019-11-14 14:17, Marek Vasut wrote:
>> The bus_flags and bus_format handling logic does not seem to cover
>> all potential usecases. Specifically, this seems to fail with an
>> "edt,etm0700g0edh6" display attached to an 24bit display interface,
>> with interface-pix-fmt = "rgb24" set in DT.
>>
>> In this specific setup, the panel-simple.c driver entry for the display
>> sets .bus_flags to non-zero value. However, as imxpd->bus_format is set
>> from the DT property "interface-pix-fmt", imx_pd_encoder_atomic_check()
>> will set imx_crtc_state->bus_flags = imxpd->bus_flags even though the
>> imxpd->bus_flags is zero, while the di->bus_flags is correctly set by
>> the panel-simple.c and non-zero. The result is incorrect flags being
>> used for the display configuration and thus an image corruption.
>> (Specifically, DRM_BUS_FLAG_PIXDATA_POSEDGE is not propagated and thus
>> the ipuv3 clocks pixels on the wrong edge).
>>
>> This patch fixes the problem by overriding the imx_crtc_state->bus_format
>> from the imxpd->bus_format only if the DT property "interface-pix-fmt" is
>> present or if the DI provides no formats. Similarly for bus_flags, which
>> are set from imxpd->bus_flags only if the DI provides no formats.
> 
> So this basically prioritizes imxpd->bus_format over what the display
> provides? Is this correct in all situations?
> 
> I was thinking that interface-pix-fmt is the legacy way to define the
> bus format and it should be provided by the display nowadays.
> 
> However, I guess there is the case where you connect a 18-bit display to
> a 24-bit bus (leaving some bits unconnected). Depending on how the
> colors/bits are distributed one cannot use 18-bit mode on SoC side but
> has to use 24-bit. So the bus format becomes a connection specific
> property... I guess the interface-pix-fmt can serve that role. 

So, can this patch go in ? It's fixing an actual issue for me -- very
much what Stefan described above.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel