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.ujfal...@gmail.com>

> 
> Signed-off-by: Tomi Valkeinen <tomi.valkei...@ideasonboard.com>
> ---
>  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

Reply via email to