[Intel-gfx] [PATCH v3] drm/i915/display: Fix the 12 BPC bits for PIPE_MISC reg

[Shankar, Uma]

> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>
> Sent: Wednesday, August 11, 2021 10:49 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar at intel.com>; Zanoni, Paulo R
> <paulo.r.zanoni at intel.com>; ville.syrjala at linux.intel.com; daniel.vetter at ffwll.ch;
> jani.nikula at linux.intel.com; joonas.lahtinen at linux.intel.com; Vivi, Rodrigo
> <rodrigo.vivi at intel.com>; stable at vger.kernel.org; Nautiyal, Ankit K
> <ankit.k.nautiyal at intel.com>
> Subject: [PATCH v3] drm/i915/display: Fix the 12 BPC bits for PIPE_MISC reg
> 
> Till DISPLAY12 the PIPE_MISC bits 5-7 are used to set the Dithering BPC, with valid
> values of 6, 8, 10 BPC.
> For ADLP+ these bits are used to set the PORT OUTPUT BPC, with valid values of: 6,
> 8, 10, 12 BPC, and need to be programmed whether dithering is enabled or not.
> 
> This patch:
> -corrects the bits 5-7 for PIPE MISC register for 12 BPC.
> -renames the bits and mask to have generic names for these bits for dithering bpc
> and port output bpc.
> 
> v3: Added a note for MIPI DSI which uses the PIPE_MISC for readout for pipe_bpp.
> (Uma Shankar)
> 
> v2: Added 'display' to the subject and fixes tag. (Uma Shankar)
> 

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

> Fixes: 756f85cffef2 ("drm/i915/bdw: Broadwell has PIPEMISC")
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com> (v1)
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Cc: <stable at vger.kernel.org> # v3.13+
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 34 ++++++++++++++------
>  drivers/gpu/drm/i915/i915_reg.h              | 16 ++++++---
>  2 files changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index b25c596f6f7e..a257e5dc381c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5838,16 +5838,18 @@ static void bdw_set_pipemisc(const struct
> intel_crtc_state *crtc_state)
> 
>  	switch (crtc_state->pipe_bpp) {
>  	case 18:
> -		val |= PIPEMISC_DITHER_6_BPC;
> +		val |= PIPEMISC_6_BPC;
>  		break;
>  	case 24:
> -		val |= PIPEMISC_DITHER_8_BPC;
> +		val |= PIPEMISC_8_BPC;
>  		break;
>  	case 30:
> -		val |= PIPEMISC_DITHER_10_BPC;
> +		val |= PIPEMISC_10_BPC;
>  		break;
>  	case 36:
> -		val |= PIPEMISC_DITHER_12_BPC;
> +		/* Port output 12BPC defined for ADLP+ */
> +		if (DISPLAY_VER(dev_priv) > 12)
> +			val |= PIPEMISC_12_BPC_ADLP;
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->pipe_bpp);
> @@ -5900,15 +5902,27 @@ int bdw_get_pipemisc_bpp(struct intel_crtc *crtc)
> 
>  	tmp = intel_de_read(dev_priv, PIPEMISC(crtc->pipe));
> 
> -	switch (tmp & PIPEMISC_DITHER_BPC_MASK) {
> -	case PIPEMISC_DITHER_6_BPC:
> +	switch (tmp & PIPEMISC_BPC_MASK) {
> +	case PIPEMISC_6_BPC:
>  		return 18;
> -	case PIPEMISC_DITHER_8_BPC:
> +	case PIPEMISC_8_BPC:
>  		return 24;
> -	case PIPEMISC_DITHER_10_BPC:
> +	case PIPEMISC_10_BPC:
>  		return 30;
> -	case PIPEMISC_DITHER_12_BPC:
> -		return 36;
> +	/*
> +	 * PORT OUTPUT 12 BPC defined for ADLP+.
> +	 *
> +	 * TODO:
> +	 * For previous platforms with DSI interface, bits 5:7
> +	 * are used for storing pipe_bpp irrespective of dithering.
> +	 * Since the value of 12 BPC is not defined for these bits
> +	 * on older platforms, need to find a workaround for 12 BPC
> +	 * MIPI DSI HW readout.
> +	 */
> +	case PIPEMISC_12_BPC_ADLP:
> +		if (DISPLAY_VER(dev_priv) > 12)
> +			return 36;
> +		fallthrough;
>  	default:
>  		MISSING_CASE(tmp);
>  		return 0;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 167eaa87501b..664970f2bc62 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6203,11 +6203,17 @@ enum {
>  #define   PIPEMISC_HDR_MODE_PRECISION	(1 << 23) /* icl+ */
>  #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
>  #define   PIPEMISC_PIXEL_ROUNDING_TRUNC	REG_BIT(8) /* tgl+ */
> -#define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
> -#define   PIPEMISC_DITHER_8_BPC		(0 << 5)
> -#define   PIPEMISC_DITHER_10_BPC	(1 << 5)
> -#define   PIPEMISC_DITHER_6_BPC		(2 << 5)
> -#define   PIPEMISC_DITHER_12_BPC	(3 << 5)
> +/*
> + * For Display < 13, Bits 5-7 of PIPE MISC represent DITHER BPC with
> + * valid values of: 6, 8, 10 BPC.
> + * ADLP+, the bits 5-7 represent PORT OUTPUT BPC with valid values of:
> + * 6, 8, 10, 12 BPC.
> + */
> +#define   PIPEMISC_BPC_MASK		(7 << 5)
> +#define   PIPEMISC_8_BPC		(0 << 5)
> +#define   PIPEMISC_10_BPC		(1 << 5)
> +#define   PIPEMISC_6_BPC		(2 << 5)
> +#define   PIPEMISC_12_BPC_ADLP		(4 << 5) /* adlp+ */
>  #define   PIPEMISC_DITHER_ENABLE	(1 << 4)
>  #define   PIPEMISC_DITHER_TYPE_MASK	(3 << 2)
>  #define   PIPEMISC_DITHER_TYPE_SP	(0 << 2)
> --
> 2.25.1