Re: [PATCH v2 31/33] drm/vc4: vec: Convert to the new TV mode property

2022-09-24 Thread Noralf Trønnes



Den 22.09.2022 16.25, skrev Maxime Ripard:
> Now that the core can deal fine with analog TV modes, let's convert the vc4
> VEC driver to leverage those new features.
> 
> We've added some backward compatibility to support the old TV mode property
> and translate it into the new TV norm property.
> 

You can mention here that atomic_check is added in order to trigger a
modeset should tv.mode change.

> Signed-off-by: Maxime Ripard 
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index 6828b79a1001..a9463364015b 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -172,6 +172,8 @@ struct vc4_vec {
>  
>   struct clk *clock;
>  
> + struct drm_property *legacy_tv_mode_property;
> +
>   struct debugfs_regset32 regset;
>  };
>  
> @@ -184,6 +186,12 @@ encoder_to_vc4_vec(struct drm_encoder *encoder)
>   return container_of(encoder, struct vc4_vec, encoder.base);
>  }
>  
> +static inline struct vc4_vec *
> +connector_to_vc4_vec(struct drm_connector *connector)
> +{
> + return container_of(connector, struct vc4_vec, connector);
> +}
> +
>  enum vc4_vec_tv_mode_id {
>   VC4_VEC_TV_MODE_NTSC,
>   VC4_VEC_TV_MODE_NTSC_J,
> @@ -192,7 +200,7 @@ enum vc4_vec_tv_mode_id {
>  };
>  
>  struct vc4_vec_tv_mode {
> - const struct drm_display_mode *mode;
> + unsigned int mode;
>   u32 config0;
>   u32 config1;
>   u32 custom_freq;
> @@ -225,43 +233,51 @@ static const struct debugfs_reg32 vec_regs[] = {
>   VC4_REG32(VEC_DAC_MISC),
>  };
>  
> -static const struct drm_display_mode ntsc_mode = {
> - DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 13500,
> -  720, 720 + 14, 720 + 14 + 64, 720 + 14 + 64 + 60, 0,
> -  480, 480 + 7, 480 + 7 + 6, 525, 0,
> -  DRM_MODE_FLAG_INTERLACE)
> -};
> -
> -static const struct drm_display_mode pal_mode = {
> - DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 13500,
> -  720, 720 + 20, 720 + 20 + 64, 720 + 20 + 64 + 60, 0,
> -  576, 576 + 4, 576 + 4 + 6, 625, 0,
> -  DRM_MODE_FLAG_INTERLACE)
> -};
> -
>  static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = {
> - [VC4_VEC_TV_MODE_NTSC] = {
> - .mode = _mode,
> + {
> + .mode = DRM_MODE_TV_MODE_NTSC,
>   .config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN,
>   .config1 = VEC_CONFIG1_C_CVBS_CVBS,
>   },
> - [VC4_VEC_TV_MODE_NTSC_J] = {
> - .mode = _mode,
> + {
> + .mode = DRM_MODE_TV_MODE_NTSC_J,
>   .config0 = VEC_CONFIG0_NTSC_STD,
>   .config1 = VEC_CONFIG1_C_CVBS_CVBS,
>   },
> - [VC4_VEC_TV_MODE_PAL] = {
> - .mode = _mode,
> + {
> + .mode = DRM_MODE_TV_MODE_PAL,
>   .config0 = VEC_CONFIG0_PAL_BDGHI_STD,
>   .config1 = VEC_CONFIG1_C_CVBS_CVBS,
>   },
> - [VC4_VEC_TV_MODE_PAL_M] = {
> - .mode = _mode,
> + {
> + .mode = DRM_MODE_TV_MODE_PAL_M,
>   .config0 = VEC_CONFIG0_PAL_M_STD,
>   .config1 = VEC_CONFIG1_C_CVBS_CVBS,
>   },
>  };
>  
> +static inline const struct vc4_vec_tv_mode *
> +vc4_vec_tv_mode_lookup(unsigned int mode)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(vc4_vec_tv_modes); i++) {
> + const struct vc4_vec_tv_mode *tv_mode = _vec_tv_modes[i];
> +
> + if (tv_mode->mode == mode)
> + return tv_mode;
> + }
> +
> + return NULL;
> +}
> +
> +static const struct drm_prop_enum_list tv_mode_names[] = {
> + { VC4_VEC_TV_MODE_NTSC, "NTSC", },
> + { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", },
> + { VC4_VEC_TV_MODE_PAL, "PAL", },
> + { VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
> +};
> +
>  static enum drm_connector_status
>  vc4_vec_connector_detect(struct drm_connector *connector, bool force)
>  {
> @@ -276,19 +292,99 @@ static void vc4_vec_connector_reset(struct 
> drm_connector *connector)
>  
>  static int vc4_vec_connector_get_modes(struct drm_connector *connector)
>  {
> - struct drm_connector_state *state = connector->state;
>   struct drm_display_mode *mode;
> + int count = 0;
>  
> - mode = drm_mode_duplicate(connector->dev,
> -   vc4_vec_tv_modes[state->tv.legacy_mode].mode);
> + mode = drm_mode_analog_ntsc_480i(connector->dev);
>   if (!mode) {
>   DRM_ERROR("Failed to create a new display mode\n");
>   return -ENOMEM;
>   }
>  
> + mode->type |= DRM_MODE_TYPE_PREFERRED;
>   drm_mode_probed_add(connector, mode);
> + count += 1;
>  
> - return 1;
> + mode = drm_mode_analog_pal_576i(connector->dev);
> + if (!mode) {
> + DRM_ERROR("Failed to create a new display mode\n");
> + return -ENOMEM;
> + }
> +
> + drm_mode_probed_add(connector, mode);
> + count += 1;
> +
> + return count;

Why not just 

[PATCH v2 31/33] drm/vc4: vec: Convert to the new TV mode property

2022-09-22 Thread Maxime Ripard
Now that the core can deal fine with analog TV modes, let's convert the vc4
VEC driver to leverage those new features.

We've added some backward compatibility to support the old TV mode property
and translate it into the new TV norm property.

Signed-off-by: Maxime Ripard 

diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index 6828b79a1001..a9463364015b 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -172,6 +172,8 @@ struct vc4_vec {
 
struct clk *clock;
 
+   struct drm_property *legacy_tv_mode_property;
+
struct debugfs_regset32 regset;
 };
 
@@ -184,6 +186,12 @@ encoder_to_vc4_vec(struct drm_encoder *encoder)
return container_of(encoder, struct vc4_vec, encoder.base);
 }
 
+static inline struct vc4_vec *
+connector_to_vc4_vec(struct drm_connector *connector)
+{
+   return container_of(connector, struct vc4_vec, connector);
+}
+
 enum vc4_vec_tv_mode_id {
VC4_VEC_TV_MODE_NTSC,
VC4_VEC_TV_MODE_NTSC_J,
@@ -192,7 +200,7 @@ enum vc4_vec_tv_mode_id {
 };
 
 struct vc4_vec_tv_mode {
-   const struct drm_display_mode *mode;
+   unsigned int mode;
u32 config0;
u32 config1;
u32 custom_freq;
@@ -225,43 +233,51 @@ static const struct debugfs_reg32 vec_regs[] = {
VC4_REG32(VEC_DAC_MISC),
 };
 
-static const struct drm_display_mode ntsc_mode = {
-   DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 13500,
-720, 720 + 14, 720 + 14 + 64, 720 + 14 + 64 + 60, 0,
-480, 480 + 7, 480 + 7 + 6, 525, 0,
-DRM_MODE_FLAG_INTERLACE)
-};
-
-static const struct drm_display_mode pal_mode = {
-   DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 13500,
-720, 720 + 20, 720 + 20 + 64, 720 + 20 + 64 + 60, 0,
-576, 576 + 4, 576 + 4 + 6, 625, 0,
-DRM_MODE_FLAG_INTERLACE)
-};
-
 static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = {
-   [VC4_VEC_TV_MODE_NTSC] = {
-   .mode = _mode,
+   {
+   .mode = DRM_MODE_TV_MODE_NTSC,
.config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
-   [VC4_VEC_TV_MODE_NTSC_J] = {
-   .mode = _mode,
+   {
+   .mode = DRM_MODE_TV_MODE_NTSC_J,
.config0 = VEC_CONFIG0_NTSC_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
-   [VC4_VEC_TV_MODE_PAL] = {
-   .mode = _mode,
+   {
+   .mode = DRM_MODE_TV_MODE_PAL,
.config0 = VEC_CONFIG0_PAL_BDGHI_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
-   [VC4_VEC_TV_MODE_PAL_M] = {
-   .mode = _mode,
+   {
+   .mode = DRM_MODE_TV_MODE_PAL_M,
.config0 = VEC_CONFIG0_PAL_M_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
 };
 
+static inline const struct vc4_vec_tv_mode *
+vc4_vec_tv_mode_lookup(unsigned int mode)
+{
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(vc4_vec_tv_modes); i++) {
+   const struct vc4_vec_tv_mode *tv_mode = _vec_tv_modes[i];
+
+   if (tv_mode->mode == mode)
+   return tv_mode;
+   }
+
+   return NULL;
+}
+
+static const struct drm_prop_enum_list tv_mode_names[] = {
+   { VC4_VEC_TV_MODE_NTSC, "NTSC", },
+   { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", },
+   { VC4_VEC_TV_MODE_PAL, "PAL", },
+   { VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
+};
+
 static enum drm_connector_status
 vc4_vec_connector_detect(struct drm_connector *connector, bool force)
 {
@@ -276,19 +292,99 @@ static void vc4_vec_connector_reset(struct drm_connector 
*connector)
 
 static int vc4_vec_connector_get_modes(struct drm_connector *connector)
 {
-   struct drm_connector_state *state = connector->state;
struct drm_display_mode *mode;
+   int count = 0;
 
-   mode = drm_mode_duplicate(connector->dev,
- vc4_vec_tv_modes[state->tv.legacy_mode].mode);
+   mode = drm_mode_analog_ntsc_480i(connector->dev);
if (!mode) {
DRM_ERROR("Failed to create a new display mode\n");
return -ENOMEM;
}
 
+   mode->type |= DRM_MODE_TYPE_PREFERRED;
drm_mode_probed_add(connector, mode);
+   count += 1;
 
-   return 1;
+   mode = drm_mode_analog_pal_576i(connector->dev);
+   if (!mode) {
+   DRM_ERROR("Failed to create a new display mode\n");
+   return -ENOMEM;
+   }
+
+   drm_mode_probed_add(connector, mode);
+   count += 1;
+
+   return count;
+}
+
+static int
+vc4_vec_connector_set_property(struct drm_connector *connector,
+  struct drm_connector_state *state,
+  struct drm_property *property,
+  uint64_t val)
+{
+   struct vc4_vec *vec =