* [PATCH v2] drm/mcde: Some fixes to handling video mode
@ 2019-09-03 9:15 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-09-03 9:15 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
Cc: Linus Walleij, Stephan Gerhold, linux-arm-kernel
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 <stephan@gerhold.net>
Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] drm/mcde: Some fixes to handling video mode
@ 2019-09-03 9:15 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-09-03 9:15 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
Cc: Linus Walleij, Stephan Gerhold, linux-arm-kernel
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 <stephan@gerhold.net>
Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/mcde: Some fixes to handling video mode
2019-09-03 9:15 ` Linus Walleij
@ 2019-09-03 14:37 ` Stephan Gerhold
-1 siblings, 0 replies; 6+ messages in thread
From: Stephan Gerhold @ 2019-09-03 14:37 UTC (permalink / raw)
To: Linus Walleij
Cc: Maxime Ripard, Sean Paul, Maarten Lankhorst, linux-arm-kernel, dri-devel
On Tue, Sep 03, 2019 at 11:15:12AM +0200, Linus Walleij wrote:
> 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 <stephan@gerhold.net>
> Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> 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);
I just want to note that the Samsung S3 Mini (golden) seems to use 88
here. I suppose we will need to see how important this property is,
or if panels can also work with a slightly wrong value.
>
> /* 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);
It does not make a difference in this case because SHIFT = 0,
but shouldn't you normally shift before applying the mask?
Otherwise, you would wipe out the lower bits before you shift them.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/mcde: Some fixes to handling video mode
@ 2019-09-03 14:37 ` Stephan Gerhold
0 siblings, 0 replies; 6+ messages in thread
From: Stephan Gerhold @ 2019-09-03 14:37 UTC (permalink / raw)
To: Linus Walleij
Cc: Maxime Ripard, Sean Paul, Maarten Lankhorst, linux-arm-kernel, dri-devel
On Tue, Sep 03, 2019 at 11:15:12AM +0200, Linus Walleij wrote:
> 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 <stephan@gerhold.net>
> Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> 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);
I just want to note that the Samsung S3 Mini (golden) seems to use 88
here. I suppose we will need to see how important this property is,
or if panels can also work with a slightly wrong value.
>
> /* 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);
It does not make a difference in this case because SHIFT = 0,
but shouldn't you normally shift before applying the mask?
Otherwise, you would wipe out the lower bits before you shift them.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/mcde: Some fixes to handling video mode
2019-09-03 14:37 ` Stephan Gerhold
@ 2019-09-05 12:16 ` Linus Walleij
-1 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-09-05 12:16 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Maxime Ripard, Maarten Lankhorst, Sean Paul,
open list:DRM PANEL DRIVERS, Linux ARM
On Tue, Sep 3, 2019 at 4:38 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> On Tue, Sep 03, 2019 at 11:15:12AM +0200, Linus Walleij wrote:
> > /*
> > * 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);
>
> I just want to note that the Samsung S3 Mini (golden) seems to use 88
> here. I suppose we will need to see how important this property is,
> or if panels can also work with a slightly wrong value.
Yeah maybe we should just figure out what to do about this.
This is the wakeup time, in clock cycles, for a LP->HS
transition on the D-PHY.
The panel driver kind of knows knows this timing, so I
guess we should add a field to struct mipi_dsi_device
such as unsigned int lp_to_hs_wakep, but it needs to
come from the device tree since per the manual this
value is system dependent:
"reg_wakeup_time must be shorter than line duration and
depends on the D-PHY cell plus some pipelines delays inserted
by the DSI link. This value strongly depends on the PLL
programming and as it is a mix of analog and digital timing, it
is practically impossible to provide a general formula. By the
way, it has to be characterized at system level (validation
and/or simulation)."
> > - 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);
>
> It does not make a difference in this case because SHIFT = 0,
> but shouldn't you normally shift before applying the mask?
> Otherwise, you would wipe out the lower bits before you shift them.
OK you're right, I fix it up and resend.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/mcde: Some fixes to handling video mode
@ 2019-09-05 12:16 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-09-05 12:16 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Maxime Ripard, Sean Paul, open list:DRM PANEL DRIVERS, Linux ARM
On Tue, Sep 3, 2019 at 4:38 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> On Tue, Sep 03, 2019 at 11:15:12AM +0200, Linus Walleij wrote:
> > /*
> > * 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);
>
> I just want to note that the Samsung S3 Mini (golden) seems to use 88
> here. I suppose we will need to see how important this property is,
> or if panels can also work with a slightly wrong value.
Yeah maybe we should just figure out what to do about this.
This is the wakeup time, in clock cycles, for a LP->HS
transition on the D-PHY.
The panel driver kind of knows knows this timing, so I
guess we should add a field to struct mipi_dsi_device
such as unsigned int lp_to_hs_wakep, but it needs to
come from the device tree since per the manual this
value is system dependent:
"reg_wakeup_time must be shorter than line duration and
depends on the D-PHY cell plus some pipelines delays inserted
by the DSI link. This value strongly depends on the PLL
programming and as it is a mix of analog and digital timing, it
is practically impossible to provide a general formula. By the
way, it has to be characterized at system level (validation
and/or simulation)."
> > - 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);
>
> It does not make a difference in this case because SHIFT = 0,
> but shouldn't you normally shift before applying the mask?
> Otherwise, you would wipe out the lower bits before you shift them.
OK you're right, I fix it up and resend.
Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-05 12:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 9:15 [PATCH v2] drm/mcde: Some fixes to handling video mode Linus Walleij
2019-09-03 9:15 ` Linus Walleij
2019-09-03 14:37 ` Stephan Gerhold
2019-09-03 14:37 ` Stephan Gerhold
2019-09-05 12:16 ` Linus Walleij
2019-09-05 12:16 ` Linus Walleij
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.