diff mbox series

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

Message ID 20180926204150.29829-2-stefan@agner.ch (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/fsl-dcu: drop unused drm_crtc_index() | expand

Commit Message

Stefan Agner Sept. 26, 2018, 8:41 p.m. UTC 
From: Satendra Singh Thakur <satendra.t@samsung.com>

Use the drm_display_mode_to_videomode function to calculate front/
back porches and sync length.

Cc: Madhur Verma <madhur.verma@samsung.com>
Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 32 ++++++++++------------
 1 file changed, 14 insertions(+), 18 deletions(-)

Comments

Daniel Vetter Oct. 1, 2018, 7:57 a.m. UTC | #1 
On Wed, Sep 26, 2018 at 10:41:50PM +0200, Stefan Agner wrote:
> From: Satendra Singh Thakur <satendra.t@samsung.com>
> 
> Use the drm_display_mode_to_videomode function to calculate front/
> back porches and sync length.
> 
> Cc: Madhur Verma <madhur.verma@samsung.com>
> Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

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@ffwll.ch>
> ---
>  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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Stefan Agner Oct. 1, 2018, 8:03 p.m. UTC | #2 
On 01.10.2018 09:57, Daniel Vetter wrote:
> On Wed, Sep 26, 2018 at 10:41:50PM +0200, Stefan Agner wrote:
>> From: Satendra Singh Thakur <satendra.t@samsung.com>
>>
>> Use the drm_display_mode_to_videomode function to calculate front/
>> back porches and sync length.
>>
>> Cc: Madhur Verma <madhur.verma@samsung.com>
>> Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
>> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> 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?

Hm, I see, yeah seems rather straight forward to calculate this
information in drm_mode_set_crtcinfo().

> 
> On both patches:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks, applied both.

--
Stefan

>> ---
>>  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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

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));