diff mbox

drm: Print some debug/error info during DP dual mode detect

Message ID 1477493408-32524-1-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak Oct. 26, 2016, 2:50 p.m. UTC
There's at least one LSPCON device that occasionally returns an unexpected
adaptor ID which leads to a failed detect. Print some debug info to help
debugging this and future cases. Also print an error for an unexpected
adaptor ID, so users can report it.

Cc: dri-devel@lists.freedesktop.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_dual_mode_helper.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Ville Syrjälä Oct. 26, 2016, 3:10 p.m. UTC | #1
On Wed, Oct 26, 2016 at 05:50:08PM +0300, Imre Deak wrote:
> There's at least one LSPCON device that occasionally returns an unexpected
> adaptor ID which leads to a failed detect. Print some debug info to help
> debugging this and future cases. Also print an error for an unexpected
> adaptor ID, so users can report it.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> index 488355b..a0e603b 100644
> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -142,6 +142,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>  		      sizeof(dp_dual_mode_hdmi_id)) == 0;
>  }
>  
> +static bool is_type1_adaptor(uint8_t adaptor_id)
> +{
> +	return adaptor_id == 0 || adaptor_id == 0xff;
> +}
> +
>  static bool is_type2_adaptor(uint8_t adaptor_id)
>  {
>  	return adaptor_id == (DP_DUAL_MODE_TYPE_TYPE2 |
> @@ -193,6 +198,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>  	 */
>  	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_HDMI_ID,
>  				    hdmi_id, sizeof(hdmi_id));
> +	DRM_DEBUG_KMS("DP dual mode HDMI ID: %*pE (err %zd)\n",
> +		      ret ? 0 : (int)sizeof(hdmi_id), hdmi_id, ret);

What does that %*pE print with size==0, nothing?

>  	if (ret)
>  		return DRM_DP_DUAL_MODE_UNKNOWN;
>  
> @@ -210,6 +217,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>  	 */
>  	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
>  				    &adaptor_id, sizeof(adaptor_id));
> +	DRM_DEBUG_KMS("DP dual mode adaptor ID: %02x (err %zd)\n",
> +		      adaptor_id, ret);
>  	if (ret == 0) {
>  		if (is_lspcon_adaptor(hdmi_id, adaptor_id))
>  			return DRM_DP_DUAL_MODE_LSPCON;
> @@ -219,6 +228,10 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>  			else
>  				return DRM_DP_DUAL_MODE_TYPE2_DVI;
>  		}
> +		if (!is_type1_adaptor(adaptor_id) && adaptor_id != hdmi_id[0])
                                                     ^^^^^^^^^^^^^^^^^^^^^^^^
I take it that was to account for the broken adaptors that ignore the
offset?

> +			DRM_ERROR("Unexpected DP dual mode adapter ID %02x\n",
> +				  adaptor_id);

s/adapter/adaptor/ since that's what the spec called it, and I continued
down the same path to not confuse it the the i2c adapter.

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

> +
>  	}
>  
>  	if (is_hdmi_adaptor(hdmi_id))
> -- 
> 2.5.0
Imre Deak Oct. 26, 2016, 3:27 p.m. UTC | #2
On Wed, 2016-10-26 at 18:10 +0300, Ville Syrjälä wrote:
> On Wed, Oct 26, 2016 at 05:50:08PM +0300, Imre Deak wrote:
> > There's at least one LSPCON device that occasionally returns an unexpected
> > adaptor ID which leads to a failed detect. Print some debug info to help
> > debugging this and future cases. Also print an error for an unexpected
> > adaptor ID, so users can report it.
> > 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> > index 488355b..a0e603b 100644
> > --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> > @@ -142,6 +142,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> >  		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> >  }
> >  
> > +static bool is_type1_adaptor(uint8_t adaptor_id)
> > +{
> > +	return adaptor_id == 0 || adaptor_id == 0xff;
> > +}
> > +
> >  static bool is_type2_adaptor(uint8_t adaptor_id)
> >  {
> >  	return adaptor_id == (DP_DUAL_MODE_TYPE_TYPE2 |
> > @@ -193,6 +198,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
> >  	 */
> >  	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_HDMI_ID,
> >  				    hdmi_id, sizeof(hdmi_id));
> > +	DRM_DEBUG_KMS("DP dual mode HDMI ID: %*pE (err %zd)\n",
> > +		      ret ? 0 : (int)sizeof(hdmi_id), hdmi_id, ret);
> 
> What does that %*pE print with size==0, nothing?

Yes.

> >  	if (ret)
> >  		return DRM_DP_DUAL_MODE_UNKNOWN;
> >  
> > @@ -210,6 +217,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
> >  	 */
> >  	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
> >  				    &adaptor_id, sizeof(adaptor_id));
> > +	DRM_DEBUG_KMS("DP dual mode adaptor ID: %02x (err %zd)\n",
> > +		      adaptor_id, ret);
> >  	if (ret == 0) {
> >  		if (is_lspcon_adaptor(hdmi_id, adaptor_id))
> >  			return DRM_DP_DUAL_MODE_LSPCON;
> > @@ -219,6 +228,10 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
> >  			else
> >  				return DRM_DP_DUAL_MODE_TYPE2_DVI;
> >  		}
> > +		if (!is_type1_adaptor(adaptor_id) && adaptor_id != hdmi_id[0])
>                                                      ^^^^^^^^^^^^^^^^^^^^^^^^
> I take it that was to account for the broken adaptors that ignore the
> offset?

Yep.

> > +			DRM_ERROR("Unexpected DP dual mode adapter ID %02x\n",
> > +				  adaptor_id);
> 
> s/adapter/adaptor/ since that's what the spec called it, and I continued
> down the same path to not confuse it the the i2c adapter.

Ok.

> With that
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +
> >  	}
> >  
> >  	if (is_hdmi_adaptor(hdmi_id))
> > -- 
> > 2.5.0
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index 488355b..a0e603b 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -142,6 +142,11 @@  static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
 		      sizeof(dp_dual_mode_hdmi_id)) == 0;
 }
 
+static bool is_type1_adaptor(uint8_t adaptor_id)
+{
+	return adaptor_id == 0 || adaptor_id == 0xff;
+}
+
 static bool is_type2_adaptor(uint8_t adaptor_id)
 {
 	return adaptor_id == (DP_DUAL_MODE_TYPE_TYPE2 |
@@ -193,6 +198,8 @@  enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
 	 */
 	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_HDMI_ID,
 				    hdmi_id, sizeof(hdmi_id));
+	DRM_DEBUG_KMS("DP dual mode HDMI ID: %*pE (err %zd)\n",
+		      ret ? 0 : (int)sizeof(hdmi_id), hdmi_id, ret);
 	if (ret)
 		return DRM_DP_DUAL_MODE_UNKNOWN;
 
@@ -210,6 +217,8 @@  enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
 	 */
 	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
 				    &adaptor_id, sizeof(adaptor_id));
+	DRM_DEBUG_KMS("DP dual mode adaptor ID: %02x (err %zd)\n",
+		      adaptor_id, ret);
 	if (ret == 0) {
 		if (is_lspcon_adaptor(hdmi_id, adaptor_id))
 			return DRM_DP_DUAL_MODE_LSPCON;
@@ -219,6 +228,10 @@  enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
 			else
 				return DRM_DP_DUAL_MODE_TYPE2_DVI;
 		}
+		if (!is_type1_adaptor(adaptor_id) && adaptor_id != hdmi_id[0])
+			DRM_ERROR("Unexpected DP dual mode adapter ID %02x\n",
+				  adaptor_id);
+
 	}
 
 	if (is_hdmi_adaptor(hdmi_id))