Regards

Shashank


On 7/18/2017 11:42 PM, Imre Deak wrote:
On Mon, Jul 17, 2017 at 08:06:24PM +0530, Shashank Sharma wrote:
To get HDMI YCBCR420 output, the PIPEMISC register should be
programmed to:
- Generate YCBCR output (bit 11)
- In case of YCBCR420 outputs, it should be programmed in full
   blend mode to use the scaler in 5x3 ratio (bits 26 and 27)

This patch:
- Adds definition of these bits.
- Programs PIPEMISC for YCBCR420 outputs.
- Adds readouts to compare HW and SW states.

V2: rebase
V3: rebase
V4: rebase
V5: added r-b from Ander
V6: Handle only YCBCR420 outputs (ville)
V7: rebase
V8: Addressed review comments from Ville
     - Add readouts for state->ycbcr420 and 420 pixel_clock.
     - Handle warning due to mismatch in clock for ycbcr420 clock.
     - Rename PIPEMISC macros to match the Bspec.
     - Add a debug print stating if YCBCR 4:2:0 output enabled.
     Added r-b from Ville

Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
Cc: Ander Conselvan de Oliveira <conselv...@gmail.com>
Cc: Daniel Vetter <daniel.vet...@intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselv...@gmail.com>
Reviewed-by: Ville Syrjala <ville.syrj...@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
---
  drivers/gpu/drm/i915/i915_reg.h      |  3 ++
  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++++--
  2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c712d01..6dfcdd3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5227,6 +5227,9 @@ enum {
#define _PIPE_MISC_A 0x70030
  #define _PIPE_MISC_B                  0x71030
+#define   PIPEMISC_YUV420_ENABLE       (1<<27)
+#define   PIPEMISC_YUV420_MODE_BLEND   (1<<26)
+#define   PIPEMISC_OUTPUT_YUV          (1<<11)
Missing the rename here requested by Ville.
Ah, I thought it was more strict on changing YCBCR->YUV, will get this done.
  #define   PIPEMISC_DITHER_BPC_MASK    (7<<5)
  #define   PIPEMISC_DITHER_8_BPC               (0<<5)
  #define   PIPEMISC_DITHER_10_BPC      (1<<5)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d78f1c2..788502a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6216,7 +6216,6 @@ static uint32_t ilk_pipe_pixel_rate(const struct 
intel_crtc_state *pipe_config)
         * We only use IF-ID interlacing. If we ever use
         * PF-ID we'll need to adjust the pixel_rate here.
         */
-
Extra w/s change.
Got it.

        if (pipe_config->pch_pfit.enabled) {
                uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
                uint32_t pfit_size = pipe_config->pch_pfit.size;
@@ -8076,6 +8075,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
  {
        struct drm_i915_private *dev_priv = to_i915(crtc->dev);
        struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+       struct intel_crtc_state *config = intel_crtc->config;
if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
                u32 val = 0;
@@ -8101,6 +8101,12 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
                if (intel_crtc->config->dither)
                        val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
+ if (config->ycbcr420) {
+                       val |= PIPEMISC_OUTPUT_YUV |
+                               PIPEMISC_YUV420_ENABLE |
+                               PIPEMISC_YUV420_MODE_BLEND;
+               }
+
                I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
        }
  }
@@ -9170,6 +9176,10 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
                pipe_config->scaler_state.scaler_users &= ~(1 << 
SKL_CRTC_INDEX);
        }
+ if (IS_GEMINILAKE(dev_priv))
+               pipe_config->ycbcr420 = I915_READ(PIPEMISC(crtc->pipe)) &
+                                       PIPEMISC_YUV420_ENABLE;
+
        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);
@@ -11377,6 +11387,9 @@ 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);
@@ -11704,6 +11717,22 @@ intel_modeset_update_crtc_state(struct 
drm_atomic_state *state)
        }
  }
+static bool intel_420_clock_check(int clock1, int clock2)
+{
+       int diff;
+
+       if (clock1 == clock2 * 2)
+               return true;
+
+       clock2 *= 2;
+       diff = abs(clock1 - clock2);
+
+       if (((((diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105)
+               return true;
+
+       return false;
+}
+
  static bool intel_fuzzy_clock_check(int clock1, int clock2)
  {
        int diff;
@@ -11832,6 +11861,18 @@ intel_pipe_config_compare(struct drm_i915_private 
*dev_priv,
                ret = false; \
        }
+#define PIPE_CONF_CHECK_CLOCK_420(name) \
+       do { \
+               if (!intel_420_clock_check(current_config->name, \
+                                          pipe_config->name)) { \
+                       pipe_config_err(adjust, __stringify(name), \
+                                 "(expected %i, found %i)\n", \
+                                 current_config->name, \
+                                 pipe_config->name); \
+                       ret = false; \
+               } \
+       } while (0)
+
  #define PIPE_CONF_CHECK_M_N(name) \
        if (!intel_compare_link_m_n(&current_config->name, \
                                    &pipe_config->name,\
@@ -11983,7 +12024,10 @@ intel_pipe_config_compare(struct drm_i915_private 
*dev_priv,
                }
PIPE_CONF_CHECK_I(scaler_state.scaler_id);
-               PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
+               if (current_config->ycbcr420)
+                       PIPE_CONF_CHECK_CLOCK_420(pixel_rate);
+               else
+                       PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
        }
/* BDW+ don't expose a synchronous way to read the state */
@@ -12009,7 +12053,11 @@ intel_pipe_config_compare(struct drm_i915_private 
*dev_priv,
        if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5)
                PIPE_CONF_CHECK_I(pipe_bpp);
- PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.crtc_clock);
+       /* YCBCR 420 pixel clock is half of the actual mode */
+       if (current_config->ycbcr420)
+               PIPE_CONF_CHECK_CLOCK_420(base.adjusted_mode.crtc_clock);
+       else
+               PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.crtc_clock);
        PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
#undef PIPE_CONF_CHECK_X
@@ -12018,6 +12066,7 @@ intel_pipe_config_compare(struct drm_i915_private 
*dev_priv,
  #undef PIPE_CONF_CHECK_FLAGS
  #undef PIPE_CONF_CHECK_CLOCK_FUZZY
  #undef PIPE_CONF_QUIRK
+#undef PIPE_CONF_CHECK_CLOCK_420
We don't need to adjust things during compare. The dotclock is 2x
the port clock in case of YUV420, so all what's needed is

if (pipe_config->ycbcr420)
        dotclock = pipe_config->port_clock * 2;

in ddi_get_clock() as Ville said.
Got it,
Also, right now if I boot with these patches all 4k YUV420 modes will be
pruned on my monitor. This is caused by the monitor reporting a 300MHz
max tmds clock (which is the port clock limit) and the driver comparing
this against the dotclock which is double of this. So I needed

if (drm_mode_is_420_only(&connector->display_info, mode))
        clock /= 2;

in intel_hdmi_mode_valid() to preserve these modes. Not sure why this
didn't come up earlier.
Because, when a monitor declares max clock on 300Mhz, it doesn't contain a mode in its EDID which needs clock > 300Mhz, but YCBCR_420 introduces a corner case here, its possible if all the modes with clock > 300Mhz are YCBCR420_only modes. Looks like you got hold of one such monitor, which is HDMI 2.0 but has all its 4k@60 modes as YCBCR420_only, and hence they fit into 300Mhz limit. In any case, its not illegal, and should have been taken care of in code, so thanks for pointing this out. Can you please share the monitor details ? I will see if I have one here in Bangalore.

With that I see the YUV420 modes, but the initial modeset for the fb
console will result in the gray screen and some jitter. A following
modeset with the same mode will fix things; this is I think what Ville
also saw. Still trying to find the reason for this.
Can you also please check if this is specific to one particular monitor ? I still can't reproduce this issue with my GLK + ACER/SAMSUNG monitors.
- Shashank

--Imre

return ret;
  }
--
2.7.4


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

Reply via email to