The video DSI mode had not really been tested. These fixes makes
it more likely to work on real hardware:
- Set the HS clock to something the video mode reported by the
  panel can handle rather than the max HS rate.
- Put the active width (x width) in the right bits and the VSA
  (vertical sync active) in the right bits (those were swapped).
- Calculate the packet sizes in bytes as in the vendor driver,
  rather than in bits.
- Handle negative result in front/back/sync packages and fall
  back to zero like in the vendor driver.

Cc: Stephan Gerhold <[email protected]>
Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
Signed-off-by: Linus Walleij <[email protected]>
---
ChangeLog v1->v2:
- Fix some more comments so we understand what is going on.
- Set up the maximum line limit size in the right register
  instead of setting it in the burst size register portion.
- Set some default wakeup time other than zero (still need
  fixing more).
---
 drivers/gpu/drm/mcde/mcde_dsi.c | 75 ++++++++++++++++++++++-----------
 1 file changed, 50 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index cd261c266f35..5c65cd70fcd3 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -365,11 +365,12 @@ void mcde_dsi_te_request(struct mipi_dsi_device *mdsi)
 static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
                                      const struct drm_display_mode *mode)
 {
-       u8 bpp = mipi_dsi_pixel_format_to_bpp(d->mdsi->format);
+       /* cpp, characters per pixel, number of bytes per pixel */
+       u8 cpp = mipi_dsi_pixel_format_to_bpp(d->mdsi->format) / 8;
        u64 bpl;
-       u32 hfp;
-       u32 hbp;
-       u32 hsa;
+       int hfp;
+       int hbp;
+       int hsa;
        u32 blkline_pck, line_duration;
        u32 blkeol_pck, blkeol_duration;
        u32 val;
@@ -408,11 +409,11 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
                return;
        }
 
-       /* TODO: TVG could be enabled here */
+       /* TODO: TVG (test video generator) could be enabled here */
 
-       /* Send blanking packet */
+       /* During blanking: go to LP mode */
        val |= DSI_VID_MAIN_CTL_REG_BLKLINE_MODE_LP_0;
-       /* Send EOL packet */
+       /* During EOL: go to LP mode */
        val |= DSI_VID_MAIN_CTL_REG_BLKEOL_MODE_LP_0;
        /* Recovery mode 1 */
        val |= 1 << DSI_VID_MAIN_CTL_RECOVERY_MODE_SHIFT;
@@ -420,13 +421,13 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
        writel(val, d->regs + DSI_VID_MAIN_CTL);
 
        /* Vertical frame parameters are pretty straight-forward */
-       val = mode->vdisplay << DSI_VID_VSIZE_VSA_LENGTH_SHIFT;
+       val = mode->vdisplay << DSI_VID_VSIZE_VACT_LENGTH_SHIFT;
        /* vertical front porch */
        val |= (mode->vsync_start - mode->vdisplay)
                << DSI_VID_VSIZE_VFP_LENGTH_SHIFT;
        /* vertical sync active */
        val |= (mode->vsync_end - mode->vsync_start)
-               << DSI_VID_VSIZE_VACT_LENGTH_SHIFT;
+               << DSI_VID_VSIZE_VSA_LENGTH_SHIFT;
        /* vertical back porch */
        val |= (mode->vtotal - mode->vsync_end)
                << DSI_VID_VSIZE_VBP_LENGTH_SHIFT;
@@ -437,21 +438,25 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
         * horizontal resolution is given in pixels and must be re-calculated
         * into bytes since this is what the hardware expects.
         *
+        * hfp = horizontal front porch in bytes
+        * hbp = horizontal back porch in bytes
+        * hsa = horizontal sync active in bytes
+        *
         * 6 + 2 is HFP header + checksum
         */
-       hfp = (mode->hsync_start - mode->hdisplay) * bpp - 6 - 2;
+       hfp = (mode->hsync_start - mode->hdisplay) * cpp - 6 - 2;
        if (d->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) {
                /*
                 * 6 is HBP header + checksum
                 * 4 is RGB header + checksum
                 */
-               hbp = (mode->htotal - mode->hsync_end) * bpp - 4 - 6;
+               hbp = (mode->htotal - mode->hsync_end) * cpp - 4 - 6;
                /*
                 * 6 is HBP header + checksum
                 * 4 is HSW packet bytes
                 * 4 is RGB header + checksum
                 */
-               hsa = (mode->hsync_end - mode->hsync_start) * bpp - 4 - 4 - 6;
+               hsa = (mode->hsync_end - mode->hsync_start) * cpp - 4 - 4 - 6;
        } else {
                /*
                 * HBP includes both back porch and sync
@@ -459,11 +464,23 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
                 * 4 is HSW packet bytes
                 * 4 is RGB header + checksum
                 */
-               hbp = (mode->htotal - mode->hsync_start) * bpp - 4 - 4 - 6;
-               /* HSA is not considered in this mode and set to 0 */
+               hbp = (mode->htotal - mode->hsync_start) * cpp - 4 - 4 - 6;
+               /* HSA is not present in this mode and set to 0 */
+               hsa = 0;
+       }
+       if (hfp < 0) {
+               dev_info(d->dev, "hfp negative, set to 0\n");
+               hfp = 0;
+       }
+       if (hbp < 0) {
+               dev_info(d->dev, "hbp negative, set to 0\n");
+               hbp = 0;
+       }
+       if (hsa < 0) {
+               dev_info(d->dev, "hsa negative, set to 0\n");
                hsa = 0;
        }
