Re: [PATCH v1.1 26/29] drm/omap: venc: Use drm_display_mode natively

2018-12-10 Thread Sebastian Reichel 
Hi,

On Thu, Dec 06, 2018 at 06:23:46PM +0200, Laurent Pinchart wrote:
> Replace internal usage of struct videomode with struct drm_display_mode
> in order to avoid converting needlessly between the data structures.
> 
> Signed-off-by: Laurent Pinchart 
> ---
> Changes since v1:
> 
> - Set mode.crtc_* fields and mode name in venc_check_timings()
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/venc.c | 90 +++---
>  1 file changed, 44 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c 
> b/drivers/gpu/drm/omapdrm/dss/venc.c
> index 7bce5898654a..f2cbecfd05b5 100644
> --- a/drivers/gpu/drm/omapdrm/dss/venc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/venc.c
> @@ -267,38 +267,34 @@ enum venc_videomode {
>   VENC_MODE_NTSC,
>  };
>  
> -static const struct videomode omap_dss_pal_vm = {
> - .hactive= 720,
> - .vactive= 574,
> - .pixelclock = 1350,
> - .hsync_len  = 64,
> - .hfront_porch   = 12,
> - .hback_porch= 68,
> - .vsync_len  = 5,
> - .vfront_porch   = 5,
> - .vback_porch= 41,
> -
> - .flags  = DISPLAY_FLAGS_INTERLACED | DISPLAY_FLAGS_HSYNC_LOW |
> -   DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_DE_HIGH |
> -   DISPLAY_FLAGS_PIXDATA_POSEDGE |
> -   DISPLAY_FLAGS_SYNC_NEGEDGE,
> +static const struct drm_display_mode omap_dss_pal_mode = {
> + .hdisplay   = 720,
> + .hsync_start= 732,
> + .hsync_end  = 796,
> + .htotal = 864,
> + .vdisplay   = 574,
> + .vsync_start= 579,
> + .vsync_end  = 584,
> + .vtotal = 625,
> + .clock  = 13500,
> +
> + .flags  = DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_NHSYNC |
> +   DRM_MODE_FLAG_NVSYNC,
>  };
>  
> -static const struct videomode omap_dss_ntsc_vm = {
> - .hactive= 720,
> - .vactive= 482,
> - .pixelclock = 1350,
> - .hsync_len  = 64,
> - .hfront_porch   = 16,
> - .hback_porch= 58,
> - .vsync_len  = 6,
> - .vfront_porch   = 6,
> - .vback_porch= 31,
> -
> - .flags  = DISPLAY_FLAGS_INTERLACED | DISPLAY_FLAGS_HSYNC_LOW |
> -   DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_DE_HIGH |
> -   DISPLAY_FLAGS_PIXDATA_POSEDGE |
> -   DISPLAY_FLAGS_SYNC_NEGEDGE,
> +static const struct drm_display_mode omap_dss_ntsc_mode = {
> + .hdisplay   = 720,
> + .hsync_start= 736,
> + .hsync_end  = 800,
> + .htotal = 858,
> + .vdisplay   = 482,
> + .vsync_start= 488,
> + .vsync_end  = 494,
> + .vtotal = 525,
> + .clock  = 13500,
> +
> + .flags  = DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_NHSYNC |
> +   DRM_MODE_FLAG_NVSYNC,
>  };
>  
>  struct venc_device {
> @@ -521,21 +517,19 @@ static void venc_display_disable(struct omap_dss_device 
> *dssdev)
>  static int venc_get_modes(struct omap_dss_device *dssdev,
> struct drm_connector *connector)
>  {
> - static const struct videomode *modes[] = {
> - _dss_pal_vm,
> - _dss_ntsc_vm,
> + static const struct drm_display_mode *modes[] = {
> + _dss_pal_mode,
> + _dss_ntsc_mode,
>   };
>   unsigned int i;
>  
>   for (i = 0; i < ARRAY_SIZE(modes); ++i) {
>   struct drm_display_mode *mode;
>  
> - mode = drm_mode_create(connector->dev);
> + mode = drm_mode_duplicate(connector->dev, modes[i]);
>   if (!mode)
>   return i;
>  
> - drm_display_mode_from_videomode(modes[i], mode);
> -
>   mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
>   drm_mode_set_name(mode);
>   drm_mode_probed_add(connector, mode);
> @@ -549,14 +543,14 @@ static enum venc_videomode venc_get_videomode(const 
> struct drm_display_mode *mod
>   if (!(mode->flags & DRM_MODE_FLAG_INTERLACE))
>   return VENC_MODE_UNKNOWN;
>  
> - if (mode->clock == omap_dss_pal_vm.pixelclock / 1000 &&
> - mode->hdisplay == omap_dss_pal_vm.hactive &&
> - mode->vdisplay == omap_dss_pal_vm.vactive)
> + if (mode->clock == omap_dss_pal_mode.clock &&
> + mode->hdisplay == omap_dss_pal_mode.hdisplay &&
> + mode->vdisplay == omap_dss_pal_mode.vdisplay)
>   return VENC_MODE_PAL;
>  
> - if (mode->clock == omap_dss_ntsc_vm.pixelclock / 1000 &&
> - mode->hdisplay == omap_dss_ntsc_vm.hactive &&
> - mode->vdisplay == omap_dss_ntsc_vm.vactive)
> + if (mode->clock == omap_dss_ntsc_mode.clock &&
> + mode->hdisplay == omap_dss_ntsc_mode.hdisplay &&
> + mode->vdisplay == omap_dss_ntsc_mode.vdisplay)
>

[PATCH v1.1 26/29] drm/omap: venc: Use drm_display_mode natively

2018-12-06 Thread Laurent Pinchart 
Replace internal usage of struct videomode with struct drm_display_mode
in order to avoid converting needlessly between the data structures.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Set mode.crtc_* fields and mode name in venc_check_timings()
---
 drivers/gpu/drm/omapdrm/dss/venc.c | 90 +++---
 1 file changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c 
b/drivers/gpu/drm/omapdrm/dss/venc.c
index 7bce5898654a..f2cbecfd05b5 100644
--- a/drivers/gpu/drm/omapdrm/dss/venc.c
+++ b/drivers/gpu/drm/omapdrm/dss/venc.c
@@ -267,38 +267,34 @@ enum venc_videomode {
VENC_MODE_NTSC,
 };
 
-static const struct videomode omap_dss_pal_vm = {
-   .hactive= 720,
-   .vactive= 574,
-   .pixelclock = 1350,
-   .hsync_len  = 64,
-   .hfront_porch   = 12,
-   .hback_porch= 68,
-   .vsync_len  = 5,
-   .vfront_porch   = 5,
-   .vback_porch= 41,
-
-   .flags  = DISPLAY_FLAGS_INTERLACED | DISPLAY_FLAGS_HSYNC_LOW |
- DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_DE_HIGH |
- DISPLAY_FLAGS_PIXDATA_POSEDGE |
- DISPLAY_FLAGS_SYNC_NEGEDGE,
+static const struct drm_display_mode omap_dss_pal_mode = {
+   .hdisplay   = 720,
+   .hsync_start= 732,
+   .hsync_end  = 796,
+   .htotal = 864,
+   .vdisplay   = 574,
+   .vsync_start= 579,
+   .vsync_end  = 584,
+   .vtotal = 625,
+   .clock  = 13500,
+
+   .flags  = DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_NHSYNC |
+ DRM_MODE_FLAG_NVSYNC,
 };
 
-static const struct videomode omap_dss_ntsc_vm = {
-   .hactive= 720,
-   .vactive= 482,
-   .pixelclock = 1350,
-   .hsync_len  = 64,
-   .hfront_porch   = 16,
-   .hback_porch= 58,
-   .vsync_len  = 6,
-   .vfront_porch   = 6,
-   .vback_porch= 31,
-
-   .flags  = DISPLAY_FLAGS_INTERLACED | DISPLAY_FLAGS_HSYNC_LOW |
- DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_DE_HIGH |
- DISPLAY_FLAGS_PIXDATA_POSEDGE |
- DISPLAY_FLAGS_SYNC_NEGEDGE,
+static const struct drm_display_mode omap_dss_ntsc_mode = {
+   .hdisplay   = 720,
+   .hsync_start= 736,
+   .hsync_end  = 800,
+   .htotal = 858,
+   .vdisplay   = 482,
+   .vsync_start= 488,
+   .vsync_end  = 494,
+   .vtotal = 525,
+   .clock  = 13500,
+
+   .flags  = DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_NHSYNC |
+ DRM_MODE_FLAG_NVSYNC,
 };
 
 struct venc_device {
@@ -521,21 +517,19 @@ static void venc_display_disable(struct omap_dss_device 
*dssdev)
 static int venc_get_modes(struct omap_dss_device *dssdev,
  struct drm_connector *connector)
 {
-   static const struct videomode *modes[] = {
-   _dss_pal_vm,
-   _dss_ntsc_vm,
+   static const struct drm_display_mode *modes[] = {
+   _dss_pal_mode,
+   _dss_ntsc_mode,
};
unsigned int i;
 
for (i = 0; i < ARRAY_SIZE(modes); ++i) {
struct drm_display_mode *mode;
 
-   mode = drm_mode_create(connector->dev);
+   mode = drm_mode_duplicate(connector->dev, modes[i]);
if (!mode)
return i;
 
-   drm_display_mode_from_videomode(modes[i], mode);
-
mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
drm_mode_set_name(mode);
drm_mode_probed_add(connector, mode);
@@ -549,14 +543,14 @@ static enum venc_videomode venc_get_videomode(const 
struct drm_display_mode *mod
if (!(mode->flags & DRM_MODE_FLAG_INTERLACE))
return VENC_MODE_UNKNOWN;
 
-   if (mode->clock == omap_dss_pal_vm.pixelclock / 1000 &&
-   mode->hdisplay == omap_dss_pal_vm.hactive &&
-   mode->vdisplay == omap_dss_pal_vm.vactive)
+   if (mode->clock == omap_dss_pal_mode.clock &&
+   mode->hdisplay == omap_dss_pal_mode.hdisplay &&
+   mode->vdisplay == omap_dss_pal_mode.vdisplay)
return VENC_MODE_PAL;
 
-   if (mode->clock == omap_dss_ntsc_vm.pixelclock / 1000 &&
-   mode->hdisplay == omap_dss_ntsc_vm.hactive &&
-   mode->vdisplay == omap_dss_ntsc_vm.vactive)
+   if (mode->clock == omap_dss_ntsc_mode.clock &&
+   mode->hdisplay == omap_dss_ntsc_mode.hdisplay &&
+   mode->vdisplay == omap_dss_ntsc_mode.vdisplay)
return VENC_MODE_NTSC;
 
return VENC_MODE_UNKNOWN;
@@ -597,16 +591,20 @@ static int venc_check_timings(struct omap_dss_device 
*dssdev,
 
switch (venc_get_videomode(mode)) {
case