[PATCH v3 1/1] drm/mediatek: Add return value check when reading DPCD

[PATCH v3 1/1] drm/mediatek: Add return value check when reading DPCD

[Liankun Yang]

Returns the number of bytes transferred (1) on success.
Check the return value to confirm that AUX communication is successful.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")

Signed-off-by: Liankun Yang <liankun.yang@mediatek.com>
---
Changes in V3:
- Using drm_dp_read_sink_count() to improve patch.
Per suggestion from the previous thread:
https://patchwork.kernel.org/project/linux-mediatek/patch/20241128030940.25657-1-liankun.yang@mediatek.com/

Changes in V2:
- Modify Fixes in Commit Message.
Per suggestion from the previous thread:
https://patchwork.kernel.org/project/linux-mediatek/patch/20240930092000.5385-1-liankun.yang@mediatek.com/
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 36713c176cfc..b26dad8783be 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2100,7 +2100,6 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
 	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
 	enum drm_connector_status ret = connector_status_disconnected;
 	bool enabled = mtk_dp->enabled;
-	u8 sink_count = 0;
 
 	if (!mtk_dp->train_info.cable_plugged_in)
 		return ret;
@@ -2115,8 +2114,8 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
 	 * function, we just need to check the HPD connection to check
 	 * whether we connect to a sink device.
 	 */
-	drm_dp_dpcd_readb(&mtk_dp->aux, DP_SINK_COUNT, &sink_count);
-	if (DP_GET_SINK_COUNT(sink_count))
+
+	if (drm_dp_read_sink_count(&mtk_dp->aux))
 		ret = connector_status_connected;
 
 	if (!enabled)
-- 
2.45.2

Re: [PATCH v3 1/1] drm/mediatek: Add return value check when reading DPCD

[Guillaume Ranquet]

On Wed, 18 Dec 2024 12:34, Liankun Yang <liankun.yang@mediatek.com> wrote:
>Returns the number of bytes transferred (1) on success.
Hi Liankun,

I'm not sure this statement above is true, you are not returning the
number of transferred bytes?


Aside of that little nitpick

Reviewed-by: Guillaume Ranquet <granquet@baylibre.com>

>Check the return value to confirm that AUX communication is successful.
>
>Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
>
>Signed-off-by: Liankun Yang <liankun.yang@mediatek.com>
>---
>Changes in V3:
>- Using drm_dp_read_sink_count() to improve patch.
>Per suggestion from the previous thread:
>https://patchwork.kernel.org/project/linux-mediatek/patch/20241128030940.25657-1-liankun.yang@mediatek.com/
>
>Changes in V2:
>- Modify Fixes in Commit Message.
>Per suggestion from the previous thread:
>https://patchwork.kernel.org/project/linux-mediatek/patch/20240930092000.5385-1-liankun.yang@mediatek.com/
>---
> drivers/gpu/drm/mediatek/mtk_dp.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
>index 36713c176cfc..b26dad8783be 100644
>--- a/drivers/gpu/drm/mediatek/mtk_dp.c
>+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
>@@ -2100,7 +2100,6 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
> 	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> 	enum drm_connector_status ret = connector_status_disconnected;
> 	bool enabled = mtk_dp->enabled;
>-	u8 sink_count = 0;
>
> 	if (!mtk_dp->train_info.cable_plugged_in)
> 		return ret;
>@@ -2115,8 +2114,8 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
> 	 * function, we just need to check the HPD connection to check
> 	 * whether we connect to a sink device.
> 	 */
>-	drm_dp_dpcd_readb(&mtk_dp->aux, DP_SINK_COUNT, &sink_count);
>-	if (DP_GET_SINK_COUNT(sink_count))
>+
>+	if (drm_dp_read_sink_count(&mtk_dp->aux))
> 		ret = connector_status_connected;
>
> 	if (!enabled)
>--
>2.45.2

Re: [PATCH v3 1/1] drm/mediatek: Add return value check when reading DPCD

[Chun-Kuang Hu]

Hi, Liankun:

