On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The TC358768 documentation uses HFP, HBP, etc. values to deal with the
> video mode, while the driver currently uses the DRM display mode
> (htotal, hsync_start, etc).
>
> Change the driver to convert the DRM display mode to struct videomode,
> which then allows us to use the same units the documentation uses. This
> makes it much easier to work on the code when using the TC358768
> documentation as a reference.
Reviewed-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxx>
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 45 +++++++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index d5831a1236e9..9b633038af33 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -650,6 +650,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> u32 dsiclk, dsibclk, video_start;
> const u32 internal_delay = 40;
> int ret, i;
> + struct videomode vm;
>
> if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> dev_warn_once(priv->dev, "Non-continuous mode unimplemented, falling back to continuous\n");
> @@ -673,6 +674,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> return;
> }
>
> + drm_display_mode_to_videomode(mode, &vm);
> +
> dsiclk = priv->dsiclk;
> dsibclk = dsiclk / 4;
>
> @@ -681,28 +684,28 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> switch (dsi_dev->format) {
> case MIPI_DSI_FMT_RGB888:
> val |= (0x3 << 4);
> - hact = mode->hdisplay * 3;
> - video_start = (mode->htotal - mode->hsync_start) * 3;
> + hact = vm.hactive * 3;
> + video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> break;
> case MIPI_DSI_FMT_RGB666:
> val |= (0x4 << 4);
> - hact = mode->hdisplay * 3;
> - video_start = (mode->htotal - mode->hsync_start) * 3;
> + hact = vm.hactive * 3;
> + video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> break;
>
> case MIPI_DSI_FMT_RGB666_PACKED:
> val |= (0x4 << 4) | BIT(3);
> - hact = mode->hdisplay * 18 / 8;
> - video_start = (mode->htotal - mode->hsync_start) * 18 / 8;
> + hact = vm.hactive * 18 / 8;
> + video_start = (vm.hsync_len + vm.hback_porch) * 18 / 8;
> data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> break;
>
> case MIPI_DSI_FMT_RGB565:
> val |= (0x5 << 4);
> - hact = mode->hdisplay * 2;
> - video_start = (mode->htotal - mode->hsync_start) * 2;
> + hact = vm.hactive * 2;
> + video_start = (vm.hsync_len + vm.hback_porch) * 2;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> break;
> default:
> @@ -814,43 +817,43 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> tc358768_write(priv, TC358768_DSI_EVENT, 0);
>
> /* vact */
> - tc358768_write(priv, TC358768_DSI_VACT, mode->vdisplay);
> + tc358768_write(priv, TC358768_DSI_VACT, vm.vactive);
>
> /* vsw */
> - tc358768_write(priv, TC358768_DSI_VSW,
> - mode->vsync_end - mode->vsync_start);
> + tc358768_write(priv, TC358768_DSI_VSW, vm.vsync_len);
> +
> /* vbp */
> - tc358768_write(priv, TC358768_DSI_VBPR,
> - mode->vtotal - mode->vsync_end);
> + tc358768_write(priv, TC358768_DSI_VBPR, vm.vback_porch);
>
> /* hsw * byteclk * ndl / pclk */
> - val = (u32)div_u64((mode->hsync_end - mode->hsync_start) *
> + val = (u32)div_u64(vm.hsync_len *
> ((u64)priv->dsiclk / 4) * priv->dsi_lanes,
> - mode->clock * 1000);
> + vm.pixelclock);
> tc358768_write(priv, TC358768_DSI_HSW, val);
>
> /* hbp * byteclk * ndl / pclk */
> - val = (u32)div_u64((mode->htotal - mode->hsync_end) *
> + val = (u32)div_u64(vm.hback_porch *
> ((u64)priv->dsiclk / 4) * priv->dsi_lanes,
> - mode->clock * 1000);
> + vm.pixelclock);
> tc358768_write(priv, TC358768_DSI_HBPR, val);
> } else {
> /* Set event mode */
> tc358768_write(priv, TC358768_DSI_EVENT, 1);
>
> /* vact */
> - tc358768_write(priv, TC358768_DSI_VACT, mode->vdisplay);
> + tc358768_write(priv, TC358768_DSI_VACT, vm.vactive);
>
> /* vsw (+ vbp) */
> tc358768_write(priv, TC358768_DSI_VSW,
> - mode->vtotal - mode->vsync_start);
> + vm.vsync_len + vm.vback_porch);
> +
> /* vbp (not used in event mode) */
> tc358768_write(priv, TC358768_DSI_VBPR, 0);
>
> /* (hsw + hbp) * byteclk * ndl / pclk */
> - val = (u32)div_u64((mode->htotal - mode->hsync_start) *
> + val = (u32)div_u64((vm.hsync_len + vm.hback_porch) *
> ((u64)priv->dsiclk / 4) * priv->dsi_lanes,
> - mode->clock * 1000);
> + vm.pixelclock);
> tc358768_write(priv, TC358768_DSI_HSW, val);
>
> /* hbp (not used in event mode) */
>
--
Péter
