linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/dp: Handle zeroed port counts in drm_dp_read_downstream_info()
@ 2021-04-30 22:34 Lyude Paul
  2021-04-30 22:34 ` [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() " Lyude Paul
  2021-05-03  3:13 ` [PATCH 1/2] drm/dp: Handle zeroed port counts " Ville Syrjälä
  0 siblings, 2 replies; 5+ messages in thread
From: Lyude Paul @ 2021-04-30 22:34 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: Jérôme de Bretagne, stable, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Sean Paul, Jani Nikula, open list

While the DP specification isn't entirely clear on if this should be
allowed or not, some branch devices report having downstream ports present
while also reporting a downstream port count of 0. So to avoid breaking
those devices, we need to handle this in drm_dp_read_downstream_info().

So, to do this we assume there's no downstream port info when the
downstream port count is 0.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Tested-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/3416
Fixes: 3d3721ccb18a ("drm/i915/dp: Extract drm_dp_read_downstream_info()")
Cc: <stable@vger.kernel.org> # v5.10+
---
 drivers/gpu/drm/drm_dp_helper.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index cb56d74e9d38..27c8c5bdf7d9 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -682,7 +682,14 @@ int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
 	    !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
 		return 0;
 
+	/* Some branches advertise having 0 downstream ports, despite also advertising they have a
+	 * downstream port present. The DP spec isn't clear on if this is allowed or not, but since
+	 * some branches do it we need to handle it regardless.
+	 */
 	len = drm_dp_downstream_port_count(dpcd);
+	if (!len)
+		return 0;
+
 	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)
 		len *= 4;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() in drm_dp_read_downstream_info()
  2021-04-30 22:34 [PATCH 1/2] drm/dp: Handle zeroed port counts in drm_dp_read_downstream_info() Lyude Paul
@ 2021-04-30 22:34 ` Lyude Paul
  2021-05-03  3:06   ` [Intel-gfx] " Ville Syrjälä
  2021-05-03  3:13 ` [PATCH 1/2] drm/dp: Handle zeroed port counts " Ville Syrjälä
  1 sibling, 1 reply; 5+ messages in thread
From: Lyude Paul @ 2021-04-30 22:34 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, open list

Noticed this while fixing another issue in drm_dp_read_downstream_info(),
the open coded DP_DOWNSTREAMPORT_PRESENT check here just duplicates what we
already do in drm_dp_is_branch(), so just get rid of it.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 27c8c5bdf7d9..0f84df8798ab 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -677,9 +677,7 @@ int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
 	memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
 
 	/* No downstream info to read */
-	if (!drm_dp_is_branch(dpcd) ||
-	    dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
-	    !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+	if (!drm_dp_is_branch(dpcd) || dpcd[DP_DPCD_REV] < DP_DPCD_REV_10)
 		return 0;
 
 	/* Some branches advertise having 0 downstream ports, despite also advertising they have a
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() in drm_dp_read_downstream_info()
  2021-04-30 22:34 ` [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() " Lyude Paul
@ 2021-05-03  3:06   ` Ville Syrjälä
  2021-05-07 21:28     ` Lyude Paul
  0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2021-05-03  3:06 UTC (permalink / raw)
  To: Lyude Paul
  Cc: intel-gfx, dri-devel, Thomas Zimmermann, David Airlie, open list,
	Maxime Ripard

On Fri, Apr 30, 2021 at 06:34:28PM -0400, Lyude Paul wrote:
> Noticed this while fixing another issue in drm_dp_read_downstream_info(),
> the open coded DP_DOWNSTREAMPORT_PRESENT check here just duplicates what we
> already do in drm_dp_is_branch(), so just get rid of it.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 27c8c5bdf7d9..0f84df8798ab 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -677,9 +677,7 @@ int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
>  	memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
>  
>  	/* No downstream info to read */
> -	if (!drm_dp_is_branch(dpcd) ||
> -	    dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
> -	    !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> +	if (!drm_dp_is_branch(dpcd) || dpcd[DP_DPCD_REV] < DP_DPCD_REV_10)

BTW that DPCD_REV check looks rather wrong.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		return 0;
>  
>  	/* Some branches advertise having 0 downstream ports, despite also advertising they have a
> -- 
> 2.30.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/dp: Handle zeroed port counts in drm_dp_read_downstream_info()
  2021-04-30 22:34 [PATCH 1/2] drm/dp: Handle zeroed port counts in drm_dp_read_downstream_info() Lyude Paul
  2021-04-30 22:34 ` [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() " Lyude Paul
@ 2021-05-03  3:13 ` Ville Syrjälä
  1 sibling, 0 replies; 5+ messages in thread
From: Ville Syrjälä @ 2021-05-03  3:13 UTC (permalink / raw)
  To: Lyude Paul
  Cc: intel-gfx, dri-devel, Jérôme de Bretagne, David Airlie,
	open list, Jani Nikula, stable, Thomas Zimmermann, Sean Paul

On Fri, Apr 30, 2021 at 06:34:27PM -0400, Lyude Paul wrote:
> While the DP specification isn't entirely clear on if this should be
> allowed or not, some branch devices report having downstream ports present
> while also reporting a downstream port count of 0. So to avoid breaking
> those devices, we need to handle this in drm_dp_read_downstream_info().
> 
> So, to do this we assume there's no downstream port info when the
> downstream port count is 0.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Tested-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
> Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/3416
> Fixes: 3d3721ccb18a ("drm/i915/dp: Extract drm_dp_read_downstream_info()")
> Cc: <stable@vger.kernel.org> # v5.10+
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index cb56d74e9d38..27c8c5bdf7d9 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -682,7 +682,14 @@ int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
>  	    !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
>  		return 0;
>  
> +	/* Some branches advertise having 0 downstream ports, despite also advertising they have a
> +	 * downstream port present. The DP spec isn't clear on if this is allowed or not, but since
> +	 * some branches do it we need to handle it regardless.
> +	 */
>  	len = drm_dp_downstream_port_count(dpcd);
> +	if (!len)
> +		return 0;
> +

Seems sane enough.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)
>  		len *= 4;
>  
> -- 
> 2.30.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() in drm_dp_read_downstream_info()
  2021-05-03  3:06   ` [Intel-gfx] " Ville Syrjälä
@ 2021-05-07 21:28     ` Lyude Paul
  0 siblings, 0 replies; 5+ messages in thread
From: Lyude Paul @ 2021-05-07 21:28 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, dri-devel, Thomas Zimmermann, David Airlie, open list,
	Maxime Ripard

On Mon, 2021-05-03 at 06:06 +0300, Ville Syrjälä wrote:
> On Fri, Apr 30, 2021 at 06:34:28PM -0400, Lyude Paul wrote:
> > Noticed this while fixing another issue in drm_dp_read_downstream_info(),
> > the open coded DP_DOWNSTREAMPORT_PRESENT check here just duplicates what
> > we
> > already do in drm_dp_is_branch(), so just get rid of it.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index 27c8c5bdf7d9..0f84df8798ab 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -677,9 +677,7 @@ int drm_dp_read_downstream_info(struct drm_dp_aux
> > *aux,
> >         memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
> >  
> >         /* No downstream info to read */
> > -       if (!drm_dp_is_branch(dpcd) ||
> > -           dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
> > -           !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> > +       if (!drm_dp_is_branch(dpcd) || dpcd[DP_DPCD_REV] < DP_DPCD_REV_10)
> 
> BTW that DPCD_REV check looks rather wrong.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'll send out a separate fix for this in just a moment, thanks for pointing it
out!

> 
> >                 return 0;
> >  
> >         /* Some branches advertise having 0 downstream ports, despite also
> > advertising they have a
> > -- 
> > 2.30.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-05-07 21:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 22:34 [PATCH 1/2] drm/dp: Handle zeroed port counts in drm_dp_read_downstream_info() Lyude Paul
2021-04-30 22:34 ` [PATCH 2/2] drm/dp: Drop open-coded drm_dp_is_branch() " Lyude Paul
2021-05-03  3:06   ` [Intel-gfx] " Ville Syrjälä
2021-05-07 21:28     ` Lyude Paul
2021-05-03  3:13 ` [PATCH 1/2] drm/dp: Handle zeroed port counts " Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).