Re: [Intel-gfx] [PATCH v10 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0
On Tue, Aug 14, 2018 at 03:23:20PM +0530, Shashank Sharma wrote:
> Currently, we are using a bool in CRTC state (state->ycbcr420),
> to indicate modeset, that the output format is YCBCR 4:2:0. Now in
> order to support other YCBCR formats, we will need more such flags.
>
> This patch adds a new enum parameter for YCBCR 4:2:0 outputs, in the
> CRTC output formats and then plugs it during the modeset.
>
> V3: Added this patch in the series, to address review comments from
> second patchset.
> V4: Added r-b from Maarten (on v3)
> Addressed review comments from Ville:
> - Change the enum name to intel_output_format.
> - Start the enum value (INVALID) from 0 instaed of 1.
> - Set the crtc's output_format to RGB in encoder's compute_config.
> V5: Broke previous patch 1 into two parts,
> - first patch to add CRTC output format in general
> - second patch (this one) to add YCBCR 4:2:0 output
> format specifically.
> - Use ARRAY_SIZE(format_str) for output format validity check (Ville)
> V6: Added a separate function to calculate crtc_state->output_format, and
> calling it from various get_config function (Fix CI build warning)
> V7: Fixed checkpatch warnings for alignment
> V8: Rebase
> V9: Rebase
> V10: Rebase
>
> Cc: Ville Syrjala
> Cc: Maarten Lankhorst
> Reviewed-by: Maarten Lankhorst
> Signed-off-by: Shashank Sharma
> ---
> drivers/gpu/drm/i915/intel_color.c | 2 +-
> drivers/gpu/drm/i915/intel_ddi.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 72
> +---
> drivers/gpu/drm/i915/intel_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_hdmi.c| 6 +--
> drivers/gpu/drm/i915/intel_panel.c | 2 +-
> 6 files changed, 50 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c
> b/drivers/gpu/drm/i915/intel_color.c
> index c6a7bea..bf9d8f6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -149,7 +149,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
> *crtc_state)
> if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> limited_color_range = intel_crtc_state->limited_color_range;
>
> - if (intel_crtc_state->ycbcr420) {
> + if (intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
> ilk_load_ycbcr_conversion_matrix(intel_crtc);
> return;
> } else if (crtc_state->ctm) {
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0adc043..a036fe6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1442,7 +1442,7 @@ static void ddi_dotclock_get(struct intel_crtc_state
> *pipe_config)
> else
> dotclock = pipe_config->port_clock;
>
> - if (pipe_config->ycbcr420)
> + if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> dotclock *= 2;
>
> if (pipe_config->pixel_multiplier)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5e5bc06..e2a1e4f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4811,7 +4811,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
> bool force_detach,
> if (pixel_format == DRM_FORMAT_NV12)
> need_scaling = true;
>
> - if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> + scaler_user == SKL_CRTC_INDEX)
> need_scaling = true;
>
> /*
> @@ -6620,7 +6621,8 @@ static int intel_crtc_compute_config(struct intel_crtc
> *crtc,
> return -EINVAL;
> }
>
> - if (pipe_config->ycbcr420 && pipe_config->base.ctm) {
> + if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> + pipe_config->base.ctm) {
> /*
>* There is only one pipe CSC unit per pipe, and we need that
>* for output conversion from RGB->YCBCR. So if CTM is already
> @@ -7818,6 +7820,35 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
> pipe_config->port_clock = chv_calc_dpll_params(refclk, );
> }
>
> +static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
> + struct intel_crtc_state *pipe_config)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
> +
> + 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)
[Intel-gfx] [PATCH v10 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0
Currently, we are using a bool in CRTC state (state->ycbcr420),
to indicate modeset, that the output format is YCBCR 4:2:0. Now in
order to support other YCBCR formats, we will need more such flags.
This patch adds a new enum parameter for YCBCR 4:2:0 outputs, in the
CRTC output formats and then plugs it during the modeset.
V3: Added this patch in the series, to address review comments from
second patchset.
V4: Added r-b from Maarten (on v3)
Addressed review comments from Ville:
- Change the enum name to intel_output_format.
- Start the enum value (INVALID) from 0 instaed of 1.
- Set the crtc's output_format to RGB in encoder's compute_config.
V5: Broke previous patch 1 into two parts,
- first patch to add CRTC output format in general
- second patch (this one) to add YCBCR 4:2:0 output
format specifically.
- Use ARRAY_SIZE(format_str) for output format validity check (Ville)
V6: Added a separate function to calculate crtc_state->output_format, and
calling it from various get_config function (Fix CI build warning)
V7: Fixed checkpatch warnings for alignment
V8: Rebase
V9: Rebase
V10: Rebase
Cc: Ville Syrjala
Cc: Maarten Lankhorst
Reviewed-by: Maarten Lankhorst
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/intel_color.c | 2 +-
drivers/gpu/drm/i915/intel_ddi.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 72 +---
drivers/gpu/drm/i915/intel_drv.h | 4 +-
drivers/gpu/drm/i915/intel_hdmi.c| 6 +--
drivers/gpu/drm/i915/intel_panel.c | 2 +-
6 files changed, 50 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index c6a7bea..bf9d8f6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -149,7 +149,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
*crtc_state)
if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
limited_color_range = intel_crtc_state->limited_color_range;
- if (intel_crtc_state->ycbcr420) {
+ if (intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
ilk_load_ycbcr_conversion_matrix(intel_crtc);
return;
} else if (crtc_state->ctm) {
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0adc043..a036fe6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1442,7 +1442,7 @@ static void ddi_dotclock_get(struct intel_crtc_state
*pipe_config)
else
dotclock = pipe_config->port_clock;
- if (pipe_config->ycbcr420)
+ if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
dotclock *= 2;
if (pipe_config->pixel_multiplier)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e5bc06..e2a1e4f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4811,7 +4811,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
if (pixel_format == DRM_FORMAT_NV12)
need_scaling = true;
- if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+ scaler_user == SKL_CRTC_INDEX)
need_scaling = true;
/*
@@ -6620,7 +6621,8 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
return -EINVAL;
}
- if (pipe_config->ycbcr420 && pipe_config->base.ctm) {
+ if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+ pipe_config->base.ctm) {
/*
* There is only one pipe CSC unit per pipe, and we need that
* for output conversion from RGB->YCBCR. So if CTM is already
@@ -7818,6 +7820,35 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
pipe_config->port_clock = chv_calc_dpll_params(refclk, );
}
+static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
+ struct intel_crtc_state *pipe_config)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
+
+ 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;
+
