Re: [PATCH v10 1/8] drm/i915: Introduce CRTC output format

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

On Tue, Aug 14, 2018 at 03:23:19PM +0530, Shashank Sharma wrote:
> This patch adds an enum "intel_output_format" to represent
> the output format of a particular CRTC. This enum will be
> used to produce a RGB/YCBCR4:4:4/YCBCR4:2:0 output format
> during the atomic modeset calculations.
> 
> V5:
> - Created this separate patch to introduce and init output_format.
> - Initialize parameters of output_format_str respectively (Jani N).
> - Call it intel_output_format than crtc_output_format(Ville).
> - Set output format in pipe_config for every encoder (Ville).
> - Get rid of extra DRM_DEBUG_KMS during get_pipe_config (Ville)
> 
> V6: Rebase
> V7: Fixed alignment warnings (checkpatch)
> V8: Another check[atch warning for alignment
> V9: Rebase
> V10: Rebase on top of DSI restructure
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c      |  1 +
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  1 +
>  drivers/gpu/drm/i915/intel_drv.h     |  8 ++++++++
>  drivers/gpu/drm/i915/intel_dvo.c     |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c    |  1 +
>  drivers/gpu/drm/i915/intel_lvds.c    |  2 ++
>  drivers/gpu/drm/i915/intel_sdvo.c    |  1 +
>  drivers/gpu/drm/i915/intel_tv.c      |  1 +
>  drivers/gpu/drm/i915/vlv_dsi.c       |  1 +
>  11 files changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 0c6bf82..e85963c 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -354,6 +354,7 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	return true;
>  }
>  
> @@ -368,6 +369,7 @@ static bool pch_crt_compute_config(struct intel_encoder *encoder,
>  		return false;
>  
>  	pipe_config->has_pch_encoder = true;
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	return true;
>  }
> @@ -389,6 +391,7 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
>  		return false;
>  
>  	pipe_config->has_pch_encoder = true;
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	/* LPT FDI RX only supports 8bpc. */
>  	if (HAS_PCH_LPT(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cd5f1e75..5e5bc06 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9535,6 +9535,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		}
>  	}
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;

Looks like we're missing this for pre-hsw.

With that fixed
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>  	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>  		power_domain_mask |= BIT_ULL(power_domain);
> @@ -10882,6 +10883,18 @@ static void snprintf_output_types(char *buf, size_t len,
>  	WARN_ON_ONCE(output_types != 0);
>  }
>  
> +static const char * const output_format_str[] = {
> +	[INTEL_OUTPUT_FORMAT_INVALID] = "Invalid",
> +	[INTEL_OUTPUT_FORMAT_RGB] = "RGB",
> +};
> +
> +static const char *output_formats(enum intel_output_format format)
> +{
> +	if (format != INTEL_OUTPUT_FORMAT_RGB)
> +		format = INTEL_OUTPUT_FORMAT_INVALID;
> +	return output_format_str[format];
> +}
> +
>  static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config,
>  				   const char *context)
> @@ -10901,6 +10914,9 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("output_types: %s (0x%x)\n",
>  		      buf, pipe_config->output_types);
>  
> +	DRM_DEBUG_KMS("output format: %s\n",
> +		      output_formats(pipe_config->output_format));
> +
>  	DRM_DEBUG_KMS("cpu_transcoder: %s, pipe bpp: %i, dithering: %i\n",
>  		      transcoder_name(pipe_config->cpu_transcoder),
>  		      pipe_config->pipe_bpp, pipe_config->dither);
> @@ -11490,6 +11506,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end);
>  
>  	PIPE_CONF_CHECK_I(pixel_multiplier);
> +	PIPE_CONF_CHECK_I(output_format);
>  	PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
>  	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
>  	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8e0e14b..5d907a2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2011,6 +2011,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>  		pipe_config->has_pch_encoder = true;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->has_drrs = false;
>  	if (IS_G4X(dev_priv) || port == PORT_A)
>  		pipe_config->has_audio = false;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e521..04da555 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -51,6 +51,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->has_pch_encoder = false;
>  	bpp = 24;
>  	if (intel_dp->compliance.test_data.bpc) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7b984ae..f704720 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -708,6 +708,11 @@ struct intel_crtc_wm_state {
>  	bool need_postvbl_update;
>  };
>  
> +enum intel_output_format {
> +	INTEL_OUTPUT_FORMAT_INVALID,
> +	INTEL_OUTPUT_FORMAT_RGB,
> +};
> +
>  struct intel_crtc_state {
>  	struct drm_crtc_state base;
>  
> @@ -897,6 +902,9 @@ struct intel_crtc_state {
>  
>  	/* output format is YCBCR 4:2:0 */
>  	bool ycbcr420;
> +
> +	/* Output format RGB/YCBCR etc */
> +	enum intel_output_format output_format;
>  };
>  
>  struct intel_crtc {
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 4e142ff..5bb4ba9 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -256,6 +256,7 @@ static bool intel_dvo_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8363fbd..8993e66 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1699,6 +1699,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->has_hdmi_sink = !force_dvi && intel_hdmi->has_hdmi_sink;
>  
>  	if (pipe_config->has_hdmi_sink)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index f9f3b08..9755c9e 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -418,6 +418,8 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  		pipe_config->pipe_bpp = lvds_bpp;
>  	}
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +
>  	/*
>  	 * We have timings from the BIOS for the panel, put them in
>  	 * to the adjusted mode.  The CRTC will be set up for this mode,
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 812fe7b..005ca08 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1132,6 +1132,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  
>  	DRM_DEBUG_KMS("forcing bpc to 8 for SDVO\n");
>  	pipe_config->pipe_bpp = 8*3;
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	if (HAS_PCH_SPLIT(to_i915(encoder->base.dev)))
>  		pipe_config->has_pch_encoder = true;
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index b5b04cb..5f5e3f8 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -885,6 +885,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	adjusted_mode->crtc_clock = tv_mode->clock;
>  	DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
>  	pipe_config->pipe_bpp = 8*3;
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index 435a2c3..552c876f 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -314,6 +314,7 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  	int ret;
>  
>  	DRM_DEBUG_KMS("\n");
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	if (fixed_mode) {
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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