* [PATCH v12 1/8] drm/i915: Introduce CRTC output format
@ 2018-10-12 6:23 Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
` (9 more replies)
0 siblings, 10 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
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
V11: Addressed review comment from Ville
- Set CRTC format for pre-HSW get_pipe_config() function too.
Added Ville's R-B
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_crt.c | 3 +++
drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
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, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index ab3d6b0..68f2fb8 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 980f4ea..7e7742a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7800,6 +7800,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
return false;
+ pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
pipe_config->shared_dpll = NULL;
@@ -8849,6 +8850,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
return false;
+ pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
pipe_config->shared_dpll = NULL;
@@ -9504,6 +9506,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
}
}
+ pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
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);
@@ -10919,6 +10922,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)
@@ -10938,6 +10953,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);
@@ -11527,6 +11545,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 13ff89b..c3f6330 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2085,6 +2085,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 bb6b8f0..b268bdd 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 3dea7a1..ff44fad 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -712,6 +712,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;
@@ -901,6 +906,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 be3c0a5..0042a7f 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 2c53efc..21be074 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1698,6 +1698,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 510585e..e6c5d98 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -409,6 +409,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 1824d94..6151d98 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1123,6 +1123,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 8b9ce0d..860f306 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 5accd0c..bafeb2a 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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4 Shashank Sharma
` (8 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
From: Shashank Sharma <shashank.sharma@linux.intel.com>
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
V11: Addressed review comments from Ville:
- Change check for CRTC output format from > ARRAY_SIZE to >= ARRAY_SIZE.
- Check for values < INTEL_OUTPUT_FORMAT_RGB is unnecessary.
- No need to get CRTC YCBCR config, for pre-BDW functions.
Added Ville's r-b.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 2 +-
drivers/gpu/drm/i915/intel_ddi.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 67 +++++++++++++++++++++---------------
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, 46 insertions(+), 37 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 47960c9..7d868f5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1517,7 +1517,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 7e7742a..b5c5dbb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4839,7 +4839,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;
/*
@@ -6590,7 +6591,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
@@ -7788,6 +7790,35 @@ 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)
+{
+ 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;
+ else if (!(IS_GEMINILAKE(dev_priv) ||
+ INTEL_GEN(dev_priv) >= 10))
+ output = INTEL_OUTPUT_FORMAT_INVALID;
+ else
+ output = INTEL_OUTPUT_FORMAT_YCBCR420;
+ }
+ }
+ }
+
+ pipe_config->output_format = output;
+}
+
static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config)
{
@@ -8422,9 +8453,9 @@ static void haswell_set_pipemisc(const struct intel_crtc_state *crtc_state)
if (crtc_state->dither)
val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
- if (crtc_state->ycbcr420) {
- val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
- PIPEMISC_YUV420_ENABLE |
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+ val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
+ val |= PIPEMISC_YUV420_ENABLE |
PIPEMISC_YUV420_MODE_FULL_BLEND;
}
@@ -9485,28 +9516,11 @@ 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);
pipe_config->gamma_mode =
I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
- if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
- u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
- bool clrspace_yuv = tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV;
-
- if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) {
- bool blend_mode_420 = tmp &
- PIPEMISC_YUV420_MODE_FULL_BLEND;
-
- pipe_config->ycbcr420 = tmp & PIPEMISC_YUV420_ENABLE;
- if (pipe_config->ycbcr420 != clrspace_yuv ||
- pipe_config->ycbcr420 != blend_mode_420)
- DRM_DEBUG_KMS("Bad 4:2:0 mode (%08x)\n", tmp);
- } else if (clrspace_yuv) {
- DRM_DEBUG_KMS("YCbCr 4:2:0 Unsupported\n");
- }
- }
-
- pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
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);
@@ -10925,11 +10939,12 @@ static void snprintf_output_types(char *buf, size_t len,
static const char * const output_format_str[] = {
[INTEL_OUTPUT_FORMAT_INVALID] = "Invalid",
[INTEL_OUTPUT_FORMAT_RGB] = "RGB",
+ [INTEL_OUTPUT_FORMAT_YCBCR420] = "YCBCR4:2:0",
};
static const char *output_formats(enum intel_output_format format)
{
- if (format != INTEL_OUTPUT_FORMAT_RGB)
+ if (format >= ARRAY_SIZE(output_format_str))
format = INTEL_OUTPUT_FORMAT_INVALID;
return output_format_str[format];
}
@@ -10965,9 +10980,6 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
pipe_config->fdi_lanes,
&pipe_config->fdi_m_n);
- if (pipe_config->ycbcr420)
- DRM_DEBUG_KMS("YCbCr 4:2:0 output enabled\n");
-
if (intel_crtc_has_dp_encoder(pipe_config)) {
intel_dump_m_n_config(pipe_config, "dp m_n",
pipe_config->lane_count, &pipe_config->dp_m_n);
@@ -11554,7 +11566,6 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
- PIPE_CONF_CHECK_BOOL(ycbcr420);
PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ff44fad..01530fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -715,6 +715,7 @@ struct intel_crtc_wm_state {
enum intel_output_format {
INTEL_OUTPUT_FORMAT_INVALID,
INTEL_OUTPUT_FORMAT_RGB,
+ INTEL_OUTPUT_FORMAT_YCBCR420,
};
struct intel_crtc_state {
@@ -904,9 +905,6 @@ struct intel_crtc_state {
/* HDMI High TMDS char rate ratio */
bool hdmi_high_tmds_clock_ratio;
- /* output format is YCBCR 4:2:0 */
- bool ycbcr420;
-
/* Output format RGB/YCBCR etc */
enum intel_output_format output_format;
};
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 21be074..33e1a9a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -478,7 +478,7 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
return;
}
- if (crtc_state->ycbcr420)
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
else
frame.avi.colorspace = HDMI_COLORSPACE_RGB;
@@ -1619,7 +1619,7 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
if (connector_state->crtc != crtc_state->base.crtc)
continue;
- if (crtc_state->ycbcr420) {
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
const struct drm_hdmi_info *hdmi = &info->hdmi;
if (bpc == 12 && !(hdmi->y420_dc_modes &
@@ -1664,7 +1664,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
*clock_12bpc /= 2;
*clock_10bpc /= 2;
*clock_8bpc /= 2;
- config->ycbcr420 = true;
+ config->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
/* YCBCR 420 output conversion needs a scaler */
if (skl_update_scaler_crtc(config)) {
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 7df9bcd..20582cf 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -111,7 +111,7 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
/* Native modes don't need fitting */
if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
- !pipe_config->ycbcr420)
+ pipe_config->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
goto done;
switch (fitting_mode) {
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
` (7 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
From: Shashank Sharma <shashank.sharma@linux.intel.com>
This patch adds support for YCBCR 4:4:4 CRTC output format.
To do this, this patch extends the existing YCBCR 4:2:0
framework by:
- Adding new parameter in for YCBCR 4:4:4 enum crtc_iutput_format.
- Adding case for YCBCR 4:4:4 in while setting AVI infoframes.
- Adding necessary checks in modeset sequence.
V3: Added this patch in the series
V4: Added r-b from Maarten (for v3)
Addressed review comment from Ville:
Do not use (config->output_format > CRTC_OUTPUT_RGB)
V5: Rebase
V6: Rebase and small change, to accommodate changes in patch 2
V7: Fixed checkpatch alignment warnings
V8: Rebase
V9: Rebase
V10: Rebase
V11: Addressed review comment from Ville
Missing output_format_str[INTEL_OUTPUT_FORMAT_YCBCR444]
Added Ville's R-B.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 3 ++-
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++----
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
4 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index bf9d8f6..5127da2 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -149,7 +149,8 @@ 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->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+ if (intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+ intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
ilk_load_ycbcr_conversion_matrix(intel_crtc);
return;
} else if (crtc_state->ctm) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b5c5dbb..3317c6c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6591,8 +6591,9 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
return -EINVAL;
}
- if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
- pipe_config->base.ctm) {
+ if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+ pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
+ 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
@@ -7812,6 +7813,8 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
output = INTEL_OUTPUT_FORMAT_INVALID;
else
output = INTEL_OUTPUT_FORMAT_YCBCR420;
+ } else {
+ output = INTEL_OUTPUT_FORMAT_YCBCR444;
}
}
}
@@ -8453,11 +8456,13 @@ static void haswell_set_pipemisc(const struct intel_crtc_state *crtc_state)
if (crtc_state->dither)
val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
- if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+ crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
+
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
val |= PIPEMISC_YUV420_ENABLE |
PIPEMISC_YUV420_MODE_FULL_BLEND;
- }
I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
}
@@ -10940,6 +10945,7 @@ static const char * const output_format_str[] = {
[INTEL_OUTPUT_FORMAT_INVALID] = "Invalid",
[INTEL_OUTPUT_FORMAT_RGB] = "RGB",
[INTEL_OUTPUT_FORMAT_YCBCR420] = "YCBCR4:2:0",
+ [INTEL_OUTPUT_FORMAT_YCBCR444] = "YCBCR4:4:4",
};
static const char *output_formats(enum intel_output_format format)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 01530fa..3d23302 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -716,6 +716,7 @@ enum intel_output_format {
INTEL_OUTPUT_FORMAT_INVALID,
INTEL_OUTPUT_FORMAT_RGB,
INTEL_OUTPUT_FORMAT_YCBCR420,
+ INTEL_OUTPUT_FORMAT_YCBCR444,
};
struct intel_crtc_state {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 33e1a9a..e2cc5ed 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -480,6 +480,8 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
+ else if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
+ frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
else
frame.avi.colorspace = HDMI_COLORSPACE_RGB;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 4/8] drm/i915: Check LSPCON vendor OUI
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4 Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 5/8] drm/i915: Add AVI infoframe support for LSPCON Shashank Sharma
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
From: "Sharma, Shashank" <shashank.sharma@intel.com>
Intel LSPCON chip is provided by 2 vendors:
- Megachips America (MCA)
- Parade technologies (Parade tech)
Its important to know the vendor of this chip, as the address to
write AVI infoframes is different for those two.
This patch reads the vendor OUI signature, and marks into LSPCON
encoder structure for future usages.
This patch also does a small re-arrangement of the code, by moving
lspcon mode change into probe function.
V2: Use dp->desc for OUI detection, dont add a helper for this
(Ville)
V3: Rebase, Added r-b from Maarten
V4: Rebase
V5: Rebase
V6: Rebase
V7: Rebase
V8: Rebase
V9: Rebase
V10: Rebase
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 6 ++++
drivers/gpu/drm/i915/intel_lspcon.c | 69 +++++++++++++++++++++++++++++--------
2 files changed, 61 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3d23302..6981e72 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1162,9 +1162,15 @@ struct intel_dp {
struct intel_dp_compliance compliance;
};
+enum lspcon_vendor {
+ LSPCON_VENDOR_MCA,
+ LSPCON_VENDOR_PARADE
+};
+
struct intel_lspcon {
bool active;
enum drm_lspcon_mode mode;
+ enum lspcon_vendor vendor;
};
struct intel_digital_port {
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 3e085c5..56f80b7 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -27,6 +27,10 @@
#include <drm/drm_dp_dual_mode_helper.h>
#include "intel_drv.h"
+/* LSPCON OUI Vendor ID(signatures) */
+#define LSPCON_VENDOR_PARADE_OUI 0x001CF8
+#define LSPCON_VENDOR_MCA_OUI 0x0060AD
+
static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
{
struct intel_digital_port *dig_port =
@@ -50,6 +54,40 @@ static const char *lspcon_mode_name(enum drm_lspcon_mode mode)
}
}
+static bool lspcon_detect_vendor(struct intel_lspcon *lspcon)
+{
+ struct intel_dp *dp = lspcon_to_intel_dp(lspcon);
+ struct drm_dp_dpcd_ident *ident;
+ u32 vendor_oui;
+
+ if (drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd))) {
+ DRM_ERROR("Can't read description\n");
+ return false;
+ }
+
+ ident = &dp->desc.ident;
+ vendor_oui = (ident->oui[0] << 16) | (ident->oui[1] << 8) |
+ ident->oui[2];
+
+ switch (vendor_oui) {
+ case LSPCON_VENDOR_MCA_OUI:
+ lspcon->vendor = LSPCON_VENDOR_MCA;
+ DRM_DEBUG_KMS("Vendor: Mega Chips\n");
+ break;
+
+ case LSPCON_VENDOR_PARADE_OUI:
+ lspcon->vendor = LSPCON_VENDOR_PARADE;
+ DRM_DEBUG_KMS("Vendor: Parade Tech\n");
+ break;
+
+ default:
+ DRM_ERROR("Invalid/Unknown vendor OUI\n");
+ return false;
+ }
+
+ return true;
+}
+
static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
{
enum drm_lspcon_mode current_mode;
@@ -159,7 +197,18 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
/* Yay ... got a LSPCON device */
DRM_DEBUG_KMS("LSPCON detected\n");
lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
- lspcon->active = true;
+
+ /*
+ * In the SW state machine, lets Put LSPCON in PCON mode only.
+ * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
+ * 2.0 sinks.
+ */
+ if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
+ if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
+ DRM_ERROR("LSPCON mode change to PCON failed\n");
+ return false;
+ }
+ }
return true;
}
@@ -230,25 +279,17 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
return false;
}
- /*
- * In the SW state machine, lets Put LSPCON in PCON mode only.
- * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
- * 2.0 sinks.
- */
- if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
- if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
- DRM_ERROR("LSPCON mode change to PCON failed\n");
- return false;
- }
- }
-
if (!intel_dp_read_dpcd(dp)) {
DRM_ERROR("LSPCON DPCD read failed\n");
return false;
}
- drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
+ if (!lspcon_detect_vendor(lspcon)) {
+ DRM_ERROR("LSPCON vendor detection failed\n");
+ return false;
+ }
+ lspcon->active = true;
DRM_DEBUG_KMS("Success: LSPCON init\n");
return true;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 5/8] drm/i915: Add AVI infoframe support for LSPCON
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (2 preceding siblings ...)
2018-10-12 6:23 ` [PATCH v12 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 6/8] drm/i915: Write AVI infoframes for MCA LSPCON Shashank Sharma
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Imre Deak
In order to pass AVI infoframes to LSPCON devices, a source has to
write them in a vendor recommended method and location.
This patch series:
- adds generic LSPCON infoframe setup functions.
- registers these functions into existing AVI infoframe framework.
- triggers these functions from modeset sequence.
Next patches in the series will add vendor specific code.
V2: Added new parameter to align with new definition of
drm_hdmi_avi_infoframe_quant_range
V3: Added r-b from Maarten (for V2)
Added new parameter output_format in struct lspcon to accommodate
Ville's review comments on last patch of the series
V4: Addressed Ville's review comment
- Do not add output_format in LSPCON state, as its non-atomic. Add
this into CRTC state (added in a later patch).
V5: Rebase
V6: Rebase
V7: Rebase
V8: Rebase
V9: Rebase
V10: Rebase
V11: Accommodated rebasing changes in intel_git_port fptrs (set_infoframes and infoframe_enabled)
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 13 +++++++++-
drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++++---
drivers/gpu/drm/i915/intel_lspcon.c | 49 +++++++++++++++++++++++++++++++++++++
4 files changed, 86 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7d868f5..be21131 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2978,10 +2978,22 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
- if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+ if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
intel_ddi_pre_enable_hdmi(encoder, crtc_state, conn_state);
- else
+ } else {
+ struct intel_lspcon *lspcon =
+ enc_to_intel_lspcon(&encoder->base);
+
intel_ddi_pre_enable_dp(encoder, crtc_state, conn_state);
+ if (lspcon->active) {
+ struct intel_digital_port *dig_port =
+ enc_to_dig_port(&encoder->base);
+
+ dig_port->set_infoframes(encoder,
+ crtc_state->has_infoframe,
+ crtc_state, conn_state);
+ }
+ }
}
static void intel_disable_ddi_buf(struct intel_encoder *encoder)
@@ -3845,8 +3857,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
MISSING_CASE(port);
}
- intel_infoframe_init(intel_dig_port);
-
if (init_dp) {
if (!intel_ddi_init_dp_connector(intel_dig_port))
goto err;
@@ -3875,6 +3885,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
port_name(port));
}
+ intel_infoframe_init(intel_dig_port);
return;
err:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6981e72..8c402d0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1318,6 +1318,12 @@ static inline bool intel_encoder_is_dp(struct intel_encoder *encoder)
}
}
+static inline struct intel_lspcon *
+enc_to_intel_lspcon(struct drm_encoder *encoder)
+{
+ return &enc_to_dig_port(encoder)->lspcon;
+}
+
static inline struct intel_digital_port *
dp_to_dig_port(struct intel_dp *intel_dp)
{
@@ -1879,7 +1885,6 @@ bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
-
/* intel_lvds.c */
bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
i915_reg_t lvds_reg, enum pipe *pipe);
@@ -2210,6 +2215,12 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
bool lspcon_init(struct intel_digital_port *intel_dig_port);
void lspcon_resume(struct intel_lspcon *lspcon);
void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
+void lspcon_set_infoframes(struct intel_encoder *encoder,
+ bool enable,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state);
+bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
+ const struct intel_crtc_state *pipe_config);
/* intel_pipe_crc.c */
#ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e2cc5ed..d2232e3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2321,9 +2321,16 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
intel_dig_port->set_infoframes = g4x_set_infoframes;
intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
} else if (HAS_DDI(dev_priv)) {
- intel_dig_port->write_infoframe = hsw_write_infoframe;
- intel_dig_port->set_infoframes = hsw_set_infoframes;
- intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
+ if (intel_dig_port->lspcon.active) {
+ intel_dig_port->set_infoframes = lspcon_set_infoframes;
+ intel_dig_port->infoframe_enabled =
+ lspcon_infoframe_enabled;
+ } else {
+ intel_dig_port->set_infoframes = hsw_set_infoframes;
+ intel_dig_port->infoframe_enabled =
+ hsw_infoframe_enabled;
+ intel_dig_port->write_infoframe = hsw_write_infoframe;
+ }
} else if (HAS_PCH_IBX(dev_priv)) {
intel_dig_port->write_infoframe = ibx_write_infoframe;
intel_dig_port->set_infoframes = ibx_set_infoframes;
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 56f80b7..8d49727 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -234,6 +234,55 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
}
+void lspcon_set_infoframes(struct intel_encoder *encoder,
+ bool enable,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ ssize_t ret;
+ union hdmi_infoframe frame;
+ uint8_t buf[VIDEO_DIP_DATA_SIZE];
+ struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+ struct intel_lspcon *lspcon = &dig_port->lspcon;
+ struct intel_dp *intel_dp = &dig_port->dp;
+ struct drm_connector *connector = &intel_dp->attached_connector->base;
+ const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
+ bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
+
+ if (!lspcon->active) {
+ DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
+ return;
+ }
+
+ ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+ mode, is_hdmi2_sink);
+ if (ret < 0) {
+ DRM_ERROR("couldn't fill AVI infoframe\n");
+ return;
+ }
+
+ drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+ crtc_state->limited_color_range ?
+ HDMI_QUANTIZATION_RANGE_LIMITED :
+ HDMI_QUANTIZATION_RANGE_FULL,
+ false, is_hdmi2_sink);
+
+ ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
+ if (ret < 0) {
+ DRM_ERROR("Failed to pack AVI IF\n");
+ return;
+ }
+
+ dig_port->write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_AVI,
+ buf, ret);
+}
+
+bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
+ const struct intel_crtc_state *pipe_config)
+{
+ return enc_to_intel_lspcon(&encoder->base)->active;
+}
+
void lspcon_resume(struct intel_lspcon *lspcon)
{
enum drm_lspcon_mode expected_mode;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 6/8] drm/i915: Write AVI infoframes for MCA LSPCON
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (3 preceding siblings ...)
2018-10-12 6:23 ` [PATCH v12 5/8] drm/i915: Add AVI infoframe support for LSPCON Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 7/8] drm/i915: Write AVI infoframes for Parade LSPCON Shashank Sharma
` (4 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
From: Shashank Sharma <shashank.sharma@linux.intel.com>
LSPCON is a DP branch device, so LSPCON vendors define
specific methods to pass AVI infoframes to the the chip.
This patch adds:
- a generic wrapper function for writing AVI infoframes for
all LSPCON devices.
- a vendor specific function to wrire AVI infoframes into
MCA LSPCON devices.
V2: Rebase
V3: Added r-b from Maarten
V4: Rebase
V5: Rebase
V6: Rebase
V7: Fixed checkpatch warnings for alignment
V8: Rebase
V9: Added the retry logic, with 50ms incremental delays while
writing AVI IF
V10: Changed the return value check
V11: Fixed checkpatch warning
V12: Rebase
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 4 ++
drivers/gpu/drm/i915/intel_hdmi.c | 2 +
drivers/gpu/drm/i915/intel_lspcon.c | 88 +++++++++++++++++++++++++++++++++++++
3 files changed, 94 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8c402d0..b8e5f95 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2215,6 +2215,10 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
bool lspcon_init(struct intel_digital_port *intel_dig_port);
void lspcon_resume(struct intel_lspcon *lspcon);
void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
+void lspcon_write_infoframe(struct intel_encoder *encoder,
+ const struct intel_crtc_state *crtc_state,
+ unsigned int type,
+ const void *buf, ssize_t len);
void lspcon_set_infoframes(struct intel_encoder *encoder,
bool enable,
const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d2232e3..89d5e39 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2322,6 +2322,8 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
} else if (HAS_DDI(dev_priv)) {
if (intel_dig_port->lspcon.active) {
+ intel_dig_port->write_infoframe =
+ lspcon_write_infoframe;
intel_dig_port->set_infoframes = lspcon_set_infoframes;
intel_dig_port->infoframe_enabled =
lspcon_infoframe_enabled;
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 8d49727..b806803 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -31,6 +31,12 @@
#define LSPCON_VENDOR_PARADE_OUI 0x001CF8
#define LSPCON_VENDOR_MCA_OUI 0x0060AD
+/* AUX addresses to write MCA AVI IF */
+#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
+#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
+#define LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
+#define LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
+
static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
{
struct intel_digital_port *dig_port =
@@ -234,6 +240,88 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
}
+static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
+ const uint8_t *buffer, ssize_t len)
+{
+ int ret;
+ uint32_t val = 0;
+ uint32_t retry;
+ uint16_t reg;
+ const uint8_t *data = buffer;
+
+ reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
+ while (val < len) {
+ /* DPCD write for AVI IF can fail on a slow FW day, so retry */
+ for (retry = 0; retry < 5; retry++) {
+ ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
+ if (ret == 1) {
+ break;
+ } else if (retry < 4) {
+ mdelay(50);
+ continue;
+ } else {
+ DRM_ERROR("DPCD write failed at:0x%x\n", reg);
+ return false;
+ }
+ }
+ val++; reg++; data++;
+ }
+
+ val = 0;
+ reg = LSPCON_MCA_AVI_IF_CTRL;
+ ret = drm_dp_dpcd_read(aux, reg, &val, 1);
+ if (ret < 0) {
+ DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
+ return false;
+ }
+
+ /* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
+ val &= ~LSPCON_MCA_AVI_IF_HANDLED;
+ val |= LSPCON_MCA_AVI_IF_KICKOFF;
+
+ ret = drm_dp_dpcd_write(aux, reg, &val, 1);
+ if (ret < 0) {
+ DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
+ return false;
+ }
+
+ val = 0;
+ ret = drm_dp_dpcd_read(aux, reg, &val, 1);
+ if (ret < 0) {
+ DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
+ return false;
+ }
+
+ if (val == LSPCON_MCA_AVI_IF_HANDLED)
+ DRM_DEBUG_KMS("AVI IF handled by FW\n");
+
+ return true;
+}
+
+void lspcon_write_infoframe(struct intel_encoder *encoder,
+ const struct intel_crtc_state *crtc_state,
+ unsigned int type,
+ const void *frame, ssize_t len)
+{
+ bool ret = true;
+ struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+ struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
+
+ /* LSPCON only needs AVI IF */
+ if (type != HDMI_INFOFRAME_TYPE_AVI)
+ return;
+
+ if (lspcon->vendor == LSPCON_VENDOR_MCA)
+ ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
+ frame, len);
+ if (!ret) {
+ DRM_ERROR("Failed to write AVI infoframes\n");
+ return;
+ }
+
+ DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
+}
+
void lspcon_set_infoframes(struct intel_encoder *encoder,
bool enable,
const struct intel_crtc_state *crtc_state,
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 7/8] drm/i915: Write AVI infoframes for Parade LSPCON
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (4 preceding siblings ...)
2018-10-12 6:23 ` [PATCH v12 6/8] drm/i915: Write AVI infoframes for MCA LSPCON Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
` (3 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
Different LSPCON vendors specify their custom methods to pass
AVI infoframes to the LSPCON chip, so does Parade tech.
This patch adds functions to arrange and write AVI infoframes
into Parade LSPCON chips.
V2: rebase
V3: Added r-b from Maarten
V4: rebase
V5: rebase
V6: rebase
V7: Fixed checkpatch warnings for alignment
V8: Rebase
V9: Rebase
V10: Rebase
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_lspcon.c | 117 +++++++++++++++++++++++++++++++++++-
1 file changed, 116 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index b806803..829c40a 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -37,6 +37,12 @@
#define LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
#define LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
+/* AUX addresses to write Parade AVI IF */
+#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
+#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
+#define LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
+#define LSPCON_PARADE_AVI_IF_DATA_SIZE 32
+
static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
{
struct intel_digital_port *dig_port =
@@ -240,6 +246,111 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
}
+static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
+{
+ u8 avi_if_ctrl;
+ u8 retry;
+ ssize_t ret;
+
+ /* Check if LSPCON FW is ready for data */
+ for (retry = 0; retry < 5; retry++) {
+ if (retry)
+ usleep_range(200, 300);
+
+ ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
+ &avi_if_ctrl, 1);
+ if (ret < 0) {
+ DRM_ERROR("Failed to read AVI IF control\n");
+ return false;
+ }
+
+ if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
+ return true;
+ }
+
+ DRM_ERROR("Parade FW not ready to accept AVI IF\n");
+ return false;
+}
+
+static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
+ uint8_t *avi_buf)
+{
+ u8 avi_if_ctrl;
+ u8 block_count = 0;
+ u8 *data;
+ uint16_t reg;
+ ssize_t ret;
+
+ while (block_count < 4) {
+ if (!lspcon_parade_fw_ready(aux)) {
+ DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
+ block_count);
+ return false;
+ }
+
+ reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
+ data = avi_buf + block_count * 8;
+ ret = drm_dp_dpcd_write(aux, reg, data, 8);
+ if (ret < 0) {
+ DRM_ERROR("Failed to write AVI IF block %d\n",
+ block_count);
+ return false;
+ }
+
+ /*
+ * Once a block of data is written, we have to inform the FW
+ * about this by writing into avi infoframe control register:
+ * - set the kickoff bit[7] to 1
+ * - write the block no. to bits[1:0]
+ */
+ reg = LSPCON_PARADE_AVI_IF_CTRL;
+ avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
+ ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
+ if (ret < 0) {
+ DRM_ERROR("Failed to update (0x%x), block %d\n",
+ reg, block_count);
+ return false;
+ }
+
+ block_count++;
+ }
+
+ DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
+ return true;
+}
+
+static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
+ const uint8_t *frame,
+ ssize_t len)
+{
+ uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
+
+ /*
+ * Parade's frames contains 32 bytes of data, divided
+ * into 4 frames:
+ * Token byte (first byte of first frame, must be non-zero)
+ * HB0 to HB2 from AVI IF (3 bytes header)
+ * PB0 to PB27 from AVI IF (28 bytes data)
+ * So it should look like this
+ * first block: | <token> <HB0-HB2> <DB0-DB3> |
+ * next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
+ */
+
+ if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
+ DRM_ERROR("Invalid length of infoframes\n");
+ return false;
+ }
+
+ memcpy(&avi_if[1], frame, len);
+
+ if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
+ DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
+ return false;
+ }
+
+ return true;
+}
+
static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
const uint8_t *buffer, ssize_t len)
{
@@ -303,7 +414,7 @@ void lspcon_write_infoframe(struct intel_encoder *encoder,
unsigned int type,
const void *frame, ssize_t len)
{
- bool ret = true;
+ bool ret;
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
@@ -314,6 +425,10 @@ void lspcon_write_infoframe(struct intel_encoder *encoder,
if (lspcon->vendor == LSPCON_VENDOR_MCA)
ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
frame, len);
+ else
+ ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
+ frame, len);
+
if (!ret) {
DRM_ERROR("Failed to write AVI infoframes\n");
return;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (5 preceding siblings ...)
2018-10-12 6:23 ` [PATCH v12 7/8] drm/i915: Write AVI infoframes for Parade LSPCON Shashank Sharma
@ 2018-10-12 6:23 ` Shashank Sharma
2018-10-12 13:58 ` Ville Syrjälä
2018-10-12 6:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v12,1/8] drm/i915: Introduce CRTC output format Patchwork
` (2 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Shashank Sharma @ 2018-10-12 6:23 UTC (permalink / raw)
To: intel-gfx
From: Shashank Sharma <shashank.sharma@linux.intel.com>
LSPCON chips can generate YCBCR outputs, if asked nicely :).
In order to generate YCBCR 4:2:0 outputs, a source must:
- send YCBCR 4:4:4 signals to LSPCON
- program color space as 4:2:0 in AVI infoframes
Whereas for YCBCR 4:4:4 outputs, the source must:
- send YCBCR 4:4:4 signals to LSPCON
- program color space as 4:4:4 in AVI infoframes
So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
information indicates LSPCON FW to start scaling down from YCBCR
4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
V2: rebase
V3: Addressed review comments from Ville
- add enum crtc_output_format instead of bool ycbcr420
- use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
cases in this way we will have YCBCR 4:4:4 framework ready (except
the ABI part)
V4: Added r-b from Maarten (for v3)
Addressed review comments from Ville:
- Do not add a non-atomic state variable to determine lspcon output.
Instead add bool in CRTC state to indicate lspcon based scaling.
V5: Addressed review comments from Ville:
- Change the state bool name from external scaling to something more
relavent.
- Keep the info and adjusted_mode structures const.
- use crtc_state instead of pipe_config.
- Push all the config change into lspcon_ycbcr420_config function.
V6: Rebase, small changes to accommodate changes in patch 2.
V7: Fixed checkpatch warnings for alignment
V8: Rebase
PS: Ignored following warnings to match the current formatting:
drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
-:53: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#53: FILE: drivers/gpu/drm/i915/i915_reg.h:8721:
+#define TRANS_MSA_SAMPLING_444 (2<<1)
^
-:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#54: FILE: drivers/gpu/drm/i915/i915_reg.h:8722:
+#define TRANS_MSA_CLRSP_YCBCR (2<<3)
V9: Rebase
V10: Rebase
V11: Rebase
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
drivers/gpu/drm/i915/intel_dp.c | 4 ++++
drivers/gpu/drm/i915/intel_drv.h | 5 +++++
drivers/gpu/drm/i915/intel_lspcon.c | 26 ++++++++++++++++++++++++++
6 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2078541..1e13e51 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9229,6 +9229,8 @@ enum skl_power_gate {
#define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
#define TRANS_MSA_SYNC_CLK (1 << 0)
+#define TRANS_MSA_SAMPLING_444 (2 << 1)
+#define TRANS_MSA_CLRSP_YCBCR (2 << 3)
#define TRANS_MSA_6_BPC (0 << 5)
#define TRANS_MSA_8_BPC (1 << 5)
#define TRANS_MSA_10_BPC (2 << 5)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index be21131..186111a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1784,6 +1784,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
break;
}
+ /*
+ * As per DP 1.2 spec section 2.3.4.3 while sending
+ * YCBCR 444 signals we should program MSA MISC1/0 fields with
+ * colorspace information. The output colorspace encoding is BT601.
+ */
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
+ temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3317c6c..41abd03 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7797,6 +7797,8 @@ static void intel_get_crtc_ycbcr_config(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));
@@ -7814,6 +7816,16 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
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;
}
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c3f6330..1156d59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2074,6 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+ struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
enum port port = encoder->port;
struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
struct intel_connector *intel_connector = intel_dp->attached_connector;
@@ -2086,6 +2087,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
pipe_config->has_pch_encoder = true;
pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+ if (lspcon->active)
+ lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
+
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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b8e5f95..95e252e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -908,6 +908,9 @@ struct intel_crtc_state {
/* Output format RGB/YCBCR etc */
enum intel_output_format output_format;
+
+ /* Output down scaling is done in LSPCON device */
+ bool lspcon_downsampling;
};
struct intel_crtc {
@@ -2225,6 +2228,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
const struct drm_connector_state *conn_state);
bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
const struct intel_crtc_state *pipe_config);
+void lspcon_ycbcr420_config(struct drm_connector *connector,
+ struct intel_crtc_state *crtc_state);
/* intel_pipe_crc.c */
#ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 829c40a..fff32b3 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
return true;
}
+void lspcon_ycbcr420_config(struct drm_connector *connector,
+ struct intel_crtc_state *crtc_state)
+{
+ const struct drm_display_info *info = &connector->display_info;
+ const struct drm_display_mode *adjusted_mode =
+ &crtc_state->base.adjusted_mode;
+
+ if (drm_mode_is_420_only(info, adjusted_mode) &&
+ connector->ycbcr_420_allowed) {
+ crtc_state->port_clock /= 2;
+ crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
+ crtc_state->lspcon_downsampling = true;
+ }
+}
+
static bool lspcon_probe(struct intel_lspcon *lspcon)
{
int retry;
@@ -464,6 +479,15 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
return;
}
+ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
+ if (crtc_state->lspcon_downsampling)
+ frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
+ else
+ frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
+ } else {
+ frame.avi.colorspace = HDMI_COLORSPACE_RGB;
+ }
+
drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
crtc_state->limited_color_range ?
HDMI_QUANTIZATION_RANGE_LIMITED :
@@ -517,6 +541,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_connector *connector = &dp->attached_connector->base;
if (!HAS_LSPCON(dev_priv)) {
DRM_ERROR("LSPCON is not supported on this platform\n");
@@ -541,6 +566,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
return false;
}
+ connector->ycbcr_420_allowed = true;
lspcon->active = true;
DRM_DEBUG_KMS("Success: LSPCON init\n");
return true;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v12,1/8] drm/i915: Introduce CRTC output format
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (6 preceding siblings ...)
2018-10-12 6:23 ` [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
@ 2018-10-12 6:34 ` Patchwork
2018-10-12 6:50 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-12 7:51 ` ✓ Fi.CI.IGT: " Patchwork
9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-12 6:34 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v12,1/8] drm/i915: Introduce CRTC output format
URL : https://patchwork.freedesktop.org/series/50897/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b69fba58f360 drm/i915: Introduce CRTC output format
fbd32b6ad34e drm/i915: Add CRTC output format YCBCR 4:2:0
-:272: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Shashank Sharma <shashank.sharma@linux.intel.com>'
total: 0 errors, 1 warnings, 0 checks, 187 lines checked
442a5056dd04 drm/i915: Add CRTC output format YCBCR 4:4:4
-:122: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Shashank Sharma <shashank.sharma@linux.intel.com>'
total: 0 errors, 1 warnings, 0 checks, 65 lines checked
134878c5cb18 drm/i915: Check LSPCON vendor OUI
-:165: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Sharma, Shashank <shashank.sharma@intel.com>'
total: 0 errors, 1 warnings, 0 checks, 114 lines checked
adea576b594d drm/i915: Add AVI infoframe support for LSPCON
-:30: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#30:
V11: Accommodated rebasing changes in intel_git_port fptrs (set_infoframes and infoframe_enabled)
total: 0 errors, 1 warnings, 0 checks, 144 lines checked
1ddfe1f97e7f drm/i915: Write AVI infoframes for MCA LSPCON
-:169: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Shashank Sharma <shashank.sharma@linux.intel.com>'
total: 0 errors, 1 warnings, 0 checks, 118 lines checked
cacda7f35a67 drm/i915: Write AVI infoframes for Parade LSPCON
0503d0b7e9f8 drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
-:153: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#153: FILE: drivers/gpu/drm/i915/intel_drv.h:913:
+ bool lspcon_downsampling;
-:223: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Shashank Sharma <shashank.sharma@linux.intel.com>'
total: 0 errors, 1 warnings, 1 checks, 128 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v12,1/8] drm/i915: Introduce CRTC output format
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (7 preceding siblings ...)
2018-10-12 6:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v12,1/8] drm/i915: Introduce CRTC output format Patchwork
@ 2018-10-12 6:50 ` Patchwork
2018-10-12 7:51 ` ✓ Fi.CI.IGT: " Patchwork
9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-12 6:50 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v12,1/8] drm/i915: Introduce CRTC output format
URL : https://patchwork.freedesktop.org/series/50897/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4976 -> Patchwork_10433 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/50897/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10433 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@kms_flip@basic-flip-vs-modeset:
fi-glk-j4005: DMESG-WARN (fdo#106097) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-c:
fi-skl-6700k2: FAIL (fdo#103191, fdo#107362) -> PASS
igt@pm_rpm@module-reload:
fi-skl-6600u: INCOMPLETE (fdo#107807) -> PASS
fi-glk-j4005: FAIL -> PASS
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
== Participating hosts (45 -> 40) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-u
== Build changes ==
* Linux: CI_DRM_4976 -> Patchwork_10433
CI_DRM_4976: dec9886eff39d38332bb5ea438b9b053d6b2177c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10433: 0503d0b7e9f84956e86f12d600a4bb33c8b4fb27 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
0503d0b7e9f8 drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
cacda7f35a67 drm/i915: Write AVI infoframes for Parade LSPCON
1ddfe1f97e7f drm/i915: Write AVI infoframes for MCA LSPCON
adea576b594d drm/i915: Add AVI infoframe support for LSPCON
134878c5cb18 drm/i915: Check LSPCON vendor OUI
442a5056dd04 drm/i915: Add CRTC output format YCBCR 4:4:4
fbd32b6ad34e drm/i915: Add CRTC output format YCBCR 4:2:0
b69fba58f360 drm/i915: Introduce CRTC output format
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10433/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [v12,1/8] drm/i915: Introduce CRTC output format
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
` (8 preceding siblings ...)
2018-10-12 6:50 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-12 7:51 ` Patchwork
9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-12 7:51 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v12,1/8] drm/i915: Introduce CRTC output format
URL : https://patchwork.freedesktop.org/series/50897/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4976_full -> Patchwork_10433_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10433_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10433_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10433_full:
=== IGT changes ===
==== Warnings ====
igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-wc:
shard-snb: PASS -> SKIP +3
== Known issues ==
Here are the changes found in Patchwork_10433_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_cpu_reloc@full:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108073)
igt@gem_exec_await@wide-contexts:
shard-glk: PASS -> FAIL (fdo#106680)
igt@gem_exec_schedule@pi-ringfull-render:
shard-skl: NOTRUN -> FAIL (fdo#103158) +1
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#103665)
igt@gem_userptr_blits@readonly-unsync:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108074)
igt@kms_busy@extended-modeset-hang-newfb-render-a:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956) +2
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
shard-snb: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
shard-skl: NOTRUN -> FAIL (fdo#105458) +1
igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge:
shard-skl: NOTRUN -> FAIL (fdo#104671)
igt@kms_cursor_crc@cursor-128x42-onscreen:
shard-apl: PASS -> FAIL (fdo#103232) +2
igt@kms_cursor_crc@cursor-128x42-sliding:
shard-skl: NOTRUN -> FAIL (fdo#103232) +2
igt@kms_cursor_crc@cursor-256x85-sliding:
shard-glk: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-64x64-suspend:
shard-apl: PASS -> FAIL (fdo#103191, fdo#103232)
igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
shard-glk: PASS -> DMESG-WARN (fdo#106538, fdo#105763)
igt@kms_draw_crc@draw-method-rgb565-mmap-cpu-ytiled:
shard-skl: NOTRUN -> FAIL (fdo#103184)
igt@kms_fbcon_fbt@fbc:
shard-skl: NOTRUN -> FAIL (fdo#103833, fdo#105682)
igt@kms_fbcon_fbt@psr-suspend:
shard-skl: NOTRUN -> FAIL (fdo#107882)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
shard-skl: NOTRUN -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
shard-apl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
shard-glk: PASS -> FAIL (fdo#103167) +3
igt@kms_frontbuffer_tracking@fbcpsr-1p-indfb-fliptrack:
shard-skl: NOTRUN -> FAIL (fdo#105682) +2
igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-render:
shard-skl: PASS -> FAIL (fdo#105682)
igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt:
shard-skl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@psr-rgb565-draw-render:
shard-skl: SKIP -> FAIL (fdo#103167)
igt@kms_plane@pixel-format-pipe-b-planes:
shard-skl: NOTRUN -> DMESG-FAIL (fdo#106885, fdo#103166) +1
igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
shard-skl: NOTRUN -> INCOMPLETE (fdo#104108, fdo#107773)
igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108145) +7
igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
shard-skl: PASS -> FAIL (fdo#108145)
igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108146) +2
igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
shard-apl: PASS -> FAIL (fdo#103166) +1
igt@kms_setmode@basic:
shard-skl: NOTRUN -> FAIL (fdo#99912)
igt@pm_backlight@fade_with_suspend:
shard-skl: NOTRUN -> FAIL (fdo#107847)
igt@pm_rpm@modeset-non-lpsp-stress:
shard-skl: SKIP -> INCOMPLETE (fdo#107807)
igt@pm_rpm@system-suspend:
shard-skl: NOTRUN -> INCOMPLETE (fdo#104108, fdo#107807) +1
==== Possible fixes ====
igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
shard-glk: FAIL (fdo#108145) -> PASS
igt@kms_cursor_crc@cursor-256x256-random:
shard-apl: FAIL (fdo#103232) -> PASS +3
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-skl: FAIL (fdo#100368) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
shard-glk: FAIL (fdo#103167) -> PASS +4
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
shard-skl: INCOMPLETE (fdo#104108, fdo#107773) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@kms_plane@plane-position-covered-pipe-a-planes:
shard-glk: FAIL (fdo#103166) -> PASS +1
igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
shard-apl: FAIL (fdo#103166) -> PASS
igt@kms_setmode@basic:
shard-kbl: FAIL (fdo#99912) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103833 https://bugs.freedesktop.org/show_bug.cgi?id=103833
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#105458 https://bugs.freedesktop.org/show_bug.cgi?id=105458
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#107847 https://bugs.freedesktop.org/show_bug.cgi?id=107847
fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4976 -> Patchwork_10433
CI_DRM_4976: dec9886eff39d38332bb5ea438b9b053d6b2177c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10433: 0503d0b7e9f84956e86f12d600a4bb33c8b4fb27 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10433/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 6:23 ` [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
@ 2018-10-12 13:58 ` Ville Syrjälä
2018-10-12 17:51 ` Sharma, Shashank
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2018-10-12 13:58 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx
On Fri, Oct 12, 2018 at 11:53:14AM +0530, Shashank Sharma wrote:
> From: Shashank Sharma <shashank.sharma@linux.intel.com>
>
> LSPCON chips can generate YCBCR outputs, if asked nicely :).
>
> In order to generate YCBCR 4:2:0 outputs, a source must:
> - send YCBCR 4:4:4 signals to LSPCON
> - program color space as 4:2:0 in AVI infoframes
>
> Whereas for YCBCR 4:4:4 outputs, the source must:
> - send YCBCR 4:4:4 signals to LSPCON
> - program color space as 4:4:4 in AVI infoframes
>
> So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
> pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
> information indicates LSPCON FW to start scaling down from YCBCR
> 4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
> LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
>
> V2: rebase
> V3: Addressed review comments from Ville
> - add enum crtc_output_format instead of bool ycbcr420
> - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
> cases in this way we will have YCBCR 4:4:4 framework ready (except
> the ABI part)
> V4: Added r-b from Maarten (for v3)
> Addressed review comments from Ville:
> - Do not add a non-atomic state variable to determine lspcon output.
> Instead add bool in CRTC state to indicate lspcon based scaling.
> V5: Addressed review comments from Ville:
> - Change the state bool name from external scaling to something more
> relavent.
> - Keep the info and adjusted_mode structures const.
> - use crtc_state instead of pipe_config.
> - Push all the config change into lspcon_ycbcr420_config function.
> V6: Rebase, small changes to accommodate changes in patch 2.
> V7: Fixed checkpatch warnings for alignment
> V8: Rebase
>
> PS: Ignored following warnings to match the current formatting:
> drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
> -:53: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> #53: FILE: drivers/gpu/drm/i915/i915_reg.h:8721:
> +#define TRANS_MSA_SAMPLING_444 (2<<1)
> ^
> -:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> #54: FILE: drivers/gpu/drm/i915/i915_reg.h:8722:
> +#define TRANS_MSA_CLRSP_YCBCR (2<<3)
> V9: Rebase
> V10: Rebase
> V11: Rebase
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
> drivers/gpu/drm/i915/intel_lspcon.c | 26 ++++++++++++++++++++++++++
> 6 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2078541..1e13e51 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9229,6 +9229,8 @@ enum skl_power_gate {
> #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
>
> #define TRANS_MSA_SYNC_CLK (1 << 0)
> +#define TRANS_MSA_SAMPLING_444 (2 << 1)
> +#define TRANS_MSA_CLRSP_YCBCR (2 << 3)
> #define TRANS_MSA_6_BPC (0 << 5)
> #define TRANS_MSA_8_BPC (1 << 5)
> #define TRANS_MSA_10_BPC (2 << 5)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index be21131..186111a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1784,6 +1784,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
> break;
> }
>
> + /*
> + * As per DP 1.2 spec section 2.3.4.3 while sending
> + * YCBCR 444 signals we should program MSA MISC1/0 fields with
> + * colorspace information. The output colorspace encoding is BT601.
> + */
> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
> I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3317c6c..41abd03 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7797,6 +7797,8 @@ static void intel_get_crtc_ycbcr_config(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));
>
> @@ -7814,6 +7816,16 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
> 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;
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c3f6330..1156d59 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2074,6 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
> enum port port = encoder->port;
> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> struct intel_connector *intel_connector = intel_dp->attached_connector;
> @@ -2086,6 +2087,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> pipe_config->has_pch_encoder = true;
>
> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> + if (lspcon->active)
> + lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
> +
> 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b8e5f95..95e252e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -908,6 +908,9 @@ struct intel_crtc_state {
>
> /* Output format RGB/YCBCR etc */
> enum intel_output_format output_format;
> +
> + /* Output down scaling is done in LSPCON device */
> + bool lspcon_downsampling;
> };
>
> struct intel_crtc {
> @@ -2225,6 +2228,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
> const struct drm_connector_state *conn_state);
> bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config);
> +void lspcon_ycbcr420_config(struct drm_connector *connector,
> + struct intel_crtc_state *crtc_state);
>
> /* intel_pipe_crc.c */
> #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 829c40a..fff32b3 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
> return true;
> }
>
> +void lspcon_ycbcr420_config(struct drm_connector *connector,
> + struct intel_crtc_state *crtc_state)
> +{
> + const struct drm_display_info *info = &connector->display_info;
> + const struct drm_display_mode *adjusted_mode =
> + &crtc_state->base.adjusted_mode;
> +
> + if (drm_mode_is_420_only(info, adjusted_mode) &&
> + connector->ycbcr_420_allowed) {
> + crtc_state->port_clock /= 2;
This looks bogus. We're talking about DP here so we should not be
frobbing port_clock. And since we output 4:4:4 from the port
anyway we don't even have to take 4:2:0 into consideration
when calculating port_clock.
Ah, this guy is called before we even calculate port_clock.
That explains why this didn't cause any problems. So this is
just dead code here.
And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
also dead code since the caller overwrites it there as well.
> + crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
> + crtc_state->lspcon_downsampling = true;
> + }
> +}
> +
> static bool lspcon_probe(struct intel_lspcon *lspcon)
> {
> int retry;
> @@ -464,6 +479,15 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
> return;
> }
>
> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
> + if (crtc_state->lspcon_downsampling)
> + frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
> + else
> + frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> + } else {
> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> + }
> +
> drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> crtc_state->limited_color_range ?
> HDMI_QUANTIZATION_RANGE_LIMITED :
> @@ -517,6 +541,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_connector *connector = &dp->attached_connector->base;
>
> if (!HAS_LSPCON(dev_priv)) {
> DRM_ERROR("LSPCON is not supported on this platform\n");
> @@ -541,6 +566,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> return false;
> }
>
> + connector->ycbcr_420_allowed = true;
> lspcon->active = true;
> DRM_DEBUG_KMS("Success: LSPCON init\n");
> return true;
> --
> 2.7.4
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 13:58 ` Ville Syrjälä
@ 2018-10-12 17:51 ` Sharma, Shashank
2018-10-12 18:09 ` Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Sharma, Shashank @ 2018-10-12 17:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Regards
Shashank
On 10/12/2018 7:28 PM, Ville Syrjälä wrote:
> On Fri, Oct 12, 2018 at 11:53:14AM +0530, Shashank Sharma wrote:
>> From: Shashank Sharma <shashank.sharma@linux.intel.com>
>>
>> LSPCON chips can generate YCBCR outputs, if asked nicely :).
>>
>> In order to generate YCBCR 4:2:0 outputs, a source must:
>> - send YCBCR 4:4:4 signals to LSPCON
>> - program color space as 4:2:0 in AVI infoframes
>>
>> Whereas for YCBCR 4:4:4 outputs, the source must:
>> - send YCBCR 4:4:4 signals to LSPCON
>> - program color space as 4:4:4 in AVI infoframes
>>
>> So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
>> pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
>> information indicates LSPCON FW to start scaling down from YCBCR
>> 4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
>> LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
>>
>> V2: rebase
>> V3: Addressed review comments from Ville
>> - add enum crtc_output_format instead of bool ycbcr420
>> - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
>> cases in this way we will have YCBCR 4:4:4 framework ready (except
>> the ABI part)
>> V4: Added r-b from Maarten (for v3)
>> Addressed review comments from Ville:
>> - Do not add a non-atomic state variable to determine lspcon output.
>> Instead add bool in CRTC state to indicate lspcon based scaling.
>> V5: Addressed review comments from Ville:
>> - Change the state bool name from external scaling to something more
>> relavent.
>> - Keep the info and adjusted_mode structures const.
>> - use crtc_state instead of pipe_config.
>> - Push all the config change into lspcon_ycbcr420_config function.
>> V6: Rebase, small changes to accommodate changes in patch 2.
>> V7: Fixed checkpatch warnings for alignment
>> V8: Rebase
>>
>> PS: Ignored following warnings to match the current formatting:
>> drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
>> -:53: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
>> #53: FILE: drivers/gpu/drm/i915/i915_reg.h:8721:
>> +#define TRANS_MSA_SAMPLING_444 (2<<1)
>> ^
>> -:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
>> #54: FILE: drivers/gpu/drm/i915/i915_reg.h:8722:
>> +#define TRANS_MSA_CLRSP_YCBCR (2<<3)
>> V9: Rebase
>> V10: Rebase
>> V11: Rebase
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 2 ++
>> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
>> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
>> drivers/gpu/drm/i915/intel_lspcon.c | 26 ++++++++++++++++++++++++++
>> 6 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 2078541..1e13e51 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -9229,6 +9229,8 @@ enum skl_power_gate {
>> #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
>>
>> #define TRANS_MSA_SYNC_CLK (1 << 0)
>> +#define TRANS_MSA_SAMPLING_444 (2 << 1)
>> +#define TRANS_MSA_CLRSP_YCBCR (2 << 3)
>> #define TRANS_MSA_6_BPC (0 << 5)
>> #define TRANS_MSA_8_BPC (1 << 5)
>> #define TRANS_MSA_10_BPC (2 << 5)
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index be21131..186111a 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1784,6 +1784,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
>> break;
>> }
>>
>> + /*
>> + * As per DP 1.2 spec section 2.3.4.3 while sending
>> + * YCBCR 444 signals we should program MSA MISC1/0 fields with
>> + * colorspace information. The output colorspace encoding is BT601.
>> + */
>> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
>> + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
>> I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3317c6c..41abd03 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7797,6 +7797,8 @@ static void intel_get_crtc_ycbcr_config(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));
>>
>> @@ -7814,6 +7816,16 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
>> 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;
>> }
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index c3f6330..1156d59 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -2074,6 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
>> enum port port = encoder->port;
>> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>> struct intel_connector *intel_connector = intel_dp->attached_connector;
>> @@ -2086,6 +2087,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>> pipe_config->has_pch_encoder = true;
>>
>> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> + if (lspcon->active)
>> + lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
>> +
>> 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index b8e5f95..95e252e 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -908,6 +908,9 @@ struct intel_crtc_state {
>>
>> /* Output format RGB/YCBCR etc */
>> enum intel_output_format output_format;
>> +
>> + /* Output down scaling is done in LSPCON device */
>> + bool lspcon_downsampling;
>> };
>>
>> struct intel_crtc {
>> @@ -2225,6 +2228,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>> const struct drm_connector_state *conn_state);
>> bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
>> const struct intel_crtc_state *pipe_config);
>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
>> + struct intel_crtc_state *crtc_state);
>>
>> /* intel_pipe_crc.c */
>> #ifdef CONFIG_DEBUG_FS
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> index 829c40a..fff32b3 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
>> return true;
>> }
>>
>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
>> + struct intel_crtc_state *crtc_state)
>> +{
>> + const struct drm_display_info *info = &connector->display_info;
>> + const struct drm_display_mode *adjusted_mode =
>> + &crtc_state->base.adjusted_mode;
>> +
>> + if (drm_mode_is_420_only(info, adjusted_mode) &&
>> + connector->ycbcr_420_allowed) {
>> + crtc_state->port_clock /= 2;
> This looks bogus. We're talking about DP here so we should not be
> frobbing port_clock. And since we output 4:4:4 from the port
> anyway we don't even have to take 4:2:0 into consideration
> when calculating port_clock.
I agree on this.
> Ah, this guy is called before we even calculate port_clock.
> That explains why this didn't cause any problems. So this is
> just dead code here.
>
> And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
> also dead code since the caller overwrites it there as well.
I am not sure if that's the case for Native HDMI 2.0.
In intel_hdmi_420_config we are updating the config->port_clock, which
is the same thing being updated for 8/12/16 BPC deep color too, just
after this function.
I have tested the output with HDMI 2.0 analyzer, which reported right
pixel clock (297Mhz). Now, as any of the port clock calculations do not
get information
about HDMI output type, there is no other way they will calculate the
right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
active.
- Shashank
>> + crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
>> + crtc_state->lspcon_downsampling = true;
>> + }
>> +}
>> +
>> static bool lspcon_probe(struct intel_lspcon *lspcon)
>> {
>> int retry;
>> @@ -464,6 +479,15 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>> return;
>> }
>>
>> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
>> + if (crtc_state->lspcon_downsampling)
>> + frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
>> + else
>> + frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
>> + } else {
>> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>> + }
>> +
>> drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
>> crtc_state->limited_color_range ?
>> HDMI_QUANTIZATION_RANGE_LIMITED :
>> @@ -517,6 +541,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>> struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>> struct drm_device *dev = intel_dig_port->base.base.dev;
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct drm_connector *connector = &dp->attached_connector->base;
>>
>> if (!HAS_LSPCON(dev_priv)) {
>> DRM_ERROR("LSPCON is not supported on this platform\n");
>> @@ -541,6 +566,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>> return false;
>> }
>>
>> + connector->ycbcr_420_allowed = true;
>> lspcon->active = true;
>> DRM_DEBUG_KMS("Success: LSPCON init\n");
>> return true;
>> --
>> 2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 17:51 ` Sharma, Shashank
@ 2018-10-12 18:09 ` Ville Syrjälä
2018-10-12 18:32 ` Sharma, Shashank
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2018-10-12 18:09 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx
On Fri, Oct 12, 2018 at 11:21:56PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 10/12/2018 7:28 PM, Ville Syrjälä wrote:
> > On Fri, Oct 12, 2018 at 11:53:14AM +0530, Shashank Sharma wrote:
> >> From: Shashank Sharma <shashank.sharma@linux.intel.com>
> >>
> >> LSPCON chips can generate YCBCR outputs, if asked nicely :).
> >>
> >> In order to generate YCBCR 4:2:0 outputs, a source must:
> >> - send YCBCR 4:4:4 signals to LSPCON
> >> - program color space as 4:2:0 in AVI infoframes
> >>
> >> Whereas for YCBCR 4:4:4 outputs, the source must:
> >> - send YCBCR 4:4:4 signals to LSPCON
> >> - program color space as 4:4:4 in AVI infoframes
> >>
> >> So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
> >> pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
> >> information indicates LSPCON FW to start scaling down from YCBCR
> >> 4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
> >> LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
> >>
> >> V2: rebase
> >> V3: Addressed review comments from Ville
> >> - add enum crtc_output_format instead of bool ycbcr420
> >> - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
> >> cases in this way we will have YCBCR 4:4:4 framework ready (except
> >> the ABI part)
> >> V4: Added r-b from Maarten (for v3)
> >> Addressed review comments from Ville:
> >> - Do not add a non-atomic state variable to determine lspcon output.
> >> Instead add bool in CRTC state to indicate lspcon based scaling.
> >> V5: Addressed review comments from Ville:
> >> - Change the state bool name from external scaling to something more
> >> relavent.
> >> - Keep the info and adjusted_mode structures const.
> >> - use crtc_state instead of pipe_config.
> >> - Push all the config change into lspcon_ycbcr420_config function.
> >> V6: Rebase, small changes to accommodate changes in patch 2.
> >> V7: Fixed checkpatch warnings for alignment
> >> V8: Rebase
> >>
> >> PS: Ignored following warnings to match the current formatting:
> >> drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
> >> -:53: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> >> #53: FILE: drivers/gpu/drm/i915/i915_reg.h:8721:
> >> +#define TRANS_MSA_SAMPLING_444 (2<<1)
> >> ^
> >> -:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> >> #54: FILE: drivers/gpu/drm/i915/i915_reg.h:8722:
> >> +#define TRANS_MSA_CLRSP_YCBCR (2<<3)
> >> V9: Rebase
> >> V10: Rebase
> >> V11: Rebase
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> >> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
> >> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> >> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> >> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
> >> drivers/gpu/drm/i915/intel_lspcon.c | 26 ++++++++++++++++++++++++++
> >> 6 files changed, 56 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index 2078541..1e13e51 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -9229,6 +9229,8 @@ enum skl_power_gate {
> >> #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
> >>
> >> #define TRANS_MSA_SYNC_CLK (1 << 0)
> >> +#define TRANS_MSA_SAMPLING_444 (2 << 1)
> >> +#define TRANS_MSA_CLRSP_YCBCR (2 << 3)
> >> #define TRANS_MSA_6_BPC (0 << 5)
> >> #define TRANS_MSA_8_BPC (1 << 5)
> >> #define TRANS_MSA_10_BPC (2 << 5)
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index be21131..186111a 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -1784,6 +1784,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
> >> break;
> >> }
> >>
> >> + /*
> >> + * As per DP 1.2 spec section 2.3.4.3 while sending
> >> + * YCBCR 444 signals we should program MSA MISC1/0 fields with
> >> + * colorspace information. The output colorspace encoding is BT601.
> >> + */
> >> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> >> + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
> >> I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 3317c6c..41abd03 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -7797,6 +7797,8 @@ static void intel_get_crtc_ycbcr_config(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));
> >>
> >> @@ -7814,6 +7816,16 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
> >> 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;
> >> }
> >> }
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index c3f6330..1156d59 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -2074,6 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
> >> enum port port = encoder->port;
> >> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> >> struct intel_connector *intel_connector = intel_dp->attached_connector;
> >> @@ -2086,6 +2087,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >> pipe_config->has_pch_encoder = true;
> >>
> >> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >> + if (lspcon->active)
> >> + lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
> >> +
> >> 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index b8e5f95..95e252e 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -908,6 +908,9 @@ struct intel_crtc_state {
> >>
> >> /* Output format RGB/YCBCR etc */
> >> enum intel_output_format output_format;
> >> +
> >> + /* Output down scaling is done in LSPCON device */
> >> + bool lspcon_downsampling;
> >> };
> >>
> >> struct intel_crtc {
> >> @@ -2225,6 +2228,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
> >> const struct drm_connector_state *conn_state);
> >> bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
> >> const struct intel_crtc_state *pipe_config);
> >> +void lspcon_ycbcr420_config(struct drm_connector *connector,
> >> + struct intel_crtc_state *crtc_state);
> >>
> >> /* intel_pipe_crc.c */
> >> #ifdef CONFIG_DEBUG_FS
> >> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> >> index 829c40a..fff32b3 100644
> >> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> >> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >> @@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
> >> return true;
> >> }
> >>
> >> +void lspcon_ycbcr420_config(struct drm_connector *connector,
> >> + struct intel_crtc_state *crtc_state)
> >> +{
> >> + const struct drm_display_info *info = &connector->display_info;
> >> + const struct drm_display_mode *adjusted_mode =
> >> + &crtc_state->base.adjusted_mode;
> >> +
> >> + if (drm_mode_is_420_only(info, adjusted_mode) &&
> >> + connector->ycbcr_420_allowed) {
> >> + crtc_state->port_clock /= 2;
> > This looks bogus. We're talking about DP here so we should not be
> > frobbing port_clock. And since we output 4:4:4 from the port
> > anyway we don't even have to take 4:2:0 into consideration
> > when calculating port_clock.
> I agree on this.
> > Ah, this guy is called before we even calculate port_clock.
> > That explains why this didn't cause any problems. So this is
> > just dead code here.
> >
> > And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
> > also dead code since the caller overwrites it there as well.
> I am not sure if that's the case for Native HDMI 2.0.
> In intel_hdmi_420_config we are updating the config->port_clock, which
> is the same thing being updated for 8/12/16 BPC deep color too, just
> after this function.
> I have tested the output with HDMI 2.0 analyzer, which reported right
> pixel clock (297Mhz). Now, as any of the port clock calculations do not
> get information
> about HDMI output type, there is no other way they will calculate the
> right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
> active.
It effectively does this:
1. port_clock = who knows
2. port_clock /= 2;
3. if (12bpc)
port_clock = clock_12bpc;
else if (10bpc)
port_clock = clock_10bpc;
else
port_clock = clock_8bpc;
Ie. step 2. is dead code.
>
> - Shashank
> >> + crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
> >> + crtc_state->lspcon_downsampling = true;
> >> + }
> >> +}
> >> +
> >> static bool lspcon_probe(struct intel_lspcon *lspcon)
> >> {
> >> int retry;
> >> @@ -464,6 +479,15 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
> >> return;
> >> }
> >>
> >> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
> >> + if (crtc_state->lspcon_downsampling)
> >> + frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
> >> + else
> >> + frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> >> + } else {
> >> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >> + }
> >> +
> >> drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> >> crtc_state->limited_color_range ?
> >> HDMI_QUANTIZATION_RANGE_LIMITED :
> >> @@ -517,6 +541,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >> struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
> >> struct drm_device *dev = intel_dig_port->base.base.dev;
> >> struct drm_i915_private *dev_priv = to_i915(dev);
> >> + struct drm_connector *connector = &dp->attached_connector->base;
> >>
> >> if (!HAS_LSPCON(dev_priv)) {
> >> DRM_ERROR("LSPCON is not supported on this platform\n");
> >> @@ -541,6 +566,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >> return false;
> >> }
> >>
> >> + connector->ycbcr_420_allowed = true;
> >> lspcon->active = true;
> >> DRM_DEBUG_KMS("Success: LSPCON init\n");
> >> return true;
> >> --
> >> 2.7.4
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 18:09 ` Ville Syrjälä
@ 2018-10-12 18:32 ` Sharma, Shashank
2018-10-12 18:37 ` Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Sharma, Shashank @ 2018-10-12 18:32 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Regards
Shashank
On 10/12/2018 11:39 PM, Ville Syrjälä wrote:
> On Fri, Oct 12, 2018 at 11:21:56PM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 10/12/2018 7:28 PM, Ville Syrjälä wrote:
>>> On Fri, Oct 12, 2018 at 11:53:14AM +0530, Shashank Sharma wrote:
>>>> From: Shashank Sharma <shashank.sharma@linux.intel.com>
>>>>
>>>> LSPCON chips can generate YCBCR outputs, if asked nicely :).
>>>>
>>>> In order to generate YCBCR 4:2:0 outputs, a source must:
>>>> - send YCBCR 4:4:4 signals to LSPCON
>>>> - program color space as 4:2:0 in AVI infoframes
>>>>
>>>> Whereas for YCBCR 4:4:4 outputs, the source must:
>>>> - send YCBCR 4:4:4 signals to LSPCON
>>>> - program color space as 4:4:4 in AVI infoframes
>>>>
>>>> So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
>>>> pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
>>>> information indicates LSPCON FW to start scaling down from YCBCR
>>>> 4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
>>>> LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
>>>>
>>>> V2: rebase
>>>> V3: Addressed review comments from Ville
>>>> - add enum crtc_output_format instead of bool ycbcr420
>>>> - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
>>>> cases in this way we will have YCBCR 4:4:4 framework ready (except
>>>> the ABI part)
>>>> V4: Added r-b from Maarten (for v3)
>>>> Addressed review comments from Ville:
>>>> - Do not add a non-atomic state variable to determine lspcon output.
>>>> Instead add bool in CRTC state to indicate lspcon based scaling.
>>>> V5: Addressed review comments from Ville:
>>>> - Change the state bool name from external scaling to something more
>>>> relavent.
>>>> - Keep the info and adjusted_mode structures const.
>>>> - use crtc_state instead of pipe_config.
>>>> - Push all the config change into lspcon_ycbcr420_config function.
>>>> V6: Rebase, small changes to accommodate changes in patch 2.
>>>> V7: Fixed checkpatch warnings for alignment
>>>> V8: Rebase
>>>>
>>>> PS: Ignored following warnings to match the current formatting:
>>>> drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
>>>> -:53: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
>>>> #53: FILE: drivers/gpu/drm/i915/i915_reg.h:8721:
>>>> +#define TRANS_MSA_SAMPLING_444 (2<<1)
>>>> ^
>>>> -:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
>>>> #54: FILE: drivers/gpu/drm/i915/i915_reg.h:8722:
>>>> +#define TRANS_MSA_CLRSP_YCBCR (2<<3)
>>>> V9: Rebase
>>>> V10: Rebase
>>>> V11: Rebase
>>>>
>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_reg.h | 2 ++
>>>> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
>>>> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
>>>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>>>> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
>>>> drivers/gpu/drm/i915/intel_lspcon.c | 26 ++++++++++++++++++++++++++
>>>> 6 files changed, 56 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>> index 2078541..1e13e51 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -9229,6 +9229,8 @@ enum skl_power_gate {
>>>> #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
>>>>
>>>> #define TRANS_MSA_SYNC_CLK (1 << 0)
>>>> +#define TRANS_MSA_SAMPLING_444 (2 << 1)
>>>> +#define TRANS_MSA_CLRSP_YCBCR (2 << 3)
>>>> #define TRANS_MSA_6_BPC (0 << 5)
>>>> #define TRANS_MSA_8_BPC (1 << 5)
>>>> #define TRANS_MSA_10_BPC (2 << 5)
>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>>> index be21131..186111a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>> @@ -1784,6 +1784,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
>>>> break;
>>>> }
>>>>
>>>> + /*
>>>> + * As per DP 1.2 spec section 2.3.4.3 while sending
>>>> + * YCBCR 444 signals we should program MSA MISC1/0 fields with
>>>> + * colorspace information. The output colorspace encoding is BT601.
>>>> + */
>>>> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
>>>> + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
>>>> I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
>>>> }
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index 3317c6c..41abd03 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -7797,6 +7797,8 @@ static void intel_get_crtc_ycbcr_config(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));
>>>>
>>>> @@ -7814,6 +7816,16 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
>>>> 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;
>>>> }
>>>> }
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index c3f6330..1156d59 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -2074,6 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>>>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>>>> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
>>>> enum port port = encoder->port;
>>>> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>>>> struct intel_connector *intel_connector = intel_dp->attached_connector;
>>>> @@ -2086,6 +2087,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>>> pipe_config->has_pch_encoder = true;
>>>>
>>>> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>>> + if (lspcon->active)
>>>> + lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
>>>> +
>>>> 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>>> index b8e5f95..95e252e 100644
>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>> @@ -908,6 +908,9 @@ struct intel_crtc_state {
>>>>
>>>> /* Output format RGB/YCBCR etc */
>>>> enum intel_output_format output_format;
>>>> +
>>>> + /* Output down scaling is done in LSPCON device */
>>>> + bool lspcon_downsampling;
>>>> };
>>>>
>>>> struct intel_crtc {
>>>> @@ -2225,6 +2228,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>>>> const struct drm_connector_state *conn_state);
>>>> bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
>>>> const struct intel_crtc_state *pipe_config);
>>>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
>>>> + struct intel_crtc_state *crtc_state);
>>>>
>>>> /* intel_pipe_crc.c */
>>>> #ifdef CONFIG_DEBUG_FS
>>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>>>> index 829c40a..fff32b3 100644
>>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>>>> @@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
>>>> return true;
>>>> }
>>>>
>>>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
>>>> + struct intel_crtc_state *crtc_state)
>>>> +{
>>>> + const struct drm_display_info *info = &connector->display_info;
>>>> + const struct drm_display_mode *adjusted_mode =
>>>> + &crtc_state->base.adjusted_mode;
>>>> +
>>>> + if (drm_mode_is_420_only(info, adjusted_mode) &&
>>>> + connector->ycbcr_420_allowed) {
>>>> + crtc_state->port_clock /= 2;
>>> This looks bogus. We're talking about DP here so we should not be
>>> frobbing port_clock. And since we output 4:4:4 from the port
>>> anyway we don't even have to take 4:2:0 into consideration
>>> when calculating port_clock.
>> I agree on this.
>>> Ah, this guy is called before we even calculate port_clock.
>>> That explains why this didn't cause any problems. So this is
>>> just dead code here.
>>>
>>> And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
>>> also dead code since the caller overwrites it there as well.
>> I am not sure if that's the case for Native HDMI 2.0.
>> In intel_hdmi_420_config we are updating the config->port_clock, which
>> is the same thing being updated for 8/12/16 BPC deep color too, just
>> after this function.
>> I have tested the output with HDMI 2.0 analyzer, which reported right
>> pixel clock (297Mhz). Now, as any of the port clock calculations do not
>> get information
>> about HDMI output type, there is no other way they will calculate the
>> right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
>> active.
> It effectively does this:
>
> 1. port_clock = who knows
> 2. port_clock /= 2;
Here, in intel_hdmi_compute_ycbcr_config() function, we are additionally
doing this also:
clock_8bpc /= 2;
clock_10bpc /= 2;
clock_12bpc /= 2;
> 3. if (12bpc)
> port_clock = clock_12bpc;
> else if (10bpc)
> port_clock = clock_10bpc;
> else
> port_clock = clock_8bpc;
This means effectively the selected clock is /2, so the code is not dead
(thankfully :-))
- Shashank
>
> Ie. step 2. is dead code.
>
>> - Shashank
>>>> + crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
>>>> + crtc_state->lspcon_downsampling = true;
>>>> + }
>>>> +}
>>>> +
>>>> static bool lspcon_probe(struct intel_lspcon *lspcon)
>>>> {
>>>> int retry;
>>>> @@ -464,6 +479,15 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>>>> return;
>>>> }
>>>>
>>>> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
>>>> + if (crtc_state->lspcon_downsampling)
>>>> + frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
>>>> + else
>>>> + frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
>>>> + } else {
>>>> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>>>> + }
>>>> +
>>>> drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
>>>> crtc_state->limited_color_range ?
>>>> HDMI_QUANTIZATION_RANGE_LIMITED :
>>>> @@ -517,6 +541,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>>>> struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>>>> struct drm_device *dev = intel_dig_port->base.base.dev;
>>>> struct drm_i915_private *dev_priv = to_i915(dev);
>>>> + struct drm_connector *connector = &dp->attached_connector->base;
>>>>
>>>> if (!HAS_LSPCON(dev_priv)) {
>>>> DRM_ERROR("LSPCON is not supported on this platform\n");
>>>> @@ -541,6 +566,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>>>> return false;
>>>> }
>>>>
>>>> + connector->ycbcr_420_allowed = true;
>>>> lspcon->active = true;
>>>> DRM_DEBUG_KMS("Success: LSPCON init\n");
>>>> return true;
>>>> --
>>>> 2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 18:32 ` Sharma, Shashank
@ 2018-10-12 18:37 ` Ville Syrjälä
2018-10-15 13:12 ` Jani Nikula
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2018-10-12 18:37 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx
On Sat, Oct 13, 2018 at 12:02:25AM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 10/12/2018 11:39 PM, Ville Syrjälä wrote:
> > On Fri, Oct 12, 2018 at 11:21:56PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 10/12/2018 7:28 PM, Ville Syrjälä wrote:
> >>> On Fri, Oct 12, 2018 at 11:53:14AM +0530, Shashank Sharma wrote:
> >>>> From: Shashank Sharma <shashank.sharma@linux.intel.com>
> >>>>
> >>>> LSPCON chips can generate YCBCR outputs, if asked nicely :).
> >>>>
> >>>> In order to generate YCBCR 4:2:0 outputs, a source must:
> >>>> - send YCBCR 4:4:4 signals to LSPCON
> >>>> - program color space as 4:2:0 in AVI infoframes
> >>>>
> >>>> Whereas for YCBCR 4:4:4 outputs, the source must:
> >>>> - send YCBCR 4:4:4 signals to LSPCON
> >>>> - program color space as 4:4:4 in AVI infoframes
> >>>>
> >>>> So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
> >>>> pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
> >>>> information indicates LSPCON FW to start scaling down from YCBCR
> >>>> 4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
> >>>> LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
> >>>>
> >>>> V2: rebase
> >>>> V3: Addressed review comments from Ville
> >>>> - add enum crtc_output_format instead of bool ycbcr420
> >>>> - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
> >>>> cases in this way we will have YCBCR 4:4:4 framework ready (except
> >>>> the ABI part)
> >>>> V4: Added r-b from Maarten (for v3)
> >>>> Addressed review comments from Ville:
> >>>> - Do not add a non-atomic state variable to determine lspcon output.
> >>>> Instead add bool in CRTC state to indicate lspcon based scaling.
> >>>> V5: Addressed review comments from Ville:
> >>>> - Change the state bool name from external scaling to something more
> >>>> relavent.
> >>>> - Keep the info and adjusted_mode structures const.
> >>>> - use crtc_state instead of pipe_config.
> >>>> - Push all the config change into lspcon_ycbcr420_config function.
> >>>> V6: Rebase, small changes to accommodate changes in patch 2.
> >>>> V7: Fixed checkpatch warnings for alignment
> >>>> V8: Rebase
> >>>>
> >>>> PS: Ignored following warnings to match the current formatting:
> >>>> drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
> >>>> -:53: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> >>>> #53: FILE: drivers/gpu/drm/i915/i915_reg.h:8721:
> >>>> +#define TRANS_MSA_SAMPLING_444 (2<<1)
> >>>> ^
> >>>> -:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> >>>> #54: FILE: drivers/gpu/drm/i915/i915_reg.h:8722:
> >>>> +#define TRANS_MSA_CLRSP_YCBCR (2<<3)
> >>>> V9: Rebase
> >>>> V10: Rebase
> >>>> V11: Rebase
> >>>>
> >>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> >>>> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
> >>>> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> >>>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> >>>> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
> >>>> drivers/gpu/drm/i915/intel_lspcon.c | 26 ++++++++++++++++++++++++++
> >>>> 6 files changed, 56 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>>> index 2078541..1e13e51 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>>> @@ -9229,6 +9229,8 @@ enum skl_power_gate {
> >>>> #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
> >>>>
> >>>> #define TRANS_MSA_SYNC_CLK (1 << 0)
> >>>> +#define TRANS_MSA_SAMPLING_444 (2 << 1)
> >>>> +#define TRANS_MSA_CLRSP_YCBCR (2 << 3)
> >>>> #define TRANS_MSA_6_BPC (0 << 5)
> >>>> #define TRANS_MSA_8_BPC (1 << 5)
> >>>> #define TRANS_MSA_10_BPC (2 << 5)
> >>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >>>> index be21131..186111a 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>>> @@ -1784,6 +1784,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
> >>>> break;
> >>>> }
> >>>>
> >>>> + /*
> >>>> + * As per DP 1.2 spec section 2.3.4.3 while sending
> >>>> + * YCBCR 444 signals we should program MSA MISC1/0 fields with
> >>>> + * colorspace information. The output colorspace encoding is BT601.
> >>>> + */
> >>>> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> >>>> + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
> >>>> I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> >>>> }
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index 3317c6c..41abd03 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -7797,6 +7797,8 @@ static void intel_get_crtc_ycbcr_config(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));
> >>>>
> >>>> @@ -7814,6 +7816,16 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
> >>>> 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;
> >>>> }
> >>>> }
> >>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>>> index c3f6330..1156d59 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>>> @@ -2074,6 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >>>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>>> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> >>>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >>>> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
> >>>> enum port port = encoder->port;
> >>>> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> >>>> struct intel_connector *intel_connector = intel_dp->attached_connector;
> >>>> @@ -2086,6 +2087,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >>>> pipe_config->has_pch_encoder = true;
> >>>>
> >>>> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>>> + if (lspcon->active)
> >>>> + lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
> >>>> +
> >>>> 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>>> index b8e5f95..95e252e 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>> @@ -908,6 +908,9 @@ struct intel_crtc_state {
> >>>>
> >>>> /* Output format RGB/YCBCR etc */
> >>>> enum intel_output_format output_format;
> >>>> +
> >>>> + /* Output down scaling is done in LSPCON device */
> >>>> + bool lspcon_downsampling;
> >>>> };
> >>>>
> >>>> struct intel_crtc {
> >>>> @@ -2225,6 +2228,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
> >>>> const struct drm_connector_state *conn_state);
> >>>> bool lspcon_infoframe_enabled(struct intel_encoder *encoder,
> >>>> const struct intel_crtc_state *pipe_config);
> >>>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
> >>>> + struct intel_crtc_state *crtc_state);
> >>>>
> >>>> /* intel_pipe_crc.c */
> >>>> #ifdef CONFIG_DEBUG_FS
> >>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> >>>> index 829c40a..fff32b3 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >>>> @@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
> >>>> return true;
> >>>> }
> >>>>
> >>>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
> >>>> + struct intel_crtc_state *crtc_state)
> >>>> +{
> >>>> + const struct drm_display_info *info = &connector->display_info;
> >>>> + const struct drm_display_mode *adjusted_mode =
> >>>> + &crtc_state->base.adjusted_mode;
> >>>> +
> >>>> + if (drm_mode_is_420_only(info, adjusted_mode) &&
> >>>> + connector->ycbcr_420_allowed) {
> >>>> + crtc_state->port_clock /= 2;
> >>> This looks bogus. We're talking about DP here so we should not be
> >>> frobbing port_clock. And since we output 4:4:4 from the port
> >>> anyway we don't even have to take 4:2:0 into consideration
> >>> when calculating port_clock.
> >> I agree on this.
> >>> Ah, this guy is called before we even calculate port_clock.
> >>> That explains why this didn't cause any problems. So this is
> >>> just dead code here.
> >>>
> >>> And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
> >>> also dead code since the caller overwrites it there as well.
> >> I am not sure if that's the case for Native HDMI 2.0.
> >> In intel_hdmi_420_config we are updating the config->port_clock, which
> >> is the same thing being updated for 8/12/16 BPC deep color too, just
> >> after this function.
> >> I have tested the output with HDMI 2.0 analyzer, which reported right
> >> pixel clock (297Mhz). Now, as any of the port clock calculations do not
> >> get information
> >> about HDMI output type, there is no other way they will calculate the
> >> right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
> >> active.
> > It effectively does this:
> >
> > 1. port_clock = who knows
> > 2. port_clock /= 2;
> Here, in intel_hdmi_compute_ycbcr_config() function, we are additionally
> doing this also:
>
> clock_8bpc /= 2;
> clock_10bpc /= 2;
> clock_12bpc /= 2;
>
> > 3. if (12bpc)
> > port_clock = clock_12bpc;
> > else if (10bpc)
> > port_clock = clock_10bpc;
> > else
> > port_clock = clock_8bpc;
> This means effectively the selected clock is /2, so the code is not dead
> (thankfully :-))
Only the port_clock/=2 part. I never claimed the rest was dead.
> - Shashank
> >
> > Ie. step 2. is dead code.
> >
> >> - Shashank
> >>>> + crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
> >>>> + crtc_state->lspcon_downsampling = true;
> >>>> + }
> >>>> +}
> >>>> +
> >>>> static bool lspcon_probe(struct intel_lspcon *lspcon)
> >>>> {
> >>>> int retry;
> >>>> @@ -464,6 +479,15 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
> >>>> return;
> >>>> }
> >>>>
> >>>> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
> >>>> + if (crtc_state->lspcon_downsampling)
> >>>> + frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
> >>>> + else
> >>>> + frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> >>>> + } else {
> >>>> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >>>> + }
> >>>> +
> >>>> drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> >>>> crtc_state->limited_color_range ?
> >>>> HDMI_QUANTIZATION_RANGE_LIMITED :
> >>>> @@ -517,6 +541,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >>>> struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
> >>>> struct drm_device *dev = intel_dig_port->base.base.dev;
> >>>> struct drm_i915_private *dev_priv = to_i915(dev);
> >>>> + struct drm_connector *connector = &dp->attached_connector->base;
> >>>>
> >>>> if (!HAS_LSPCON(dev_priv)) {
> >>>> DRM_ERROR("LSPCON is not supported on this platform\n");
> >>>> @@ -541,6 +566,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >>>> return false;
> >>>> }
> >>>>
> >>>> + connector->ycbcr_420_allowed = true;
> >>>> lspcon->active = true;
> >>>> DRM_DEBUG_KMS("Success: LSPCON init\n");
> >>>> return true;
> >>>> --
> >>>> 2.7.4
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-12 18:37 ` Ville Syrjälä
@ 2018-10-15 13:12 ` Jani Nikula
2018-10-15 14:28 ` Sharma, Shashank
0 siblings, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2018-10-15 13:12 UTC (permalink / raw)
To: Ville Syrjälä, Sharma, Shashank; +Cc: intel-gfx
On Fri, 12 Oct 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Sat, Oct 13, 2018 at 12:02:25AM +0530, Sharma, Shashank wrote:
>> >>>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
>> >>>> + struct intel_crtc_state *crtc_state)
>> >>>> +{
>> >>>> + const struct drm_display_info *info = &connector->display_info;
>> >>>> + const struct drm_display_mode *adjusted_mode =
>> >>>> + &crtc_state->base.adjusted_mode;
>> >>>> +
>> >>>> + if (drm_mode_is_420_only(info, adjusted_mode) &&
>> >>>> + connector->ycbcr_420_allowed) {
>> >>>> + crtc_state->port_clock /= 2;
>> >>> This looks bogus. We're talking about DP here so we should not be
>> >>> frobbing port_clock. And since we output 4:4:4 from the port
>> >>> anyway we don't even have to take 4:2:0 into consideration
>> >>> when calculating port_clock.
>> >> I agree on this.
>> >>> Ah, this guy is called before we even calculate port_clock.
>> >>> That explains why this didn't cause any problems. So this is
>> >>> just dead code here.
>> >>>
>> >>> And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
>> >>> also dead code since the caller overwrites it there as well.
>> >> I am not sure if that's the case for Native HDMI 2.0.
>> >> In intel_hdmi_420_config we are updating the config->port_clock, which
>> >> is the same thing being updated for 8/12/16 BPC deep color too, just
>> >> after this function.
>> >> I have tested the output with HDMI 2.0 analyzer, which reported right
>> >> pixel clock (297Mhz). Now, as any of the port clock calculations do not
>> >> get information
>> >> about HDMI output type, there is no other way they will calculate the
>> >> right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
>> >> active.
>> > It effectively does this:
>> >
>> > 1. port_clock = who knows
>> > 2. port_clock /= 2;
>> Here, in intel_hdmi_compute_ycbcr_config() function, we are additionally
>> doing this also:
>>
>> clock_8bpc /= 2;
>> clock_10bpc /= 2;
>> clock_12bpc /= 2;
>>
>> > 3. if (12bpc)
>> > port_clock = clock_12bpc;
>> > else if (10bpc)
>> > port_clock = clock_10bpc;
>> > else
>> > port_clock = clock_8bpc;
>> This means effectively the selected clock is /2, so the code is not dead
>> (thankfully :-))
>
> Only the port_clock/=2 part. I never claimed the rest was dead.
The whole series pushed to dinq, dismissing this last part with Ville's
approval, to get some closure here. Thanks for the patches and
review. Further patches to clean up the existing and new dead code will
be appreciated.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
2018-10-15 13:12 ` Jani Nikula
@ 2018-10-15 14:28 ` Sharma, Shashank
0 siblings, 0 replies; 18+ messages in thread
From: Sharma, Shashank @ 2018-10-15 14:28 UTC (permalink / raw)
To: Jani Nikula, Ville Syrjälä; +Cc: intel-gfx
Regards
Shashank
On 10/15/2018 6:42 PM, Jani Nikula wrote:
> On Fri, 12 Oct 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Sat, Oct 13, 2018 at 12:02:25AM +0530, Sharma, Shashank wrote:
>>>>>>> +void lspcon_ycbcr420_config(struct drm_connector *connector,
>>>>>>> + struct intel_crtc_state *crtc_state)
>>>>>>> +{
>>>>>>> + const struct drm_display_info *info = &connector->display_info;
>>>>>>> + const struct drm_display_mode *adjusted_mode =
>>>>>>> + &crtc_state->base.adjusted_mode;
>>>>>>> +
>>>>>>> + if (drm_mode_is_420_only(info, adjusted_mode) &&
>>>>>>> + connector->ycbcr_420_allowed) {
>>>>>>> + crtc_state->port_clock /= 2;
>>>>>> This looks bogus. We're talking about DP here so we should not be
>>>>>> frobbing port_clock. And since we output 4:4:4 from the port
>>>>>> anyway we don't even have to take 4:2:0 into consideration
>>>>>> when calculating port_clock.
>>>>> I agree on this.
>>>>>> Ah, this guy is called before we even calculate port_clock.
>>>>>> That explains why this didn't cause any problems. So this is
>>>>>> just dead code here.
>>>>>>
>>>>>> And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
>>>>>> also dead code since the caller overwrites it there as well.
>>>>> I am not sure if that's the case for Native HDMI 2.0.
>>>>> In intel_hdmi_420_config we are updating the config->port_clock, which
>>>>> is the same thing being updated for 8/12/16 BPC deep color too, just
>>>>> after this function.
>>>>> I have tested the output with HDMI 2.0 analyzer, which reported right
>>>>> pixel clock (297Mhz). Now, as any of the port clock calculations do not
>>>>> get information
>>>>> about HDMI output type, there is no other way they will calculate the
>>>>> right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
>>>>> active.
>>>> It effectively does this:
>>>>
>>>> 1. port_clock = who knows
>>>> 2. port_clock /= 2;
>>> Here, in intel_hdmi_compute_ycbcr_config() function, we are additionally
>>> doing this also:
>>>
>>> clock_8bpc /= 2;
>>> clock_10bpc /= 2;
>>> clock_12bpc /= 2;
>>>
>>>> 3. if (12bpc)
>>>> port_clock = clock_12bpc;
>>>> else if (10bpc)
>>>> port_clock = clock_10bpc;
>>>> else
>>>> port_clock = clock_8bpc;
>>> This means effectively the selected clock is /2, so the code is not dead
>>> (thankfully :-))
>> Only the port_clock/=2 part. I never claimed the rest was dead.
> The whole series pushed to dinq, dismissing this last part with Ville's
> approval, to get some closure here. Thanks for the patches and
> review. Further patches to clean up the existing and new dead code will
> be appreciated.
Thanks Jani, I will have a look at the series post merge, and send any
follow up / cleanup patches if required.
- Shashank
> BR,
> Jani.
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-15 14:29 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 6:23 [PATCH v12 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4 Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 5/8] drm/i915: Add AVI infoframe support for LSPCON Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 6/8] drm/i915: Write AVI infoframes for MCA LSPCON Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 7/8] drm/i915: Write AVI infoframes for Parade LSPCON Shashank Sharma
2018-10-12 6:23 ` [PATCH v12 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
2018-10-12 13:58 ` Ville Syrjälä
2018-10-12 17:51 ` Sharma, Shashank
2018-10-12 18:09 ` Ville Syrjälä
2018-10-12 18:32 ` Sharma, Shashank
2018-10-12 18:37 ` Ville Syrjälä
2018-10-15 13:12 ` Jani Nikula
2018-10-15 14:28 ` Sharma, Shashank
2018-10-12 6:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v12,1/8] drm/i915: Introduce CRTC output format Patchwork
2018-10-12 6:50 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-12 7:51 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.