On Thu, 2019-07-18 at 17:50 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Make intel_get_crtc_ycbcr_config() simpler and rename it
> to bdw_get_pipemisc_output_format() to better reflect what
> it does.
>
> Also toss in some comments to document that the 4:2:0 PIPECONF
> bits are glk+ only. They are mbz on earlier platforms so reading
> them unconditionally is safe however.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 71 +++++++++---------
> --
> drivers/gpu/drm/i915/i915_reg.h | 4 +-
> 2 files changed, 34 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index ffdc350dc24a..1dd1aa29a649 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8713,47 +8713,24 @@ static void chv_crtc_clock_get(struct
> intel_crtc *crtc,
> pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock);
> }
>
> -static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
> - struct intel_crtc_state
> *pipe_config)
> +static enum intel_output_format
> +bdw_get_pipemisc_output_format(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
> -
> - pipe_config->lspcon_downsampling = false;
> -
> - if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
> - u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
> -
> - if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
> - bool ycbcr420_enabled = tmp &
> PIPEMISC_YUV420_ENABLE;
> - bool blend = tmp &
> PIPEMISC_YUV420_MODE_FULL_BLEND;
> -
> - if (ycbcr420_enabled) {
> - /* We support 4:2:0 in full blend mode
> only */
> - if (!blend)
> - output =
> INTEL_OUTPUT_FORMAT_INVALID;
> - else if (!(IS_GEMINILAKE(dev_priv) ||
> - INTEL_GEN(dev_priv) >= 10))
> - output =
> INTEL_OUTPUT_FORMAT_INVALID;
> - else
> - output =
> INTEL_OUTPUT_FORMAT_YCBCR420;
> - } else {
> - /*
> - * Currently there is no interface
> defined to
> - * check user preference between
> RGB/YCBCR444
> - * or YCBCR420. So the only possible
> case for
> - * YCBCR444 usage is driving YCBCR420
> output
> - * with LSPCON, when pipe is configured
> for
> - * YCBCR444 output and LSPCON takes
> care of
> - * downsampling it.
> - */
> - pipe_config->lspcon_downsampling =
> true;
> - output = INTEL_OUTPUT_FORMAT_YCBCR444;
> - }
> - }
> - }
> + u32 tmp;
> +
> + tmp = I915_READ(PIPEMISC(crtc->pipe));
>
> - pipe_config->output_format = output;
> + if (tmp & PIPEMISC_YUV420_ENABLE) {
> + /* We support 4:2:0 in full blend mode only */
> + WARN_ON((tmp & PIPEMISC_YUV420_MODE_FULL_BLEND) == 0);
> +
> + return INTEL_OUTPUT_FORMAT_YCBCR420;
> + } else if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
> + return INTEL_OUTPUT_FORMAT_YCBCR444;
> + } else {
> + return INTEL_OUTPUT_FORMAT_RGB;
> + }
> }
>
> static void i9xx_get_pipe_color_config(struct intel_crtc_state
> *crtc_state)
> @@ -10445,7 +10422,23 @@ static bool haswell_get_pipe_config(struct
> intel_crtc *crtc,
> }
>
> intel_get_pipe_src_size(crtc, pipe_config);
> - intel_get_crtc_ycbcr_config(crtc, pipe_config);
> +
> + if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
> + pipe_config->output_format =
> + bdw_get_pipemisc_output_format(crtc);
> +
> + /*
> + * Currently there is no interface defined to
> + * check user preference between RGB/YCBCR444
> + * or YCBCR420. So the only possible case for
> + * YCBCR444 usage is driving YCBCR420 output
> + * with LSPCON, when pipe is configured for
> + * YCBCR444 output and LSPCON takes care of
> + * downsampling it.
> + */
> + pipe_config->lspcon_downsampling =
> + pipe_config->output_format ==
> INTEL_OUTPUT_FORMAT_YCBCR444;
> + }
>
> pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe));
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 91bf714897e5..66f7f417231f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5803,8 +5803,8 @@ enum {
>
> #define _PIPE_MISC_A 0x70030
> #define _PIPE_MISC_B 0x71030
> -#define PIPEMISC_YUV420_ENABLE (1 << 27)
> -#define PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
> +#define PIPEMISC_YUV420_ENABLE (1 << 27) /* glk+ */
> +#define PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26) /* glk+ */
> #define PIPEMISC_HDR_MODE_PRECISION (1 << 23) /* icl+ */
> #define PIPEMISC_OUTPUT_COLORSPACE_YUV (1 << 11)
> #define PIPEMISC_DITHER_BPC_MASK (7 << 5)
The changes look good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
