From: Christoph Niedermaier <cniedermaier@dh-electronics.com> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "Marek MV. Vasut" <marex@denx.de>, Sam Ravnborg <sam@ravnborg.org>, "Maxime Ripard" <mripard@kernel.org>, Philipp Zabel <p.zabel@pengutronix.de>, "David Airlie" <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, "NXP Linux Team" <linux-imx@nxp.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: RE: [RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property" Date: Tue, 8 Feb 2022 21:27:17 +0000 [thread overview] Message-ID: <4a2f4707c03d4509aa5f3a388cbfbe04@dh-electronics.com> (raw) In-Reply-To: <YfsXt1lU6l9cSctX@pendragon.ideasonboard.com> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com] Sent: Thursday, February 3, 2022 12:46 AM > > Hi Christoph, > Hi Laurent, > On Tue, Feb 01, 2022 at 12:07:17PM +0100, Christoph Niedermaier wrote: >> Without the data-mapping devicetree property my display won't >> work properly. It is flickering, because the bus flags won't >> be assigned without a defined bus format by the imx parallel >> display driver. There was a discussion about the removal [1] >> and an agreement that a better solution is needed, but it is >> missing so far. So what would be the better approach? >> >> [1] https://patchwork.freedesktop.org/patch/357659/?series=74705&rev=1 >> >> This reverts commit d021d751c14752a0266865700f6f212fab40a18c. >> >> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >> Cc: Marek Vasut <marex@denx.de> >> Cc: Sam Ravnborg <sam@ravnborg.org> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Cc: Maxime Ripard <mripard@kernel.org> >> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> Cc: David Airlie <airlied@linux.ie> >> Cc: Daniel Vetter <daniel@ffwll.ch> >> Cc: Shawn Guo <shawnguo@kernel.org> >> Cc: Sascha Hauer <s.hauer@pengutronix.de> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> >> Cc: Fabio Estevam <festevam@gmail.com> >> Cc: NXP Linux Team <linux-imx@nxp.com> >> Cc: linux-arm-kernel@lists.infradead.org >> To: dri-devel@lists.freedesktop.org >> --- >> drivers/gpu/drm/panel/panel-simple.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >> index 3c08f9827acf..2c683d94a3f3 100644 >> --- a/drivers/gpu/drm/panel/panel-simple.c >> +++ b/drivers/gpu/drm/panel/panel-simple.c >> @@ -453,6 +453,7 @@ static int panel_dpi_probe(struct device *dev, >> struct panel_desc *desc; >> unsigned int bus_flags; >> struct videomode vm; >> + const char *mapping; >> int ret; >> >> np = dev->of_node; >> @@ -477,6 +478,16 @@ static int panel_dpi_probe(struct device *dev, >> of_property_read_u32(np, "width-mm", &desc->size.width); >> of_property_read_u32(np, "height-mm", &desc->size.height); >> >> + of_property_read_string(np, "data-mapping", &mapping); >> + if (!strcmp(mapping, "rgb24")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; >> + else if (!strcmp(mapping, "rgb565")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; >> + else if (!strcmp(mapping, "bgr666")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; >> + else if (!strcmp(mapping, "lvds666")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; > > You're right that there's an issue, but a revert isn't the right option. > The commit you're reverting never made it in a stable release, because > it was deemed to not be a good enough option. > > First of all, any attempt to fix this should include an update to the DT > binding. Second, as this is about DPI panels, the LVDS option should be > dropped. Finally, I've shared some initial thoughts in [1], maybe you > can reply to that e-mail to continue the discussion there ? According to your thoughts in [1] you mean that the bus format should be build out of the devicetree properties bus-width and data-shift. It would be possible for evenly structured busses like RGB888_1X24 and RGB666_1X18, but what about a bus like RGB565_1X16, where each color has different bus width. Also the order of the colors should be defined to differ between busses like RGB888_1X24 and GBR888_1X24. Are there any ideas how can this be covered? > > [1] https://lore.kernel.org/all/20200303185531.GJ11333@pendragon.ideasonboard.com/ > >> + >> /* Extract bus_flags from display_timing */ >> bus_flags = 0; >> vm.flags = timing->flags; > Regards Christoph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Niedermaier <cniedermaier@dh-electronics.com> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: "Marek MV. Vasut" <marex@denx.de>, David Airlie <airlied@linux.ie>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, Sam Ravnborg <sam@ravnborg.org>, Pengutronix Kernel Team <kernel@pengutronix.de>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, NXP Linux Team <linux-imx@nxp.com> Subject: RE: [RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property" Date: Tue, 8 Feb 2022 21:27:17 +0000 [thread overview] Message-ID: <4a2f4707c03d4509aa5f3a388cbfbe04@dh-electronics.com> (raw) In-Reply-To: <YfsXt1lU6l9cSctX@pendragon.ideasonboard.com> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com] Sent: Thursday, February 3, 2022 12:46 AM > > Hi Christoph, > Hi Laurent, > On Tue, Feb 01, 2022 at 12:07:17PM +0100, Christoph Niedermaier wrote: >> Without the data-mapping devicetree property my display won't >> work properly. It is flickering, because the bus flags won't >> be assigned without a defined bus format by the imx parallel >> display driver. There was a discussion about the removal [1] >> and an agreement that a better solution is needed, but it is >> missing so far. So what would be the better approach? >> >> [1] https://patchwork.freedesktop.org/patch/357659/?series=74705&rev=1 >> >> This reverts commit d021d751c14752a0266865700f6f212fab40a18c. >> >> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >> Cc: Marek Vasut <marex@denx.de> >> Cc: Sam Ravnborg <sam@ravnborg.org> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Cc: Maxime Ripard <mripard@kernel.org> >> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> Cc: David Airlie <airlied@linux.ie> >> Cc: Daniel Vetter <daniel@ffwll.ch> >> Cc: Shawn Guo <shawnguo@kernel.org> >> Cc: Sascha Hauer <s.hauer@pengutronix.de> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> >> Cc: Fabio Estevam <festevam@gmail.com> >> Cc: NXP Linux Team <linux-imx@nxp.com> >> Cc: linux-arm-kernel@lists.infradead.org >> To: dri-devel@lists.freedesktop.org >> --- >> drivers/gpu/drm/panel/panel-simple.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >> index 3c08f9827acf..2c683d94a3f3 100644 >> --- a/drivers/gpu/drm/panel/panel-simple.c >> +++ b/drivers/gpu/drm/panel/panel-simple.c >> @@ -453,6 +453,7 @@ static int panel_dpi_probe(struct device *dev, >> struct panel_desc *desc; >> unsigned int bus_flags; >> struct videomode vm; >> + const char *mapping; >> int ret; >> >> np = dev->of_node; >> @@ -477,6 +478,16 @@ static int panel_dpi_probe(struct device *dev, >> of_property_read_u32(np, "width-mm", &desc->size.width); >> of_property_read_u32(np, "height-mm", &desc->size.height); >> >> + of_property_read_string(np, "data-mapping", &mapping); >> + if (!strcmp(mapping, "rgb24")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; >> + else if (!strcmp(mapping, "rgb565")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; >> + else if (!strcmp(mapping, "bgr666")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; >> + else if (!strcmp(mapping, "lvds666")) >> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; > > You're right that there's an issue, but a revert isn't the right option. > The commit you're reverting never made it in a stable release, because > it was deemed to not be a good enough option. > > First of all, any attempt to fix this should include an update to the DT > binding. Second, as this is about DPI panels, the LVDS option should be > dropped. Finally, I've shared some initial thoughts in [1], maybe you > can reply to that e-mail to continue the discussion there ? According to your thoughts in [1] you mean that the bus format should be build out of the devicetree properties bus-width and data-shift. It would be possible for evenly structured busses like RGB888_1X24 and RGB666_1X18, but what about a bus like RGB565_1X16, where each color has different bus width. Also the order of the colors should be defined to differ between busses like RGB888_1X24 and GBR888_1X24. Are there any ideas how can this be covered? > > [1] https://lore.kernel.org/all/20200303185531.GJ11333@pendragon.ideasonboard.com/ > >> + >> /* Extract bus_flags from display_timing */ >> bus_flags = 0; >> vm.flags = timing->flags; > Regards Christoph
next prev parent reply other threads:[~2022-02-08 21:30 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-02-01 11:07 [RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property" Christoph Niedermaier 2022-02-01 11:07 ` Christoph Niedermaier 2022-02-02 15:42 ` Denys Drozdov 2022-02-02 15:54 ` Denys Drozdov 2022-02-02 15:54 ` Denys Drozdov 2022-02-02 23:45 ` Laurent Pinchart 2022-02-02 23:45 ` Laurent Pinchart 2022-02-03 8:01 ` (EXT) " Alexander Stein 2022-02-03 8:01 ` Alexander Stein 2022-02-08 21:27 ` Christoph Niedermaier [this message] 2022-02-08 21:27 ` Christoph Niedermaier 2022-02-08 23:52 ` Marek Vasut 2022-02-08 23:52 ` Marek Vasut 2022-02-09 13:14 ` Max Krummenacher 2022-02-09 13:14 ` Max Krummenacher 2022-02-19 9:37 ` Christoph Niedermaier 2022-02-19 9:37 ` Christoph Niedermaier 2022-02-22 8:51 ` Max 2022-02-22 8:51 ` Max
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4a2f4707c03d4509aa5f3a388cbfbe04@dh-electronics.com \ --to=cniedermaier@dh-electronics.com \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=marex@denx.de \ --cc=mripard@kernel.org \ --cc=p.zabel@pengutronix.de \ --cc=s.hauer@pengutronix.de \ --cc=sam@ravnborg.org \ --cc=shawnguo@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.