Re: [PATCH 2/2] drm/fsl-dcu: use drm_display_mode_to_videomode to calculate timing parameters

[Daniel Vetter]

On Wed, Sep 26, 2018 at 10:41:50PM +0200, Stefan Agner wrote:
> From: Satendra Singh Thakur <satendra.t@xxxxxxxxxxx>
> 
> Use the drm_display_mode_to_videomode function to calculate front/
> back porches and sync length.
> 
> Cc: Madhur Verma <madhur.verma@xxxxxxxxxxx>
> Cc: Hemanshu Srivastava <hemanshu.s@xxxxxxxxxxx>
> Signed-off-by: Satendra Singh Thakur <satendra.t@xxxxxxxxxxx>
> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>

I think if lots of people want this (iirc yours not the only driver that
wants front/back-porch) we should extend the computed crtc_* fields with
this. Kinda silly we force people to go through v4l videomode in drm
drivers. But then, this gets the job done, so maybe an idea for todo.rst?

On both patches:

Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 32 ++++++++++------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> index 05d1c9977395..18afc94e4dff 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> @@ -17,6 +17,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <video/videomode.h>
>  
>  #include "fsl_dcu_drm_crtc.h"
>  #include "fsl_dcu_drm_drv.h"
> @@ -85,39 +86,34 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>  	struct drm_connector *con = &fsl_dev->connector.base;
>  	struct drm_display_mode *mode = &crtc->state->mode;
> -	unsigned int hbp, hfp, hsw, vbp, vfp, vsw, pol = 0;
> +	unsigned int pol = 0;
> +	struct videomode vm;
>  
>  	clk_set_rate(fsl_dev->pix_clk, mode->clock * 1000);
>  
> -	/* Configure timings: */
> -	hbp = mode->htotal - mode->hsync_end;
> -	hfp = mode->hsync_start - mode->hdisplay;
> -	hsw = mode->hsync_end - mode->hsync_start;
> -	vbp = mode->vtotal - mode->vsync_end;
> -	vfp = mode->vsync_start - mode->vdisplay;
> -	vsw = mode->vsync_end - mode->vsync_start;
> +	drm_display_mode_to_videomode(mode, &vm);
>  
>  	/* INV_PXCK as default (most display sample data on rising edge) */
>  	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE))
>  		pol |= DCU_SYN_POL_INV_PXCK;
>  
> -	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> +	if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
>  		pol |= DCU_SYN_POL_INV_HS_LOW;
>  
> -	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> +	if (vm.flags & DISPLAY_FLAGS_VSYNC_LOW)
>  		pol |= DCU_SYN_POL_INV_VS_LOW;
>  
>  	regmap_write(fsl_dev->regmap, DCU_HSYN_PARA,
> -		     DCU_HSYN_PARA_BP(hbp) |
> -		     DCU_HSYN_PARA_PW(hsw) |
> -		     DCU_HSYN_PARA_FP(hfp));
> +		     DCU_HSYN_PARA_BP(vm.hback_porch) |
> +		     DCU_HSYN_PARA_PW(vm.hsync_len) |
> +		     DCU_HSYN_PARA_FP(vm.hfront_porch));
>  	regmap_write(fsl_dev->regmap, DCU_VSYN_PARA,
> -		     DCU_VSYN_PARA_BP(vbp) |
> -		     DCU_VSYN_PARA_PW(vsw) |
> -		     DCU_VSYN_PARA_FP(vfp));
> +		     DCU_VSYN_PARA_BP(vm.vback_porch) |
> +		     DCU_VSYN_PARA_PW(vm.vsync_len) |
> +		     DCU_VSYN_PARA_FP(vm.vfront_porch));
>  	regmap_write(fsl_dev->regmap, DCU_DISP_SIZE,
> -		     DCU_DISP_SIZE_DELTA_Y(mode->vdisplay) |
> -		     DCU_DISP_SIZE_DELTA_X(mode->hdisplay));
> +		     DCU_DISP_SIZE_DELTA_Y(vm.vactive) |
> +		     DCU_DISP_SIZE_DELTA_X(vm.hactive));
>  	regmap_write(fsl_dev->regmap, DCU_SYN_POL, pol);
>  	regmap_write(fsl_dev->regmap, DCU_BGND, DCU_BGND_R(0) |
>  		     DCU_BGND_G(0) | DCU_BGND_B(0));
> -- 
> 2.19.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch