diff mbox series

[RFC,11/13] drm/i915: Read DSC capabilities of the HDMI2.1 PCON encoder

Message ID 20201015105259.27934-12-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show
Series Add support for DP-HDMI2.1 PCON | expand

Commit Message

Nautiyal, Ankit K Oct. 15, 2020, 10:52 a.m. UTC
This patch adds a helper function to read the DSC capabilities of the
HDMI2.1 PCon encoder. It also adds a new structure to store these caps,
which can then be used to get the PPS parameters for PCON-HDMI2.1 sink
pair. Which inturn will be used to take a call to override the existing
PPS-metadata, by either writing the entire new PPS metadata, or by
writing only the PPS override parameters.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  16 ++
 drivers/gpu/drm/i915/display/intel_dp.c       | 178 ++++++++++++++++++
 2 files changed, 194 insertions(+)

Comments

Shankar, Uma Oct. 18, 2020, 11:32 p.m. UTC | #1
> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Sent: Thursday, October 15, 2020 4:23 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
> Kulkarni, Vandita <vandita.kulkarni@intel.com>; ville.syrjala@linux.intel.com;
> Sharma, Swati2 <swati2.sharma@intel.com>
> Subject: [RFC 11/13] drm/i915: Read DSC capabilities of the HDMI2.1 PCON
> encoder
> 
> This patch adds a helper function to read the DSC capabilities of the
> HDMI2.1 PCon encoder. It also adds a new structure to store these caps, which
> can then be used to get the PPS parameters for PCON-HDMI2.1 sink pair. Which
> inturn will be used to take a call to override the existing PPS-metadata, by either
> writing the entire new PPS metadata, or by writing only the PPS override
> parameters.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  16 ++
>  drivers/gpu/drm/i915/display/intel_dp.c       | 178 ++++++++++++++++++
>  2 files changed, 194 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 6c69922313d6..23282695a47f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1292,6 +1292,21 @@ struct intel_dp_pcon_frl {
>  	int trained_rate_gbps;
>  };
> 
> +struct intel_dp_pcon_dsc {
> +	bool enc_support;
> +	bool pps_override_support;
> +	bool blk_prediction_support;
> +	u8 version_major;
> +	u8 version_minor;
> +	u8 color_fmt_mask;
> +	u8 color_depth_mask;
> +	u8 max_slices;;
> +	u8 max_slice_width;
> +	u8 line_buf_bit_depth;
> +	u8 bpp_precision_incr;
> +	int rc_buf_size;
> +};
> +
>  struct intel_dp {
>  	i915_reg_t output_reg;
>  	u32 DP;
> @@ -1415,6 +1430,7 @@ struct intel_dp {
>  	bool hobl_active;
> 
>  	struct intel_dp_pcon_frl frl;
> +	struct intel_dp_pcon_dsc pcon_dsc;
>  };
> 
>  enum lspcon_vendor {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index e6c4cb844e37..b4f8abaea607 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3882,6 +3882,182 @@ cpt_set_link_train(struct intel_dp *intel_dp,
>  	intel_de_posting_read(dev_priv, intel_dp->output_reg);  }
> 
> +void intel_dp_get_pcon_dsc_cap(struct intel_dp *intel_dp) {
> +	u8 buf;
> +	u8 rc_buf_blk_size;
> +	u8 max_slices = 0;
> +
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	struct intel_dp_pcon_dsc *pcon_dsc = &intel_dp->pcon_dsc;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENCODER, &buf)
> < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_ENCODER\n");
> +		return;
> +	}
> +	pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED;
> +	pcon_dsc->pps_override_support = buf &
> DP_PCON_DSC_PPS_ENC_OVERRIDE;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_VERSION, &buf)
> < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_VERSION\n");
> +		return;

If we fail here or in any of the subsequent calls below shouldn't we reset the dsc params saying DSC not
supported. Else we may return with ex.
pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED
which would ideally not be right.

> +	}
> +	pcon_dsc->version_major = (buf & DP_PCON_DSC_MAJOR_MASK) >>
> +				  DP_PCON_DSC_MAJOR_SHIFT;
> +	pcon_dsc->version_minor = (buf & DP_PCON_DSC_MINOR_MASK) >>
> +				  DP_PCON_DSC_MINOR_SHIFT;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_PCON_DSC_RC_BUF_BLK_INFO, &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_RC_BUF_BLK_INFO\n");
> +		return;
> +	}
> +
> +	switch (buf & DP_PCON_DSC_RC_BUF_BLK_SIZE) {
> +	case DP_PCON_DSC_RC_BUF_BLK_1KB :
> +		rc_buf_blk_size = 1;
> +		break;
> +	case DP_PCON_DSC_RC_BUF_BLK_4KB :
> +		rc_buf_blk_size = 4;
> +		break;
> +	case DP_PCON_DSC_RC_BUF_BLK_16KB :
> +		rc_buf_blk_size = 16;
> +		break;
> +	case DP_PCON_DSC_RC_BUF_BLK_64KB :
> +		rc_buf_blk_size = 64;
> +		break;
> +	default :
> +		rc_buf_blk_size = 0;
> +	}

It would be good if you create sub-functions for each of these aspects to make it more readable.

> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_RC_BUF_SIZE,
> &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_RC_BUF_SIZE\n");
> +		return;
> +	}
> +	/* storing rc_buf_size in bytes */
> +	pcon_dsc->rc_buf_size = (buf + 1) * rc_buf_blk_size * 1024;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_SLICE_CAP_2,
> &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_SLICE_CAP_2\n");
> +		return;
> +	}
> +	if (buf & DP_PCON_DSC_24_PER_DSC_ENC)
> +	       max_slices = 24;
> +	else if (buf & DP_PCON_DSC_20_PER_DSC_ENC)
> +		max_slices = 20;
> +	else if (buf & DP_PCON_DSC_16_PER_DSC_ENC)
> +		max_slices = 16;
> +
> +	if (max_slices == 0) {
> +		if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_PCON_DSC_SLICE_CAP_1,
> +				      &buf) < 0) {
> +			drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_SLICE_CAP_2\n");

Its SLICE_CAP_1.

> +			return;
> +		}
> +
> +		if (buf & DP_PCON_DSC_12_PER_DSC_ENC)
> +			max_slices = 12;
> +		else if (buf & DP_PCON_DSC_10_PER_DSC_ENC)
> +			max_slices = 10;
> +		else if (buf & DP_PCON_DSC_8_PER_DSC_ENC)
> +			max_slices = 8;
> +		else if (buf & DP_PCON_DSC_6_PER_DSC_ENC)
> +			max_slices = 6;
> +		else if (buf & DP_PCON_DSC_4_PER_DSC_ENC)
> +			max_slices = 4;
> +		else if (buf & DP_PCON_DSC_2_PER_DSC_ENC)
> +			max_slices = 2;
> +		else if (buf & DP_PCON_DSC_1_PER_DSC_ENC)
> +			max_slices = 1;

Use switch here as well.

> +	}
> +
> +	pcon_dsc->max_slices = max_slices;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BUF_BIT_DEPTH,
> &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_BUF_BIT_DEPTH\n");
> +		return;
> +	}
> +	switch (buf & DP_PCON_DSC_BIT_DEPTH_MASK) {
> +	case DP_PCON_DSC_DEPTH_8_BITS :
> +		pcon_dsc->line_buf_bit_depth = 8;
> +		break;
> +	case DP_PCON_DSC_DEPTH_9_BITS :
> +		pcon_dsc->line_buf_bit_depth = 9;
> +		break;
> +	case DP_PCON_DSC_DEPTH_10_BITS :
> +		pcon_dsc->line_buf_bit_depth = 10;
> +		break;
> +	case DP_PCON_DSC_DEPTH_11_BITS :
> +		pcon_dsc->line_buf_bit_depth = 11;
> +		break;
> +	case DP_PCON_DSC_DEPTH_12_BITS :
> +		pcon_dsc->line_buf_bit_depth = 12;
> +		break;
> +	case DP_PCON_DSC_DEPTH_13_BITS :
> +		pcon_dsc->line_buf_bit_depth = 13;
> +		break;
> +	case DP_PCON_DSC_DEPTH_14_BITS :
> +		pcon_dsc->line_buf_bit_depth = 14;
> +		break;
> +	case DP_PCON_DSC_DEPTH_15_BITS :
> +		pcon_dsc->line_buf_bit_depth = 15;
> +		break;
> +	case DP_PCON_DSC_DEPTH_16_BITS :
> +		pcon_dsc->line_buf_bit_depth = 16;
> +		break;
> +	default :
> +		pcon_dsc->line_buf_bit_depth = 0;
> +	}
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_PCON_DSC_BLOCK_PREDICTION, &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_BLOCK_PREDICTION\n");
> +		return;
> +	}
> +	if (buf && DP_PCON_DSC_BLOCK_PRED_SUPPORT)
> +		pcon_dsc->blk_prediction_support = true;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_PCON_DSC_ENC_COLOR_FMT_CAP, &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_ENC_COLOR_FMT_CAP\n");
> +		return;
> +	}
> +	pcon_dsc->color_fmt_mask = buf;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_PCON_DSC_ENC_COLOR_DEPTH_CAP, &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_ENC_COLOR_DEPTH_CAP\n");
> +		return;
> +	}
> +	pcon_dsc->color_depth_mask = buf;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_PCON_DSC_MAX_SLICE_WIDTH, &buf) < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_MAX_SLICE_WIDTH\n");
> +		return;
> +	}
> +	pcon_dsc->max_slice_width = buf;;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BPP_INCR, &buf)
> < 0) {
> +		drm_err(&i915->drm, "Failed to read
> DP_PCON_DSC_BPP_INCR\n");
> +		return;
> +	}
> +	switch(buf & DP_PCON_DSC_BPP_INCR_MASK) {
> +	case DP_PCON_DSC_ONE_16TH_BPP:
> +		pcon_dsc->bpp_precision_incr = 16;
> +		break;
> +	case DP_PCON_DSC_ONE_8TH_BPP:
> +		pcon_dsc->bpp_precision_incr = 8;
> +		break;
> +	case DP_PCON_DSC_ONE_4TH_BPP:
> +		pcon_dsc->bpp_precision_incr = 4;
> +		break;
> +	case DP_PCON_DSC_ONE_HALF_BPP:
> +		pcon_dsc->bpp_precision_incr = 2;
> +		break;
> +	case DP_PCON_DSC_ONE_BPP:
> +		pcon_dsc->bpp_precision_incr = 1;
> +		break;
> +	default :
> +		pcon_dsc->bpp_precision_incr = 0;
> +	}
> +}
> +
>  static int intel_dp_get_max_rate_gbps(struct intel_dp *intel_dp)  {
>  	int max_link_clock, max_lanes, max_rate_khz, max_rate_gbps; @@ -
> 6659,6 +6835,8 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
>  		    intel_dp->dfp.max_tmds_clock,
>  		    intel_dp->dfp.pcon_max_frl,
>  		    intel_dp->dfp.sink_max_frl);
> +
> +	intel_dp_get_pcon_dsc_cap(intel_dp);
>  }
> 
>  static void
> --
> 2.17.1
Shankar, Uma Oct. 18, 2020, 11:34 p.m. UTC | #2
> -----Original Message-----
> From: Shankar, Uma
> Sent: Monday, October 19, 2020 5:02 AM
> To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Kulkarni, Vandita
> <vandita.kulkarni@intel.com>; ville.syrjala@linux.intel.com; Sharma, Swati2
> <swati2.sharma@intel.com>
> Subject: RE: [RFC 11/13] drm/i915: Read DSC capabilities of the HDMI2.1 PCON
> encoder

Also it would be good to move to DRM Core.

> 
> 
> > -----Original Message-----
> > From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > Sent: Thursday, October 15, 2020 4:23 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Shankar, Uma
> > <uma.shankar@intel.com>; Kulkarni, Vandita
> > <vandita.kulkarni@intel.com>; ville.syrjala@linux.intel.com; Sharma,
> > Swati2 <swati2.sharma@intel.com>
> > Subject: [RFC 11/13] drm/i915: Read DSC capabilities of the HDMI2.1
> > PCON encoder
> >
> > This patch adds a helper function to read the DSC capabilities of the
> > HDMI2.1 PCon encoder. It also adds a new structure to store these
> > caps, which can then be used to get the PPS parameters for
> > PCON-HDMI2.1 sink pair. Which inturn will be used to take a call to
> > override the existing PPS-metadata, by either writing the entire new
> > PPS metadata, or by writing only the PPS override parameters.
> >
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_types.h    |  16 ++
> >  drivers/gpu/drm/i915/display/intel_dp.c       | 178 ++++++++++++++++++
> >  2 files changed, 194 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 6c69922313d6..23282695a47f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1292,6 +1292,21 @@ struct intel_dp_pcon_frl {
> >  	int trained_rate_gbps;
> >  };
> >
> > +struct intel_dp_pcon_dsc {
> > +	bool enc_support;
> > +	bool pps_override_support;
> > +	bool blk_prediction_support;
> > +	u8 version_major;
> > +	u8 version_minor;
> > +	u8 color_fmt_mask;
> > +	u8 color_depth_mask;
> > +	u8 max_slices;;
> > +	u8 max_slice_width;
> > +	u8 line_buf_bit_depth;
> > +	u8 bpp_precision_incr;
> > +	int rc_buf_size;
> > +};
> > +
> >  struct intel_dp {
> >  	i915_reg_t output_reg;
> >  	u32 DP;
> > @@ -1415,6 +1430,7 @@ struct intel_dp {
> >  	bool hobl_active;
> >
> >  	struct intel_dp_pcon_frl frl;
> > +	struct intel_dp_pcon_dsc pcon_dsc;
> >  };
> >
> >  enum lspcon_vendor {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index e6c4cb844e37..b4f8abaea607 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -3882,6 +3882,182 @@ cpt_set_link_train(struct intel_dp *intel_dp,
> >  	intel_de_posting_read(dev_priv, intel_dp->output_reg);  }
> >
> > +void intel_dp_get_pcon_dsc_cap(struct intel_dp *intel_dp) {
> > +	u8 buf;
> > +	u8 rc_buf_blk_size;
> > +	u8 max_slices = 0;
> > +
> > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > +	struct intel_dp_pcon_dsc *pcon_dsc = &intel_dp->pcon_dsc;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENCODER, &buf)
> > < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_ENCODER\n");
> > +		return;
> > +	}
> > +	pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED;
> > +	pcon_dsc->pps_override_support = buf &
> > DP_PCON_DSC_PPS_ENC_OVERRIDE;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_VERSION, &buf)
> > < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_VERSION\n");
> > +		return;
> 
> If we fail here or in any of the subsequent calls below shouldn't we reset the dsc
> params saying DSC not supported. Else we may return with ex.
> pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED which
> would ideally not be right.
> 
> > +	}
> > +	pcon_dsc->version_major = (buf & DP_PCON_DSC_MAJOR_MASK) >>
> > +				  DP_PCON_DSC_MAJOR_SHIFT;
> > +	pcon_dsc->version_minor = (buf & DP_PCON_DSC_MINOR_MASK) >>
> > +				  DP_PCON_DSC_MINOR_SHIFT;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_PCON_DSC_RC_BUF_BLK_INFO, &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_RC_BUF_BLK_INFO\n");
> > +		return;
> > +	}
> > +
> > +	switch (buf & DP_PCON_DSC_RC_BUF_BLK_SIZE) {
> > +	case DP_PCON_DSC_RC_BUF_BLK_1KB :
> > +		rc_buf_blk_size = 1;
> > +		break;
> > +	case DP_PCON_DSC_RC_BUF_BLK_4KB :
> > +		rc_buf_blk_size = 4;
> > +		break;
> > +	case DP_PCON_DSC_RC_BUF_BLK_16KB :
> > +		rc_buf_blk_size = 16;
> > +		break;
> > +	case DP_PCON_DSC_RC_BUF_BLK_64KB :
> > +		rc_buf_blk_size = 64;
> > +		break;
> > +	default :
> > +		rc_buf_blk_size = 0;
> > +	}
> 
> It would be good if you create sub-functions for each of these aspects to make it
> more readable.
> 
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_RC_BUF_SIZE,
> > &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_RC_BUF_SIZE\n");
> > +		return;
> > +	}
> > +	/* storing rc_buf_size in bytes */
> > +	pcon_dsc->rc_buf_size = (buf + 1) * rc_buf_blk_size * 1024;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_SLICE_CAP_2,
> > &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_SLICE_CAP_2\n");
> > +		return;
> > +	}
> > +	if (buf & DP_PCON_DSC_24_PER_DSC_ENC)
> > +	       max_slices = 24;
> > +	else if (buf & DP_PCON_DSC_20_PER_DSC_ENC)
> > +		max_slices = 20;
> > +	else if (buf & DP_PCON_DSC_16_PER_DSC_ENC)
> > +		max_slices = 16;
> > +
> > +	if (max_slices == 0) {
> > +		if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_PCON_DSC_SLICE_CAP_1,
> > +				      &buf) < 0) {
> > +			drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_SLICE_CAP_2\n");
> 
> Its SLICE_CAP_1.
> 
> > +			return;
> > +		}
> > +
> > +		if (buf & DP_PCON_DSC_12_PER_DSC_ENC)
> > +			max_slices = 12;
> > +		else if (buf & DP_PCON_DSC_10_PER_DSC_ENC)
> > +			max_slices = 10;
> > +		else if (buf & DP_PCON_DSC_8_PER_DSC_ENC)
> > +			max_slices = 8;
> > +		else if (buf & DP_PCON_DSC_6_PER_DSC_ENC)
> > +			max_slices = 6;
> > +		else if (buf & DP_PCON_DSC_4_PER_DSC_ENC)
> > +			max_slices = 4;
> > +		else if (buf & DP_PCON_DSC_2_PER_DSC_ENC)
> > +			max_slices = 2;
> > +		else if (buf & DP_PCON_DSC_1_PER_DSC_ENC)
> > +			max_slices = 1;
> 
> Use switch here as well.
> 
> > +	}
> > +
> > +	pcon_dsc->max_slices = max_slices;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BUF_BIT_DEPTH,
> > &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_BUF_BIT_DEPTH\n");
> > +		return;
> > +	}
> > +	switch (buf & DP_PCON_DSC_BIT_DEPTH_MASK) {
> > +	case DP_PCON_DSC_DEPTH_8_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 8;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_9_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 9;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_10_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 10;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_11_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 11;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_12_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 12;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_13_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 13;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_14_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 14;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_15_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 15;
> > +		break;
> > +	case DP_PCON_DSC_DEPTH_16_BITS :
> > +		pcon_dsc->line_buf_bit_depth = 16;
> > +		break;
> > +	default :
> > +		pcon_dsc->line_buf_bit_depth = 0;
> > +	}
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_PCON_DSC_BLOCK_PREDICTION, &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_BLOCK_PREDICTION\n");
> > +		return;
> > +	}
> > +	if (buf && DP_PCON_DSC_BLOCK_PRED_SUPPORT)
> > +		pcon_dsc->blk_prediction_support = true;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_PCON_DSC_ENC_COLOR_FMT_CAP, &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_ENC_COLOR_FMT_CAP\n");
> > +		return;
> > +	}
> > +	pcon_dsc->color_fmt_mask = buf;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_PCON_DSC_ENC_COLOR_DEPTH_CAP, &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_ENC_COLOR_DEPTH_CAP\n");
> > +		return;
> > +	}
> > +	pcon_dsc->color_depth_mask = buf;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> > DP_PCON_DSC_MAX_SLICE_WIDTH, &buf) < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_MAX_SLICE_WIDTH\n");
> > +		return;
> > +	}
> > +	pcon_dsc->max_slice_width = buf;;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BPP_INCR, &buf)
> > < 0) {
> > +		drm_err(&i915->drm, "Failed to read
> > DP_PCON_DSC_BPP_INCR\n");
> > +		return;
> > +	}
> > +	switch(buf & DP_PCON_DSC_BPP_INCR_MASK) {
> > +	case DP_PCON_DSC_ONE_16TH_BPP:
> > +		pcon_dsc->bpp_precision_incr = 16;
> > +		break;
> > +	case DP_PCON_DSC_ONE_8TH_BPP:
> > +		pcon_dsc->bpp_precision_incr = 8;
> > +		break;
> > +	case DP_PCON_DSC_ONE_4TH_BPP:
> > +		pcon_dsc->bpp_precision_incr = 4;
> > +		break;
> > +	case DP_PCON_DSC_ONE_HALF_BPP:
> > +		pcon_dsc->bpp_precision_incr = 2;
> > +		break;
> > +	case DP_PCON_DSC_ONE_BPP:
> > +		pcon_dsc->bpp_precision_incr = 1;
> > +		break;
> > +	default :
> > +		pcon_dsc->bpp_precision_incr = 0;
> > +	}
> > +}
> > +
> >  static int intel_dp_get_max_rate_gbps(struct intel_dp *intel_dp)  {
> >  	int max_link_clock, max_lanes, max_rate_khz, max_rate_gbps; @@ -
> > 6659,6 +6835,8 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
> >  		    intel_dp->dfp.max_tmds_clock,
> >  		    intel_dp->dfp.pcon_max_frl,
> >  		    intel_dp->dfp.sink_max_frl);
> > +
> > +	intel_dp_get_pcon_dsc_cap(intel_dp);
> >  }
> >
> >  static void
> > --
> > 2.17.1
Nautiyal, Ankit K Nov. 1, 2020, 7:13 a.m. UTC | #3
On 10/19/2020 5:02 AM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
>> Sent: Thursday, October 15, 2020 4:23 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>> Kulkarni, Vandita <vandita.kulkarni@intel.com>; ville.syrjala@linux.intel.com;
>> Sharma, Swati2 <swati2.sharma@intel.com>
>> Subject: [RFC 11/13] drm/i915: Read DSC capabilities of the HDMI2.1 PCON
>> encoder
>>
>> This patch adds a helper function to read the DSC capabilities of the
>> HDMI2.1 PCon encoder. It also adds a new structure to store these caps, which
>> can then be used to get the PPS parameters for PCON-HDMI2.1 sink pair. Which
>> inturn will be used to take a call to override the existing PPS-metadata, by either
>> writing the entire new PPS metadata, or by writing only the PPS override
>> parameters.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   .../drm/i915/display/intel_display_types.h    |  16 ++
>>   drivers/gpu/drm/i915/display/intel_dp.c       | 178 ++++++++++++++++++
>>   2 files changed, 194 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 6c69922313d6..23282695a47f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1292,6 +1292,21 @@ struct intel_dp_pcon_frl {
>>   int trained_rate_gbps;
>>   };
>>
>> +struct intel_dp_pcon_dsc {
>> +bool enc_support;
>> +bool pps_override_support;
>> +bool blk_prediction_support;
>> +u8 version_major;
>> +u8 version_minor;
>> +u8 color_fmt_mask;
>> +u8 color_depth_mask;
>> +u8 max_slices;;
>> +u8 max_slice_width;
>> +u8 line_buf_bit_depth;
>> +u8 bpp_precision_incr;
>> +int rc_buf_size;
>> +};
>> +
>>   struct intel_dp {
>>   i915_reg_t output_reg;
>>   u32 DP;
>> @@ -1415,6 +1430,7 @@ struct intel_dp {
>>   bool hobl_active;
>>
>>   struct intel_dp_pcon_frl frl;
>> +struct intel_dp_pcon_dsc pcon_dsc;
>>   };
>>
>>   enum lspcon_vendor {
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index e6c4cb844e37..b4f8abaea607 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -3882,6 +3882,182 @@ cpt_set_link_train(struct intel_dp *intel_dp,
>>   intel_de_posting_read(dev_priv, intel_dp->output_reg);  }
>>
>> +void intel_dp_get_pcon_dsc_cap(struct intel_dp *intel_dp) {
>> +u8 buf;
>> +u8 rc_buf_blk_size;
>> +u8 max_slices = 0;
>> +
>> +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> +struct intel_dp_pcon_dsc *pcon_dsc = &intel_dp->pcon_dsc;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENCODER, &buf)
>> < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_ENCODER\n");
>> +return;
>> +}
>> +pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED;
>> +pcon_dsc->pps_override_support = buf &
>> DP_PCON_DSC_PPS_ENC_OVERRIDE;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_VERSION, &buf)
>> < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_VERSION\n");
>> +return;
> If we fail here or in any of the subsequent calls below shouldn't we reset the dsc params saying DSC not
> supported. Else we may return with ex.
> pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED
> which would ideally not be right.

You are right, will take care in next patch. If dsc 1.2 is not supported 
we will not configure PCON encode DSC DPCDs.

Also I have realized, we do not use all the information for writing PPS 
parameters.

So aligning with the existing code, will read all the pcon's DSC encoder 
caps and store in intel_dp structure in a u8 array.

Will have drm helper functions to retrieve values like max slices, max 
slice with and bpp precision which need some processing, based on the 
port caps.


>> +}
>> +pcon_dsc->version_major = (buf & DP_PCON_DSC_MAJOR_MASK) >>
>> +  DP_PCON_DSC_MAJOR_SHIFT;
>> +pcon_dsc->version_minor = (buf & DP_PCON_DSC_MINOR_MASK) >>
>> +  DP_PCON_DSC_MINOR_SHIFT;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux,
>> DP_PCON_DSC_RC_BUF_BLK_INFO, &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_RC_BUF_BLK_INFO\n");
>> +return;
>> +}
>> +
>> +switch (buf & DP_PCON_DSC_RC_BUF_BLK_SIZE) {
>> +case DP_PCON_DSC_RC_BUF_BLK_1KB :
>> +rc_buf_blk_size = 1;
>> +break;
>> +case DP_PCON_DSC_RC_BUF_BLK_4KB :
>> +rc_buf_blk_size = 4;.
>> +break;
>> +case DP_PCON_DSC_RC_BUF_BLK_16KB :
>> +rc_buf_blk_size = 16;
>> +break;
>> +case DP_PCON_DSC_RC_BUF_BLK_64KB :
>> +rc_buf_blk_size = 64;
>> +break;
>> +default :
>> +rc_buf_blk_size = 0;
>> +}
> It would be good if you create sub-functions for each of these aspects to make it more readable.

Agreed. As mentioned above will have drm_helper functions for these.


>
>> +if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_RC_BUF_SIZE,
>> &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_RC_BUF_SIZE\n");
>> +return;
>> +}
>> +/* storing rc_buf_size in bytes */
>> +pcon_dsc->rc_buf_size = (buf + 1) * rc_buf_blk_size * 1024;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_SLICE_CAP_2,
>> &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_SLICE_CAP_2\n");
>> +return;
>> +}
>> +if (buf & DP_PCON_DSC_24_PER_DSC_ENC)
>> +       max_slices = 24;
>> +else if (buf & DP_PCON_DSC_20_PER_DSC_ENC)
>> +max_slices = 20;
>> +else if (buf & DP_PCON_DSC_16_PER_DSC_ENC)
>> +max_slices = 16;
>> +
>> +if (max_slices == 0) {
>> +if (drm_dp_dpcd_readb(&intel_dp->aux,
>> DP_PCON_DSC_SLICE_CAP_1,
>> +      &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_SLICE_CAP_2\n");
> Its SLICE_CAP_1.


Agreed, will fix in next version.


>
>> +return;
>> +}
>> +
>> +if (buf & DP_PCON_DSC_12_PER_DSC_ENC)
>> +max_slices = 12;
>> +else if (buf & DP_PCON_DSC_10_PER_DSC_ENC)
>> +max_slices = 10;
>> +else if (buf & DP_PCON_DSC_8_PER_DSC_ENC)
>> +max_slices = 8;
>> +else if (buf & DP_PCON_DSC_6_PER_DSC_ENC)
>> +max_slices = 6;
>> +else if (buf & DP_PCON_DSC_4_PER_DSC_ENC)
>> +max_slices = 4;
>> +else if (buf & DP_PCON_DSC_2_PER_DSC_ENC)
>> +max_slices = 2;
>> +else if (buf & DP_PCON_DSC_1_PER_DSC_ENC)
>> +max_slices = 1;
> Use switch here as well.

Wont be able to use switch here as logical & with different values is 
done. I will make it more compact and readable in next version.

Regards,

Ankit


>> +}
>> +
>> +pcon_dsc->max_slices = max_slices;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BUF_BIT_DEPTH,
>> &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_BUF_BIT_DEPTH\n");
>> +return;
>> +}
>> +switch (buf & DP_PCON_DSC_BIT_DEPTH_MASK) {
>> +case DP_PCON_DSC_DEPTH_8_BITS :
>> +pcon_dsc->line_buf_bit_depth = 8;
>> +break;
>> +case DP_PCON_DSC_DEPTH_9_BITS :
>> +pcon_dsc->line_buf_bit_depth = 9;
>> +break;
>> +case DP_PCON_DSC_DEPTH_10_BITS :
>> +pcon_dsc->line_buf_bit_depth = 10;
>> +break;
>> +case DP_PCON_DSC_DEPTH_11_BITS :
>> +pcon_dsc->line_buf_bit_depth = 11;
>> +break;
>> +case DP_PCON_DSC_DEPTH_12_BITS :
>> +pcon_dsc->line_buf_bit_depth = 12;
>> +break;
>> +case DP_PCON_DSC_DEPTH_13_BITS :
>> +pcon_dsc->line_buf_bit_depth = 13;
>> +break;
>> +case DP_PCON_DSC_DEPTH_14_BITS :
>> +pcon_dsc->line_buf_bit_depth = 14;
>> +break;
>> +case DP_PCON_DSC_DEPTH_15_BITS :
>> +pcon_dsc->line_buf_bit_depth = 15;
>> +break;
>> +case DP_PCON_DSC_DEPTH_16_BITS :
>> +pcon_dsc->line_buf_bit_depth = 16;
>> +break;
>> +default :
>> +pcon_dsc->line_buf_bit_depth = 0;
>> +}
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux,
>> DP_PCON_DSC_BLOCK_PREDICTION, &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_BLOCK_PREDICTION\n");
>> +return;
>> +}
>> +if (buf && DP_PCON_DSC_BLOCK_PRED_SUPPORT)
>> +pcon_dsc->blk_prediction_support = true;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux,
>> DP_PCON_DSC_ENC_COLOR_FMT_CAP, &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_ENC_COLOR_FMT_CAP\n");
>> +return;
>> +}
>> +pcon_dsc->color_fmt_mask = buf;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux,
>> DP_PCON_DSC_ENC_COLOR_DEPTH_CAP, &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_ENC_COLOR_DEPTH_CAP\n");
>> +return;
>> +}
>> +pcon_dsc->color_depth_mask = buf;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux,
>> DP_PCON_DSC_MAX_SLICE_WIDTH, &buf) < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_MAX_SLICE_WIDTH\n");
>> +return;
>> +}
>> +pcon_dsc->max_slice_width = buf;;
>> +
>> +if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BPP_INCR, &buf)
>> < 0) {
>> +drm_err(&i915->drm, "Failed to read
>> DP_PCON_DSC_BPP_INCR\n");
>> +return;
>> +}
>> +switch(buf & DP_PCON_DSC_BPP_INCR_MASK) {
>> +case DP_PCON_DSC_ONE_16TH_BPP:
>> +pcon_dsc->bpp_precision_incr = 16;
>> +break;
>> +case DP_PCON_DSC_ONE_8TH_BPP:
>> +pcon_dsc->bpp_precision_incr = 8;
>> +break;
>> +case DP_PCON_DSC_ONE_4TH_BPP:
>> +pcon_dsc->bpp_precision_incr = 4;
>> +break;
>> +case DP_PCON_DSC_ONE_HALF_BPP:
>> +pcon_dsc->bpp_precision_incr = 2;
>> +break;
>> +case DP_PCON_DSC_ONE_BPP:
>> +pcon_dsc->bpp_precision_incr = 1;
>> +break;
>> +default :
>> +pcon_dsc->bpp_precision_incr = 0;
>> +}
>> +}
>> +
>>   static int intel_dp_get_max_rate_gbps(struct intel_dp *intel_dp)  {
>>   int max_link_clock, max_lanes, max_rate_khz, max_rate_gbps; @@ -
>> 6659,6 +6835,8 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
>>       intel_dp->dfp.max_tmds_clock,
>>       intel_dp->dfp.pcon_max_frl,
>>       intel_dp->dfp.sink_max_frl);
>> +
>> +intel_dp_get_pcon_dsc_cap(intel_dp);
>>   }
>>
>>   static void
>> --
>> 2.17.1
Nautiyal, Ankit K Nov. 1, 2020, 7:14 a.m. UTC | #4
On 10/19/2020 5:04 AM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, October 19, 2020 5:02 AM
>> To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org; Kulkarni, Vandita
>> <vandita.kulkarni@intel.com>; ville.syrjala@linux.intel.com; Sharma, Swati2
>> <swati2.sharma@intel.com>
>> Subject: RE: [RFC 11/13] drm/i915: Read DSC capabilities of the HDMI2.1 PCON
>> encoder
> Also it would be good to move to DRM Core.


Agreed, as mentioned in earlier patch, will move the appropriate code as 
drm_helper functions.


Regards,

Ankit

>
>>
>>> -----Original Message-----
>>> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
>>> Sent: Thursday, October 15, 2020 4:23 PM
>>> To: intel-gfx@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org; Shankar, Uma
>>> <uma.shankar@intel.com>; Kulkarni, Vandita
>>> <vandita.kulkarni@intel.com>; ville.syrjala@linux.intel.com; Sharma,
>>> Swati2 <swati2.sharma@intel.com>
>>> Subject: [RFC 11/13] drm/i915: Read DSC capabilities of the HDMI2.1
>>> PCON encoder
>>>
>>> This patch adds a helper function to read the DSC capabilities of the
>>> HDMI2.1 PCon encoder. It also adds a new structure to store these
>>> caps, which can then be used to get the PPS parameters for
>>> PCON-HDMI2.1 sink pair. Which inturn will be used to take a call to
>>> override the existing PPS-metadata, by either writing the entire new
>>> PPS metadata, or by writing only the PPS override parameters.
>>>
>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>> ---
>>>   .../drm/i915/display/intel_display_types.h    |  16 ++
>>>   drivers/gpu/drm/i915/display/intel_dp.c       | 178 ++++++++++++++++++
>>>   2 files changed, 194 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>>> index 6c69922313d6..23282695a47f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>>> @@ -1292,6 +1292,21 @@ struct intel_dp_pcon_frl {
>>>   	int trained_rate_gbps;
>>>   };
>>>
>>> +struct intel_dp_pcon_dsc {
>>> +	bool enc_support;
>>> +	bool pps_override_support;
>>> +	bool blk_prediction_support;
>>> +	u8 version_major;
>>> +	u8 version_minor;
>>> +	u8 color_fmt_mask;
>>> +	u8 color_depth_mask;
>>> +	u8 max_slices;;
>>> +	u8 max_slice_width;
>>> +	u8 line_buf_bit_depth;
>>> +	u8 bpp_precision_incr;
>>> +	int rc_buf_size;
>>> +};
>>> +
>>>   struct intel_dp {
>>>   	i915_reg_t output_reg;
>>>   	u32 DP;
>>> @@ -1415,6 +1430,7 @@ struct intel_dp {
>>>   	bool hobl_active;
>>>
>>>   	struct intel_dp_pcon_frl frl;
>>> +	struct intel_dp_pcon_dsc pcon_dsc;
>>>   };
>>>
>>>   enum lspcon_vendor {
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index e6c4cb844e37..b4f8abaea607 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -3882,6 +3882,182 @@ cpt_set_link_train(struct intel_dp *intel_dp,
>>>   	intel_de_posting_read(dev_priv, intel_dp->output_reg);  }
>>>
>>> +void intel_dp_get_pcon_dsc_cap(struct intel_dp *intel_dp) {
>>> +	u8 buf;
>>> +	u8 rc_buf_blk_size;
>>> +	u8 max_slices = 0;
>>> +
>>> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>>> +	struct intel_dp_pcon_dsc *pcon_dsc = &intel_dp->pcon_dsc;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENCODER, &buf)
>>> < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_ENCODER\n");
>>> +		return;
>>> +	}
>>> +	pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED;
>>> +	pcon_dsc->pps_override_support = buf &
>>> DP_PCON_DSC_PPS_ENC_OVERRIDE;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_VERSION, &buf)
>>> < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_VERSION\n");
>>> +		return;
>> If we fail here or in any of the subsequent calls below shouldn't we reset the dsc
>> params saying DSC not supported. Else we may return with ex.
>> pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED which
>> would ideally not be right.
>>
>>> +	}
>>> +	pcon_dsc->version_major = (buf & DP_PCON_DSC_MAJOR_MASK) >>
>>> +				  DP_PCON_DSC_MAJOR_SHIFT;
>>> +	pcon_dsc->version_minor = (buf & DP_PCON_DSC_MINOR_MASK) >>
>>> +				  DP_PCON_DSC_MINOR_SHIFT;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
>>> DP_PCON_DSC_RC_BUF_BLK_INFO, &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_RC_BUF_BLK_INFO\n");
>>> +		return;
>>> +	}
>>> +
>>> +	switch (buf & DP_PCON_DSC_RC_BUF_BLK_SIZE) {
>>> +	case DP_PCON_DSC_RC_BUF_BLK_1KB :
>>> +		rc_buf_blk_size = 1;
>>> +		break;
>>> +	case DP_PCON_DSC_RC_BUF_BLK_4KB :
>>> +		rc_buf_blk_size = 4;
>>> +		break;
>>> +	case DP_PCON_DSC_RC_BUF_BLK_16KB :
>>> +		rc_buf_blk_size = 16;
>>> +		break;
>>> +	case DP_PCON_DSC_RC_BUF_BLK_64KB :
>>> +		rc_buf_blk_size = 64;
>>> +		break;
>>> +	default :
>>> +		rc_buf_blk_size = 0;
>>> +	}
>> It would be good if you create sub-functions for each of these aspects to make it
>> more readable.
>>
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_RC_BUF_SIZE,
>>> &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_RC_BUF_SIZE\n");
>>> +		return;
>>> +	}
>>> +	/* storing rc_buf_size in bytes */
>>> +	pcon_dsc->rc_buf_size = (buf + 1) * rc_buf_blk_size * 1024;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_SLICE_CAP_2,
>>> &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_SLICE_CAP_2\n");
>>> +		return;
>>> +	}
>>> +	if (buf & DP_PCON_DSC_24_PER_DSC_ENC)
>>> +	       max_slices = 24;
>>> +	else if (buf & DP_PCON_DSC_20_PER_DSC_ENC)
>>> +		max_slices = 20;
>>> +	else if (buf & DP_PCON_DSC_16_PER_DSC_ENC)
>>> +		max_slices = 16;
>>> +
>>> +	if (max_slices == 0) {
>>> +		if (drm_dp_dpcd_readb(&intel_dp->aux,
>>> DP_PCON_DSC_SLICE_CAP_1,
>>> +				      &buf) < 0) {
>>> +			drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_SLICE_CAP_2\n");
>> Its SLICE_CAP_1.
>>
>>> +			return;
>>> +		}
>>> +
>>> +		if (buf & DP_PCON_DSC_12_PER_DSC_ENC)
>>> +			max_slices = 12;
>>> +		else if (buf & DP_PCON_DSC_10_PER_DSC_ENC)
>>> +			max_slices = 10;
>>> +		else if (buf & DP_PCON_DSC_8_PER_DSC_ENC)
>>> +			max_slices = 8;
>>> +		else if (buf & DP_PCON_DSC_6_PER_DSC_ENC)
>>> +			max_slices = 6;
>>> +		else if (buf & DP_PCON_DSC_4_PER_DSC_ENC)
>>> +			max_slices = 4;
>>> +		else if (buf & DP_PCON_DSC_2_PER_DSC_ENC)
>>> +			max_slices = 2;
>>> +		else if (buf & DP_PCON_DSC_1_PER_DSC_ENC)
>>> +			max_slices = 1;
>> Use switch here as well.
>>
>>> +	}
>>> +
>>> +	pcon_dsc->max_slices = max_slices;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BUF_BIT_DEPTH,
>>> &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_BUF_BIT_DEPTH\n");
>>> +		return;
>>> +	}
>>> +	switch (buf & DP_PCON_DSC_BIT_DEPTH_MASK) {
>>> +	case DP_PCON_DSC_DEPTH_8_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 8;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_9_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 9;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_10_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 10;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_11_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 11;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_12_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 12;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_13_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 13;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_14_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 14;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_15_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 15;
>>> +		break;
>>> +	case DP_PCON_DSC_DEPTH_16_BITS :
>>> +		pcon_dsc->line_buf_bit_depth = 16;
>>> +		break;
>>> +	default :
>>> +		pcon_dsc->line_buf_bit_depth = 0;
>>> +	}
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
>>> DP_PCON_DSC_BLOCK_PREDICTION, &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_BLOCK_PREDICTION\n");
>>> +		return;
>>> +	}
>>> +	if (buf && DP_PCON_DSC_BLOCK_PRED_SUPPORT)
>>> +		pcon_dsc->blk_prediction_support = true;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
>>> DP_PCON_DSC_ENC_COLOR_FMT_CAP, &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_ENC_COLOR_FMT_CAP\n");
>>> +		return;
>>> +	}
>>> +	pcon_dsc->color_fmt_mask = buf;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
>>> DP_PCON_DSC_ENC_COLOR_DEPTH_CAP, &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_ENC_COLOR_DEPTH_CAP\n");
>>> +		return;
>>> +	}
>>> +	pcon_dsc->color_depth_mask = buf;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
>>> DP_PCON_DSC_MAX_SLICE_WIDTH, &buf) < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_MAX_SLICE_WIDTH\n");
>>> +		return;
>>> +	}
>>> +	pcon_dsc->max_slice_width = buf;;
>>> +
>>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BPP_INCR, &buf)
>>> < 0) {
>>> +		drm_err(&i915->drm, "Failed to read
>>> DP_PCON_DSC_BPP_INCR\n");
>>> +		return;
>>> +	}
>>> +	switch(buf & DP_PCON_DSC_BPP_INCR_MASK) {
>>> +	case DP_PCON_DSC_ONE_16TH_BPP:
>>> +		pcon_dsc->bpp_precision_incr = 16;
>>> +		break;
>>> +	case DP_PCON_DSC_ONE_8TH_BPP:
>>> +		pcon_dsc->bpp_precision_incr = 8;
>>> +		break;
>>> +	case DP_PCON_DSC_ONE_4TH_BPP:
>>> +		pcon_dsc->bpp_precision_incr = 4;
>>> +		break;
>>> +	case DP_PCON_DSC_ONE_HALF_BPP:
>>> +		pcon_dsc->bpp_precision_incr = 2;
>>> +		break;
>>> +	case DP_PCON_DSC_ONE_BPP:
>>> +		pcon_dsc->bpp_precision_incr = 1;
>>> +		break;
>>> +	default :
>>> +		pcon_dsc->bpp_precision_incr = 0;
>>> +	}
>>> +}
>>> +
>>>   static int intel_dp_get_max_rate_gbps(struct intel_dp *intel_dp)  {
>>>   	int max_link_clock, max_lanes, max_rate_khz, max_rate_gbps; @@ -
>>> 6659,6 +6835,8 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
>>>   		    intel_dp->dfp.max_tmds_clock,
>>>   		    intel_dp->dfp.pcon_max_frl,
>>>   		    intel_dp->dfp.sink_max_frl);
>>> +
>>> +	intel_dp_get_pcon_dsc_cap(intel_dp);
>>>   }
>>>
>>>   static void
>>> --
>>> 2.17.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 6c69922313d6..23282695a47f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1292,6 +1292,21 @@  struct intel_dp_pcon_frl {
 	int trained_rate_gbps;
 };
 
+struct intel_dp_pcon_dsc {
+	bool enc_support;
+	bool pps_override_support;
+	bool blk_prediction_support;
+	u8 version_major;
+	u8 version_minor;
+	u8 color_fmt_mask;
+	u8 color_depth_mask;
+	u8 max_slices;;
+	u8 max_slice_width;
+	u8 line_buf_bit_depth;
+	u8 bpp_precision_incr;
+	int rc_buf_size;
+};
+
 struct intel_dp {
 	i915_reg_t output_reg;
 	u32 DP;
@@ -1415,6 +1430,7 @@  struct intel_dp {
 	bool hobl_active;
 
 	struct intel_dp_pcon_frl frl;
+	struct intel_dp_pcon_dsc pcon_dsc;
 };
 
 enum lspcon_vendor {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e6c4cb844e37..b4f8abaea607 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3882,6 +3882,182 @@  cpt_set_link_train(struct intel_dp *intel_dp,
 	intel_de_posting_read(dev_priv, intel_dp->output_reg);
 }
 
+void intel_dp_get_pcon_dsc_cap(struct intel_dp *intel_dp)
+{
+	u8 buf;
+	u8 rc_buf_blk_size;
+	u8 max_slices = 0;
+
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	struct intel_dp_pcon_dsc *pcon_dsc = &intel_dp->pcon_dsc;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENCODER, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_ENCODER\n");
+		return;
+	}
+	pcon_dsc->enc_support = buf & DP_PCON_DSC_ENCODER_SUPPORTED;
+	pcon_dsc->pps_override_support = buf & DP_PCON_DSC_PPS_ENC_OVERRIDE;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_VERSION, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_VERSION\n");
+		return;
+	}
+	pcon_dsc->version_major = (buf & DP_PCON_DSC_MAJOR_MASK) >>
+				  DP_PCON_DSC_MAJOR_SHIFT;
+	pcon_dsc->version_minor = (buf & DP_PCON_DSC_MINOR_MASK) >>
+				  DP_PCON_DSC_MINOR_SHIFT;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_RC_BUF_BLK_INFO, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_RC_BUF_BLK_INFO\n");
+		return;
+	}
+
+	switch (buf & DP_PCON_DSC_RC_BUF_BLK_SIZE) {
+	case DP_PCON_DSC_RC_BUF_BLK_1KB :
+		rc_buf_blk_size = 1;
+		break;
+	case DP_PCON_DSC_RC_BUF_BLK_4KB :
+		rc_buf_blk_size = 4;
+		break;
+	case DP_PCON_DSC_RC_BUF_BLK_16KB :
+		rc_buf_blk_size = 16;
+		break;
+	case DP_PCON_DSC_RC_BUF_BLK_64KB :
+		rc_buf_blk_size = 64;
+		break;
+	default :
+		rc_buf_blk_size = 0;
+	}
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_RC_BUF_SIZE, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_RC_BUF_SIZE\n");
+		return;
+	}
+	/* storing rc_buf_size in bytes */
+	pcon_dsc->rc_buf_size = (buf + 1) * rc_buf_blk_size * 1024;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_SLICE_CAP_2, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_SLICE_CAP_2\n");
+		return;
+	}
+	if (buf & DP_PCON_DSC_24_PER_DSC_ENC)
+	       max_slices = 24;
+	else if (buf & DP_PCON_DSC_20_PER_DSC_ENC)
+		max_slices = 20;
+	else if (buf & DP_PCON_DSC_16_PER_DSC_ENC)
+		max_slices = 16;
+
+	if (max_slices == 0) {
+		if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_SLICE_CAP_1,
+				      &buf) < 0) {
+			drm_err(&i915->drm, "Failed to read DP_PCON_DSC_SLICE_CAP_2\n");
+			return;
+		}
+
+		if (buf & DP_PCON_DSC_12_PER_DSC_ENC)
+			max_slices = 12;
+		else if (buf & DP_PCON_DSC_10_PER_DSC_ENC)
+			max_slices = 10;
+		else if (buf & DP_PCON_DSC_8_PER_DSC_ENC)
+			max_slices = 8;
+		else if (buf & DP_PCON_DSC_6_PER_DSC_ENC)
+			max_slices = 6;
+		else if (buf & DP_PCON_DSC_4_PER_DSC_ENC)
+			max_slices = 4;
+		else if (buf & DP_PCON_DSC_2_PER_DSC_ENC)
+			max_slices = 2;
+		else if (buf & DP_PCON_DSC_1_PER_DSC_ENC)
+			max_slices = 1;
+	}
+
+	pcon_dsc->max_slices = max_slices;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BUF_BIT_DEPTH, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_BUF_BIT_DEPTH\n");
+		return;
+	}
+	switch (buf & DP_PCON_DSC_BIT_DEPTH_MASK) {
+	case DP_PCON_DSC_DEPTH_8_BITS :
+		pcon_dsc->line_buf_bit_depth = 8;
+		break;
+	case DP_PCON_DSC_DEPTH_9_BITS :
+		pcon_dsc->line_buf_bit_depth = 9;
+		break;
+	case DP_PCON_DSC_DEPTH_10_BITS :
+		pcon_dsc->line_buf_bit_depth = 10;
+		break;
+	case DP_PCON_DSC_DEPTH_11_BITS :
+		pcon_dsc->line_buf_bit_depth = 11;
+		break;
+	case DP_PCON_DSC_DEPTH_12_BITS :
+		pcon_dsc->line_buf_bit_depth = 12;
+		break;
+	case DP_PCON_DSC_DEPTH_13_BITS :
+		pcon_dsc->line_buf_bit_depth = 13;
+		break;
+	case DP_PCON_DSC_DEPTH_14_BITS :
+		pcon_dsc->line_buf_bit_depth = 14;
+		break;
+	case DP_PCON_DSC_DEPTH_15_BITS :
+		pcon_dsc->line_buf_bit_depth = 15;
+		break;
+	case DP_PCON_DSC_DEPTH_16_BITS :
+		pcon_dsc->line_buf_bit_depth = 16;
+		break;
+	default :
+		pcon_dsc->line_buf_bit_depth = 0;
+	}
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BLOCK_PREDICTION, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_BLOCK_PREDICTION\n");
+		return;
+	}
+	if (buf && DP_PCON_DSC_BLOCK_PRED_SUPPORT)
+		pcon_dsc->blk_prediction_support = true;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENC_COLOR_FMT_CAP, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_ENC_COLOR_FMT_CAP\n");
+		return;
+	}
+	pcon_dsc->color_fmt_mask = buf;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_ENC_COLOR_DEPTH_CAP, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_ENC_COLOR_DEPTH_CAP\n");
+		return;
+	}
+	pcon_dsc->color_depth_mask = buf;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_MAX_SLICE_WIDTH, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_MAX_SLICE_WIDTH\n");
+		return;
+	}
+	pcon_dsc->max_slice_width = buf;;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PCON_DSC_BPP_INCR, &buf) < 0) {
+		drm_err(&i915->drm, "Failed to read DP_PCON_DSC_BPP_INCR\n");
+		return;
+	}
+	switch(buf & DP_PCON_DSC_BPP_INCR_MASK) {
+	case DP_PCON_DSC_ONE_16TH_BPP:
+		pcon_dsc->bpp_precision_incr = 16;
+		break;
+	case DP_PCON_DSC_ONE_8TH_BPP:
+		pcon_dsc->bpp_precision_incr = 8;
+		break;
+	case DP_PCON_DSC_ONE_4TH_BPP:
+		pcon_dsc->bpp_precision_incr = 4;
+		break;
+	case DP_PCON_DSC_ONE_HALF_BPP:
+		pcon_dsc->bpp_precision_incr = 2;
+		break;
+	case DP_PCON_DSC_ONE_BPP:
+		pcon_dsc->bpp_precision_incr = 1;
+		break;
+	default :
+		pcon_dsc->bpp_precision_incr = 0;
+	}
+}
+
 static int intel_dp_get_max_rate_gbps(struct intel_dp *intel_dp)
 {
 	int max_link_clock, max_lanes, max_rate_khz, max_rate_gbps;
@@ -6659,6 +6835,8 @@  intel_dp_update_dfp(struct intel_dp *intel_dp,
 		    intel_dp->dfp.max_tmds_clock,
 		    intel_dp->dfp.pcon_max_frl,
 		    intel_dp->dfp.sink_max_frl);
+
+	intel_dp_get_pcon_dsc_cap(intel_dp);
 }
 
 static void