drm/i915/dsi: Fix pipe_bpp for handling for 6 bpc pixel-formats

There are 3 problems with the dsi code's pipe_bpp handling for 6 bpc
pixel-formats which this commit addresses:

1) It assumes that the pipe_bpp is the same as the bpp going over the dsi
lanes. This assumption is not valid for MIPI_DSI_FMT_RGB666, where pipe_bpp
should be 18 so that we do proper dithering but we actually send 24 bpp
over the dsi lanes (MIPI_DSI_FMT_RGB666_PACKED sends 18 bpp).

This assumption is enforced by an assert in *_dsi_get_pclk(). This assert
triggers on the initial hw-state readback on BYT/CHT devices which use
MIPI_DSI_FMT_RGB666, such as the Prowise PT301 tablet. PIPECONF is set to
6BPC / 18 bpp by the GOP, while mipi_dsi_pixel_format_to_bpp() returns 24.

This commits switches the calculations in *_dsi_get_pclk() to use the bpp
from mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format) which
returns the bpp going over the mipi lanes and drops the assert.

2) On BXT bxt_dsi_get_pipe_config() wrongly overrides the pipe_bpp which
i9xx_get_pipe_config() reads from PIPECONF with the return value from
mipi_dsi_pixel_format_to_bpp(). This avoids the assert from 1. but is wrong
since the pipe is actually running at the value configured in PIPECONF.

This commit drops the override of pipe_bpp from bxt_dsi_get_pipe_config().

3) The dsi encoder's compute_config() never assigns a value to pipe_bpp,
unlike most other encoders. Falling back on compute_baseline_pipe_bpp()
which always picks 24. 24 is only correct for MIPI_DSI_FMT_RGB88 for the
others we should use 18 bpp so that we correctly do 6bpc color dithering.

This commit adds code to intel_dsi_compute_config() to properly set
pipe_bpp based on intel_dsi->pixel_format.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181201113148.23184-1-hdegoede@redhat.com

diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index df3d390..a9a1977 100644 (file)
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -173,7 +173,7 @@ int vlv_dsi_pll_compute(struct intel_encoder *encoder,
 void vlv_dsi_pll_enable(struct intel_encoder *encoder,
                        const struct intel_crtc_state *config);
 void vlv_dsi_pll_disable(struct intel_encoder *encoder);
-u32 vlv_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
+u32 vlv_dsi_get_pclk(struct intel_encoder *encoder,
                     struct intel_crtc_state *config);
 void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
 
@@ -183,7 +183,7 @@ int bxt_dsi_pll_compute(struct intel_encoder *encoder,
 void bxt_dsi_pll_enable(struct intel_encoder *encoder,
                        const struct intel_crtc_state *config);
 void bxt_dsi_pll_disable(struct intel_encoder *encoder);
-u32 bxt_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
+u32 bxt_dsi_get_pclk(struct intel_encoder *encoder,
                     struct intel_crtc_state *config);
 void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
 

diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index c247ce7..54cbd8e 100644 (file)
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -289,6 +289,11 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
        /* DSI uses short packets for sync events, so clear mode flags for DSI */
        adjusted_mode->flags = 0;
 
+       if (intel_dsi->pixel_format == MIPI_DSI_FMT_RGB888)
+               pipe_config->pipe_bpp = 24;
+       else
+               pipe_config->pipe_bpp = 18;
+
        if (IS_GEN9_LP(dev_priv)) {
                /* Enable Frame time stamp based scanline reporting */
                adjusted_mode->private_flags |=
@@ -1059,10 +1064,8 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
        }
 
        fmt = I915_READ(MIPI_DSI_FUNC_PRG(port)) & VID_MODE_FORMAT_MASK;
-       pipe_config->pipe_bpp =
-                       mipi_dsi_pixel_format_to_bpp(
-                               pixel_format_from_register_bits(fmt));
-       bpp = pipe_config->pipe_bpp;
+       bpp = mipi_dsi_pixel_format_to_bpp(
+                       pixel_format_from_register_bits(fmt));
 
        /* Enable Frame time stamo based scanline reporting */
        adjusted_mode->private_flags |=
@@ -1200,11 +1203,9 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 
        if (IS_GEN9_LP(dev_priv)) {
                bxt_dsi_get_pipe_config(encoder, pipe_config);
-               pclk = bxt_dsi_get_pclk(encoder, pipe_config->pipe_bpp,
-                                       pipe_config);
+               pclk = bxt_dsi_get_pclk(encoder, pipe_config);
        } else {
-               pclk = vlv_dsi_get_pclk(encoder, pipe_config->pipe_bpp,
-                                       pipe_config);
+               pclk = vlv_dsi_get_pclk(encoder, pipe_config);
        }
 
        if (pclk) {

diff --git a/drivers/gpu/drm/i915/vlv_dsi_pll.c b/drivers/gpu/drm/i915/vlv_dsi_pll.c
index a132a80..954d5a8 100644 (file)
--- a/drivers/gpu/drm/i915/vlv_dsi_pll.c
+++ b/drivers/gpu/drm/i915/vlv_dsi_pll.c
@@ -252,20 +252,12 @@ void bxt_dsi_pll_disable(struct intel_encoder *encoder)
                DRM_ERROR("Timeout waiting for PLL lock deassertion\n");
 }
 
-static void assert_bpp_mismatch(enum mipi_dsi_pixel_format fmt, int pipe_bpp)
-{
-       int bpp = mipi_dsi_pixel_format_to_bpp(fmt);
-
-       WARN(bpp != pipe_bpp,
-            "bpp match assertion failure (expected %d, current %d)\n",
-            bpp, pipe_bpp);
-}
-
-u32 vlv_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
+u32 vlv_dsi_get_pclk(struct intel_encoder *encoder,
                     struct intel_crtc_state *config)
 {
        struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
        struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+       int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
        u32 dsi_clock, pclk;
        u32 pll_ctl, pll_div;
        u32 m = 0, p = 0, n;
@@ -319,15 +311,12 @@ u32 vlv_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
 
        dsi_clock = (m * refclk) / (p * n);
 
-       /* pixel_format and pipe_bpp should agree */
-       assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
-
-       pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
+       pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, bpp);
 
        return pclk;
 }
 
-u32 bxt_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
+u32 bxt_dsi_get_pclk(struct intel_encoder *encoder,
                     struct intel_crtc_state *config)
 {
        u32 pclk;
@@ -335,12 +324,7 @@ u32 bxt_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
        u32 dsi_ratio;
        struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
        struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-
-       /* Divide by zero */
-       if (!pipe_bpp) {
-               DRM_ERROR("Invalid BPP(0)\n");
-               return 0;
-       }
+       int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 
        config->dsi_pll.ctrl = I915_READ(BXT_DSI_PLL_CTL);
 
@@ -348,10 +332,7 @@ u32 bxt_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
 
        dsi_clk = (dsi_ratio * BXT_REF_CLOCK_KHZ) / 2;
 
-       /* pixel_format and pipe_bpp should agree */
-       assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
-
-       pclk = DIV_ROUND_CLOSEST(dsi_clk * intel_dsi->lane_count, pipe_bpp);
+       pclk = DIV_ROUND_CLOSEST(dsi_clk * intel_dsi->lane_count, bpp);
 
        DRM_DEBUG_DRIVER("Calculated pclk=%u\n", pclk);
        return pclk;