Liankun Yang <liankun.yang@mediatek.com> 於 2024年12月18日 週三 下午7:35寫道：
>
> Returns the number of bytes transferred (1) on success.
> Check the return value to confirm that AUX communication is successful.
>
> Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
>
> Signed-off-by: Liankun Yang <liankun.yang@mediatek.com>
> ---
> Changes in V3:
> - Using drm_dp_read_sink_count() to improve patch.
> Per suggestion from the previous thread:
> https://patchwork.kernel.org/project/linux-mediatek/patch/20241128030940.25657-1-liankun.yang@mediatek.com/
>
> Changes in V2:
> - Modify Fixes in Commit Message.
> Per suggestion from the previous thread:
> https://patchwork.kernel.org/project/linux-mediatek/patch/20240930092000.5385-1-liankun.yang@mediatek.com/
> ---
>  drivers/gpu/drm/mediatek/mtk_dp.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 36713c176cfc..b26dad8783be 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -2100,7 +2100,6 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
>         struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
>         enum drm_connector_status ret = connector_status_disconnected;
>         bool enabled = mtk_dp->enabled;
> -       u8 sink_count = 0;
>
>         if (!mtk_dp->train_info.cable_plugged_in)
>                 return ret;
> @@ -2115,8 +2114,8 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
>          * function, we just need to check the HPD connection to check
>          * whether we connect to a sink device.
>          */
> -       drm_dp_dpcd_readb(&mtk_dp->aux, DP_SINK_COUNT, &sink_count);
> -       if (DP_GET_SINK_COUNT(sink_count))
> +
> +       if (drm_dp_read_sink_count(&mtk_dp->aux))

if (drm_dp_read_sink_count(&mtk_dp->aux) > 0)

With this fix-up, applied to mediatek-drm-fixes [1], thanks.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-fixes

Regards,
Chun-Kuang.

>                 ret = connector_status_connected;
>
>         if (!enabled)
> --
> 2.45.2
>

Re: [PATCH v3 1/1] drm/mediatek: Add return value check when reading DPCD

[LIANKUN YANG (杨连坤)]

On Thu, 2024-12-19 at 03:51 -0600, Guillaume Ranquet wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Wed, 18 Dec 2024 12:34, Liankun Yang <liankun.yang@mediatek.com>
> wrote:
> > Returns the number of bytes transferred (1) on success.
> 
> Hi Liankun,
> 
> I'm not sure this statement above is true, you are not returning the
> number of transferred bytes?
> 
> 
> Aside of that little nitpick
> 
> Reviewed-by: Guillaume Ranquet <granquet@baylibre.com>
> 
Hi Guillaume,

1. DRM_DP_READ_SINK_COUNT modification is true, returning the current
number of real connection devices.
2. SOME DONGLES Still Source HPD When they do not connected to any sink
device. To avoid this, we need to read the sink count to make good to
sink devices.Therefore, the current judgment of the number of connected
devices is just enough.

Best Regards,
Liankun Yang

> > Check the return value to confirm that AUX communication is
> > successful.
> > 
> > Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort
> > driver")
> > 
> > Signed-off-by: Liankun Yang <liankun.yang@mediatek.com>
> > ---
> > Changes in V3:
> > - Using drm_dp_read_sink_count() to improve patch.
> > Per suggestion from the previous thread:
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20241128030940.25657-1-liankun.yang@mediatek.com/__;!!CTRNKA9wMg0ARbw!hB4quDn5mHgRtBK-Gf2pmkMN_4MbA66eggh1o8gdUWP7pA72uPoZlmbisRetswwyYSenG2ErIZK6oOaQqdCxdx4$
> > 
> > Changes in V2:
> > - Modify Fixes in Commit Message.
> > Per suggestion from the previous thread:
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240930092000.5385-1-liankun.yang@mediatek.com/__;!!CTRNKA9wMg0ARbw!hB4quDn5mHgRtBK-Gf2pmkMN_4MbA66eggh1o8gdUWP7pA72uPoZlmbisRetswwyYSenG2ErIZK6oOaQWWaDClg$
> > ---
> > drivers/gpu/drm/mediatek/mtk_dp.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > index 36713c176cfc..b26dad8783be 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -2100,7 +2100,6 @@ static enum drm_connector_status
> > mtk_dp_bdg_detect(struct drm_bridge *bridge)
> >       struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> >       enum drm_connector_status ret =
> > connector_status_disconnected;
> >       bool enabled = mtk_dp->enabled;
> > -      u8 sink_count = 0;
> > 
> >       if (!mtk_dp->train_info.cable_plugged_in)
> >               return ret;
> > @@ -2115,8 +2114,8 @@ static enum drm_connector_status
> > mtk_dp_bdg_detect(struct drm_bridge *bridge)
> >        * function, we just need to check the HPD connection to
> > check
> >        * whether we connect to a sink device.
> >        */
> > -      drm_dp_dpcd_readb(&mtk_dp->aux, DP_SINK_COUNT, &sink_count);
> > -      if (DP_GET_SINK_COUNT(sink_count))
> > +
> > +      if (drm_dp_read_sink_count(&mtk_dp->aux))
> >               ret = connector_status_connected;
> > 
> >       if (!enabled)
> > --
> > 2.45.2