-       dev_dbg(d->dev, "hfp: %u, hbp: %u, hsa: %u\n",
+       dev_dbg(d->dev, "hfp: %u, hbp: %u, hsa: %u bytes\n",
                hfp, hbp, hsa);
 
        /* Frame parameters: horizontal sync active */
@@ -474,8 +491,8 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
        val |= hfp << DSI_VID_HSIZE1_HFP_LENGTH_SHIFT;
        writel(val, d->regs + DSI_VID_HSIZE1);
 
-       /* RGB data length (bytes on one scanline) */
-       val = mode->hdisplay * (bpp / 8);
+       /* RGB data length (visible bytes on one scanline) */
+       val = mode->hdisplay * cpp;
        writel(val, d->regs + DSI_VID_HSIZE2);
 
        /* TODO: further adjustments for TVG mode here */
@@ -507,37 +524,43 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
        }
 
        line_duration = (blkline_pck + 6) / d->mdsi->lanes;
-       dev_dbg(d->dev, "line duration %u\n", line_duration);
+       dev_dbg(d->dev, "line duration %u bytes\n", line_duration);
        val = line_duration << DSI_VID_DPHY_TIME_REG_LINE_DURATION_SHIFT;
        /*
         * This is the time to perform LP->HS on D-PHY
         * FIXME: nowhere to get this from: DT property on the DSI?
+        * values like 48 and 72 seen in the vendor code.
         */
-       val |= 0 << DSI_VID_DPHY_TIME_REG_WAKEUP_TIME_SHIFT;
+       val |= 48 << DSI_VID_DPHY_TIME_REG_WAKEUP_TIME_SHIFT;
        writel(val, d->regs + DSI_VID_DPHY_TIME);
 
        /* Calculate block end of line */
-       blkeol_pck = bpl - mode->hdisplay * bpp - 6;
+       blkeol_pck = bpl - mode->hdisplay * cpp - 6;
        blkeol_duration = (blkeol_pck + 6) / d->mdsi->lanes;
-       dev_dbg(d->dev, "blkeol pck: %u, duration: %u\n",
-                blkeol_pck, blkeol_duration);
+       dev_dbg(d->dev, "blkeol pck: %u bytes, duration: %u bytes\n",
+               blkeol_pck, blkeol_duration);
 
        if (d->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
                /* Set up EOL clock for burst mode */
                val = readl(d->regs + DSI_VID_BLKSIZE1);
                val |= blkeol_pck << DSI_VID_BLKSIZE1_BLKEOL_PCK_SHIFT;
                writel(val, d->regs + DSI_VID_BLKSIZE1);
-               writel(blkeol_pck, d->regs + DSI_VID_VCA_SETTING2);
+               writel((blkeol_pck & 
DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_MASK)
+                      << DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_SHIFT,
+                      d->regs + DSI_VID_VCA_SETTING2);
 
                writel(blkeol_duration, d->regs + DSI_VID_PCK_TIME);
+               /* Max burst limit */
                writel(blkeol_duration - 6, d->regs + DSI_VID_VCA_SETTING1);
        }
 
        /* Maximum line limit */
        val = readl(d->regs + DSI_VID_VCA_SETTING2);
+       val &= ~DSI_VID_VCA_SETTING2_MAX_LINE_LIMIT_MASK;
        val |= blkline_pck <<
-               DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_SHIFT;
+               DSI_VID_VCA_SETTING2_MAX_LINE_LIMIT_SHIFT;
        writel(val, d->regs + DSI_VID_VCA_SETTING2);
+       dev_dbg(d->dev, "blkline pck: %u bytes\n", blkline_pck);
 
        /* Put IF1 into video mode */
        val = readl(d->regs + DSI_MCTL_MAIN_DATA_CTL);
@@ -699,7 +722,9 @@ static void mcde_dsi_bridge_mode_set(struct drm_bridge 
*bridge,
                lp_freq = d->mdsi->lp_rate;
        else
                lp_freq = DSI_DEFAULT_LP_FREQ_HZ;
-       if (d->mdsi->hs_rate)
+       if (pixel_clock_hz)
+               hs_freq = pixel_clock_hz;
+       else if (d->mdsi->hs_rate)
                hs_freq = d->mdsi->hs_rate;
        else
                hs_freq = DSI_DEFAULT_HS_FREQ_HZ;
-- 
2.21.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to