Re: [PATCH] drm/i915: Fix AVI/HDMI/SPD infoframes on HSW+

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Dec 16, 2015 at 06:10:00PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I broke AVI/HDMI/SPD infoframes on HSW+ with the register type
> safety changes. We were supposed to check that the infoframe data
> register is valid before writing the infoframe data, but the check
> ended up inverted, and so in practice we never wrote or enabled
> these infoframes.
> 
> We were still sending out the GCP infoframe when the sink was
> deep-color capable. That and the fact that we use a single
> bool to track our infoframe state meant that the state checker
> only caught this when a HDMI sink that doesn't do deep-color was
> used.
> 
> We really need to fix our infoframe state checking to be much
> more anal. But in the meantime let's just fix the regression.
> In fact let's just throw out the register validity check and
> convert some of the "unknown info frame type" debug messages
> into MISSING_CASE(). So far we support the same set of infoframe
> types on all platforms, so the silent debug messages make no
> sense.
> 
> Fixes: f0f59a00a1c9 ("drm/i915: Type safe register read/write")
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (irc)
> Tested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (irc)
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

And of course I forgot the bugzilla link
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93119

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 682554385631..7fae62626b71 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -78,7 +78,7 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>  	case HDMI_INFOFRAME_TYPE_VENDOR:
>  		return VIDEO_DIP_SELECT_VENDOR;
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
> +		MISSING_CASE(type);
>  		return 0;
>  	}
>  }
> @@ -93,7 +93,7 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
>  	case HDMI_INFOFRAME_TYPE_VENDOR:
>  		return VIDEO_DIP_ENABLE_VENDOR;
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
> +		MISSING_CASE(type);
>  		return 0;
>  	}
>  }
> @@ -108,7 +108,7 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
>  	case HDMI_INFOFRAME_TYPE_VENDOR:
>  		return VIDEO_DIP_ENABLE_VS_HSW;
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
> +		MISSING_CASE(type);
>  		return 0;
>  	}
>  }
> @@ -127,7 +127,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>  	case HDMI_INFOFRAME_TYPE_VENDOR:
>  		return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i);
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
> +		MISSING_CASE(type);
>  		return INVALID_MMIO_REG;
>  	}
>  }
> @@ -375,8 +375,6 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>  	u32 val = I915_READ(ctl_reg);
>  
>  	data_reg = hsw_dip_data_reg(dev_priv, cpu_transcoder, type, 0);
> -	if (i915_mmio_reg_valid(data_reg))
> -		return;
>  
>  	val &= ~hsw_infoframe_enable(type);
>  	I915_WRITE(ctl_reg, val);
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx