Re: [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output

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

On Thu, Jul 13, 2017 at 09:03:17PM +0530, Shashank Sharma wrote:
> To get HDMI YCBCR420 output, the PIPEMISC register should be
> programmed to:
> - Generate YCBCR output (bit 11)
> - In case of YCBCR420 outputs, it should be programmed in full
>   blend mode to use the scaler in 5x3 ratio (bits 26 and 27)
> 
> This patch:
> - Adds definition of these bits.
> - Programs PIPEMISC for YCBCR420 outputs.
> 
> V2: rebase
> V3: rebase
> V4: rebase
> V5: added r-b from Ander
> V6: Handle only YCBCR420 outputs (ville)
> V7: rebase
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> 
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>  drivers/gpu/drm/i915/intel_display.c | 7 +++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c712d01..e5020d6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5227,6 +5227,9 @@ enum {
>  
>  #define _PIPE_MISC_A			0x70030
>  #define _PIPE_MISC_B			0x71030
> +#define   PIPEMISC_YCBCR420_ENABLE	(1<<27)
> +#define   PIPEMISC_YCBCR420_MODE_BLEND	(1<<26)
> +#define   PIPEMISC_OUTPUT_YCBCR		(1<<11)

Please rename to match spec. So something like:
PIPEMISC_YUV420_ENABLE       (1<<27)
PIPEMISC_YUV420_MODE_FULL_BLEND      (1<<26)
PIPEMISC_OUTPUT_COLORSPACE_YUV       (1<<11)

>  #define   PIPEMISC_DITHER_BPC_MASK	(7<<5)
>  #define   PIPEMISC_DITHER_8_BPC		(0<<5)
>  #define   PIPEMISC_DITHER_10_BPC	(1<<5)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d78f1c2..1a23ec0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *config = intel_crtc->config;
>  
>  	if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
>  		u32 val = 0;
> @@ -8101,6 +8102,12 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  		if (intel_crtc->config->dither)
>  			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
>  
> +		if (config->ycbcr420) {
> +			val |= PIPEMISC_OUTPUT_YCBCR |
> +				PIPEMISC_YCBCR420_ENABLE |
> +				PIPEMISC_YCBCR420_MODE_BLEND;
> +		}

I think we'll want two flags. One to specify whether we're outputting
YCbCr and the other to indicate whether we need the 4:2:0 subsamling.
So maybe something like
bool ycbcr;
bool ycbcr420;

We also need state readout for this stuff. With those two flags I think
we can do something like:

if (IS_BDW || GEN >= 9) {
	tmp = READ(PIPEMISC);

	crtc_state->ycbcr = tmp & OUTPUT_YUV;
	
 	if (IS_GLK || GEN >= 10)
		crtc_state->ycbcr420 = tmp & YUV420_ENABLE;
}

The other missing readout thing is adjustment of the clock.
ddi_dotclock_get() will need to double the dotclock when we're
outputting ycbcr420.

Pls also add something along the lines of 
DRM_DEBUG_KMS("ycbcr: %i, ycbcr420: %i\n", ...);
to intel_dump_pipe_config() so that we can actually tell when we're
outputting YCbCr and 4:2:0.


> +
>  		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
>  	}
>  }
> -- 
> 2.7.4

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