All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Niedermaier <cniedermaier@dh-electronics.com>
To: Max Krummenacher <max.oss.09@gmail.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Cc: "Marek MV. Vasut" <marex@denx.de>,
	David Airlie <airlied@linux.ie>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()
Date: Sat, 19 Feb 2022 09:28:44 +0000	[thread overview]
Message-ID: <e00e00585b9945e396bdc3bbd8f30f7e@dh-electronics.com> (raw)
In-Reply-To: <36f517300a15b2460a02512ef3c5814ad0ed2290.camel@gmail.com>

From: Max Krummenacher [mailto:max.oss.09@gmail.com]
Sent: Wednesday, February 9, 2022 10:38 AM
>> If display timings were read from the devicetree using
>> of_get_display_timing() and pixelclk-active is defined
>> there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is
>> automatically generated. Through the function
>> drm_bus_flags_from_videomode() e.g. called in the
>> panel-simple driver this flag got into the bus flags,
>> but then in imx_pd_bridge_atomic_check() the bus flag
>> check failed and will not initialize the display. The
>> original commit fe141cedc433 does not explain why this
>> check was introduced. So remove the bus flags check,
>> because it stops the initialization of the display with
>> valid bus flags.
>>
>> Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the bridge when available")
>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> 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
>> ---
>> V2: - Add Boris to the Cc list
>> ---
>>  drivers/gpu/drm/imx/parallel-display.c | 8 --------
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
>> index a8aba0141ce7..06cb1a59b9bc 100644
>> --- a/drivers/gpu/drm/imx/parallel-display.c
>> +++ b/drivers/gpu/drm/imx/parallel-display.c
>> @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct drm_bridge *bridge,
>>       if (!imx_pd_format_supported(bus_fmt))
>>               return -EINVAL;
>>
>> -     if (bus_flags &
>> -         ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH |
>> -           DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
>> -           DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) {
>> -             dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", bus_flags);
>> -             return -EINVAL;
>> -     }
>> -
>>       bridge_state->output_bus_cfg.flags = bus_flags;
>>       bridge_state->input_bus_cfg.flags = bus_flags;
>>       imx_crtc_state->bus_flags = bus_flags;
> 
> Tested on a Colibri iMX6DL with a panel-dpi based panel.
> 
> Tested-by: Max Krummenacher <max.krummenacher@toradex.com>

I still ask myself why this bus flag check is in the code.
Is there a reason not to remove that?

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: Max Krummenacher <max.oss.09@gmail.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Cc: "Marek MV. Vasut" <marex@denx.de>,
	David Airlie <airlied@linux.ie>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()
Date: Sat, 19 Feb 2022 09:28:44 +0000	[thread overview]
Message-ID: <e00e00585b9945e396bdc3bbd8f30f7e@dh-electronics.com> (raw)
In-Reply-To: <36f517300a15b2460a02512ef3c5814ad0ed2290.camel@gmail.com>

From: Max Krummenacher [mailto:max.oss.09@gmail.com]
Sent: Wednesday, February 9, 2022 10:38 AM
>> If display timings were read from the devicetree using
>> of_get_display_timing() and pixelclk-active is defined
>> there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is
>> automatically generated. Through the function
>> drm_bus_flags_from_videomode() e.g. called in the
>> panel-simple driver this flag got into the bus flags,
>> but then in imx_pd_bridge_atomic_check() the bus flag
>> check failed and will not initialize the display. The
>> original commit fe141cedc433 does not explain why this
>> check was introduced. So remove the bus flags check,
>> because it stops the initialization of the display with
>> valid bus flags.
>>
>> Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the bridge when available")
>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> 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
>> ---
>> V2: - Add Boris to the Cc list
>> ---
>>  drivers/gpu/drm/imx/parallel-display.c | 8 --------
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
>> index a8aba0141ce7..06cb1a59b9bc 100644
>> --- a/drivers/gpu/drm/imx/parallel-display.c
>> +++ b/drivers/gpu/drm/imx/parallel-display.c
>> @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct drm_bridge *bridge,
>>       if (!imx_pd_format_supported(bus_fmt))
>>               return -EINVAL;
>>
>> -     if (bus_flags &
>> -         ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH |
>> -           DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
>> -           DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) {
>> -             dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", bus_flags);
>> -             return -EINVAL;
>> -     }
>> -
>>       bridge_state->output_bus_cfg.flags = bus_flags;
>>       bridge_state->input_bus_cfg.flags = bus_flags;
>>       imx_crtc_state->bus_flags = bus_flags;
> 
> Tested on a Colibri iMX6DL with a panel-dpi based panel.
> 
> Tested-by: Max Krummenacher <max.krummenacher@toradex.com>

I still ask myself why this bus flag check is in the code.
Is there a reason not to remove that?

Regards
Christoph

  reply	other threads:[~2022-02-19  9:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-01 11:36 [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check() Christoph Niedermaier
2022-02-01 11:36 ` Christoph Niedermaier
2022-02-09  9:38 ` Max Krummenacher
2022-02-09  9:38   ` Max Krummenacher
2022-02-19  9:28   ` Christoph Niedermaier [this message]
2022-02-19  9:28     ` Christoph Niedermaier
2022-02-21  8:29     ` Boris Brezillon
2022-02-21  8:29       ` Boris Brezillon
2022-02-21 11:56       ` Max Krummenacher
2022-02-21 11:56         ` Max Krummenacher
2022-02-21 12:31         ` Boris Brezillon
2022-02-28  1:04           ` Marek Vasut
2022-02-28  1:04             ` Marek Vasut
2022-02-28  1:10             ` Marek Vasut
2022-02-28  1:10               ` Marek Vasut

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=e00e00585b9945e396bdc3bbd8f30f7e@dh-electronics.com \
    --to=cniedermaier@dh-electronics.com \
    --cc=airlied@linux.ie \
    --cc=boris.brezillon@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=marex@denx.de \
    --cc=max.oss.09@gmail.com \
    --cc=s.hauer@pengutronix.de \
    --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: link
Be 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.