Re: [PATCH v6 21/28] drm/i915/dp: Use the existing write_infoframe() for DSC PPS SDPs

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

On Wed, Oct 24, 2018 at 03:28:33PM -0700, Manasi Navare wrote:
> Infoframes are used to send secondary data packets. This patch
> adds support for DSC Picture parameter set secondary data packets
> in the existing write_infoframe helpers.
> 
> v2:
> * Rebase on drm-tip (Manasi)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h   |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c | 23 +++++++++++++++++++++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 64cca0a83cf7..0ecdc95f56d8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4545,6 +4545,7 @@ enum {
>   * of the infoframe structure specified by CEA-861. */
>  #define   VIDEO_DIP_DATA_SIZE	32
>  #define   VIDEO_DIP_VSC_DATA_SIZE	36
> +#define   VIDEO_DIP_PPS_DATA_SIZE	132
>  #define VIDEO_DIP_CTL		_MMIO(0x61170)
>  /* Pre HSW: */
>  #define   VIDEO_DIP_ENABLE		(1 << 31)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index d3e653640ce7..02fb54737d92 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -115,6 +115,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
>  	switch (type) {
>  	case DP_SDP_VSC:
>  		return VIDEO_DIP_ENABLE_VSC_HSW;
> +	case DP_SDP_PPS:
> +		return VDIP_ENABLE_PPS;

Hmm. Why is that bit named so differently to the rest?

>  	case HDMI_INFOFRAME_TYPE_AVI:
>  		return VIDEO_DIP_ENABLE_AVI_HSW;
>  	case HDMI_INFOFRAME_TYPE_SPD:
> @@ -136,6 +138,8 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>  	switch (type) {
>  	case DP_SDP_VSC:
>  		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
> +	case DP_SDP_PPS:
> +		return ICL_VIDEO_DIP_PPS_DATA(cpu_transcoder, i);
>  	case HDMI_INFOFRAME_TYPE_AVI:
>  		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
>  	case HDMI_INFOFRAME_TYPE_SPD:
> @@ -148,6 +152,18 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +static int hsw_dip_data_size(unsigned int type)
> +{
> +	switch (type) {
> +	case DP_SDP_VSC:
> +		return VIDEO_DIP_VSC_DATA_SIZE;
> +	case DP_SDP_PPS:
> +		return VIDEO_DIP_PPS_DATA_SIZE;
> +	default:
> +		return VIDEO_DIP_DATA_SIZE;
> +	}
> +}
> +
>  static void g4x_write_infoframe(struct intel_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
>  				unsigned int type,
> @@ -382,11 +398,14 @@ static void hsw_write_infoframe(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
> -	int data_size = type == DP_SDP_VSC ?
> -		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
> +	i915_reg_t data_reg;
> +	int data_size = 0;

=0 is unnecessary.

>  	int i;
>  	u32 val = I915_READ(ctl_reg);
>  
> +	data_size = hsw_dip_data_size(type);
> +	data_reg = hsw_dip_data_reg(dev_priv, cpu_transcoder, type, 0);

data_reg is unused.

> +
>  	val &= ~hsw_infoframe_enable(type);
>  	I915_WRITE(ctl_reg, val);
>  
> -- 
> 2.18.0

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