diff mbox

[4/4] drm/dp/mst: Check peer device type before attempting EDID read

Message ID 1477472755-15288-5-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 26, 2016, 9:05 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Only certain types of pdts have the DDC bus registered, so check for
that before we attempt the EDID read. Othwewise we risk playing around
with an i2c adapter that doesn't actually exist.

Cc: stable@vger.kernel.org
Cc: Carlos Santa <carlos.santa@intel.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Tested-by: Carlos Santa <carlos.santa@intel.com>
Tested-by: Kirill A. Shutemov <kirill@shutemov.name>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Oct. 26, 2016, 12:55 p.m. UTC | #1 
On Wed, Oct 26, 2016 at 12:05:55PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Only certain types of pdts have the DDC bus registered, so check for
> that before we attempt the EDID read. Othwewise we risk playing around
> with an i2c adapter that doesn't actually exist.
> 
> Cc: stable@vger.kernel.org
> Cc: Carlos Santa <carlos.santa@intel.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Tested-by: Carlos Santa <carlos.santa@intel.com>
> Tested-by: Kirill A. Shutemov <kirill@shutemov.name>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 956babc161e5..690d1b407a90 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>  			drm_dp_put_port(port);
>  			goto out;
>  		}
> -		if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> +		if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
> +		     port->pdt == DP_PEER_DEVICE_SST_SINK) &&
> +		    port->port_num >= DP_MST_LOGICAL_PORT_0) {

Matches what's in drm_dp_port_setup_pdt. Not sure this is the most
reliable way to do this though, but I can't come up with anything better.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  			port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
>  			drm_mode_connector_set_tile_property(port->connector);
>  		}
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjälä Oct. 26, 2016, 1:05 p.m. UTC | #2 
On Wed, Oct 26, 2016 at 02:55:14PM +0200, Daniel Vetter wrote:
> On Wed, Oct 26, 2016 at 12:05:55PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Only certain types of pdts have the DDC bus registered, so check for
> > that before we attempt the EDID read. Othwewise we risk playing around
> > with an i2c adapter that doesn't actually exist.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Carlos Santa <carlos.santa@intel.com>
> > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > Tested-by: Carlos Santa <carlos.santa@intel.com>
> > Tested-by: Kirill A. Shutemov <kirill@shutemov.name>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 956babc161e5..690d1b407a90 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
> >  			drm_dp_put_port(port);
> >  			goto out;
> >  		}
> > -		if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> > +		if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
> > +		     port->pdt == DP_PEER_DEVICE_SST_SINK) &&
> > +		    port->port_num >= DP_MST_LOGICAL_PORT_0) {
> 
> Matches what's in drm_dp_port_setup_pdt. Not sure this is the most
> reliable way to do this though, but I can't come up with anything better.

Yeah. I just went with whatever looks semi-correct based on where the
thing is set up. I'd have to go read the actual spec to figure out if
this makes any real sense. But at least it avoids the explosion.

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> >  			port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
> >  			drm_mode_connector_set_tile_property(port->connector);
> >  		}
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Santa, Carlos Oct. 26, 2016, 6:31 p.m. UTC | #3 
On Wed, 2016-10-26 at 02:05 -0700, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Only certain types of pdts have the DDC bus registered, so check for
> that before we attempt the EDID read. Othwewise we risk playing 

typo above

> around
> with an i2c adapter that doesn't actually exist.
> 

Would it be worth pasting the stack trace of the crash??

Carlos


> Cc: stable@vger.kernel.org
> Cc: Carlos Santa <carlos.santa@intel.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Tested-by: Carlos Santa <carlos.santa@intel.com>
> Tested-by: Kirill A. Shutemov <kirill@shutemov.name>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 956babc161e5..690d1b407a90 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct
> drm_dp_mst_branch *mstb,
>  			drm_dp_put_port(port);
>  			goto out;
>  		}
> -		if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> +		if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
> +		     port->pdt == DP_PEER_DEVICE_SST_SINK) &&
> +		    port->port_num >= DP_MST_LOGICAL_PORT_0) {
>  			port->cached_edid = drm_get_edid(port-
> >connector, &port->aux.ddc);
>  			drm_mode_connector_set_tile_property(port-
> >connector);
>  		}
> -- 
> 2.7.4
>
Ville Syrjälä Oct. 26, 2016, 6:45 p.m. UTC | #4 
On Wed, Oct 26, 2016 at 11:31:55AM -0700, Carlos Santa wrote:
> On Wed, 2016-10-26 at 02:05 -0700, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Only certain types of pdts have the DDC bus registered, so check for
> > that before we attempt the EDID read. Othwewise we risk playing 
> 
> typo above
> 
> > around
> > with an i2c adapter that doesn't actually exist.
> > 
> 
> Would it be worth pasting the stack trace of the crash??

Can't hurt.

> 
> Carlos
> 
> 
> > Cc: stable@vger.kernel.org
> > Cc: Carlos Santa <carlos.santa@intel.com>
> > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > Tested-by: Carlos Santa <carlos.santa@intel.com>
> > Tested-by: Kirill A. Shutemov <kirill@shutemov.name>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 956babc161e5..690d1b407a90 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct
> > drm_dp_mst_branch *mstb,
> >  			drm_dp_put_port(port);
> >  			goto out;
> >  		}
> > -		if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> > +		if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
> > +		     port->pdt == DP_PEER_DEVICE_SST_SINK) &&
> > +		    port->port_num >= DP_MST_LOGICAL_PORT_0) {
> >  			port->cached_edid = drm_get_edid(port-
> > >connector, &port->aux.ddc);
> >  			drm_mode_connector_set_tile_property(port-
> > >connector);
> >  		}
> > -- 
> > 2.7.4
> >
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 956babc161e5..690d1b407a90 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1162,7 +1162,9 @@  static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 			drm_dp_put_port(port);
 			goto out;
 		}
-		if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
+		if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
+		     port->pdt == DP_PEER_DEVICE_SST_SINK) &&
+		    port->port_num >= DP_MST_LOGICAL_PORT_0) {
 			port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
 			drm_mode_connector_set_tile_property(port->connector);
 		}