Re: [PATCH 2/3] drm/vc4: Add monochrome mode to the VEC.
Hi Maxime
On Tue, 18 Jun 2024 at 10:28, Maxime Ripard wrote:
>
> Hi,
>
> On Fri, Feb 16, 2024 at 06:48:56PM GMT, Dave Stevenson wrote:
> > The VEC supports not producing colour bursts for monochrome output.
> > It also has an option for disabling the chroma input to remove
> > chroma from the signal.
> >
> > Now that there is a DRM_MODE_TV_MODE_MONOCHROME defined, plumb
> > this in.
> >
> > Signed-off-by: Dave Stevenson
> > ---
> > drivers/gpu/drm/vc4/vc4_vec.c | 28 +++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> > index 268f18b10ee0..f9e134dd1e3b 100644
> > --- a/drivers/gpu/drm/vc4/vc4_vec.c
> > +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> > @@ -234,6 +234,7 @@ enum vc4_vec_tv_mode_id {
> > VC4_VEC_TV_MODE_PAL_60,
> > VC4_VEC_TV_MODE_PAL_N,
> > VC4_VEC_TV_MODE_SECAM,
> > + VC4_VEC_TV_MODE_MONOCHROME,
> > };
> >
> > struct vc4_vec_tv_mode {
> > @@ -324,6 +325,22 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_modes[]
> > = {
> > .config1 = VEC_CONFIG1_C_CVBS_CVBS,
> > .custom_freq = 0x29c71c72,
> > },
> > + {
> > + /* 50Hz mono */
> > + .mode = DRM_MODE_TV_MODE_MONOCHROME,
> > + .expected_htotal = 864,
> > + .config0 = VEC_CONFIG0_PAL_BDGHI_STD | VEC_CONFIG0_BURDIS |
> > +VEC_CONFIG0_CHRDIS,
> > + .config1 = VEC_CONFIG1_C_CVBS_CVBS,
> > + },
> > + {
> > + /* 60Hz mono */
> > + .mode = DRM_MODE_TV_MODE_MONOCHROME,
> > + .expected_htotal = 858,
> > + .config0 = VEC_CONFIG0_PAL_M_STD | VEC_CONFIG0_BURDIS |
> > +VEC_CONFIG0_CHRDIS,
> > + .config1 = VEC_CONFIG1_C_CVBS_CVBS,
> > + },
> > };
> >
> > static inline const struct vc4_vec_tv_mode *
> > @@ -351,6 +368,7 @@ static const struct drm_prop_enum_list
> > legacy_tv_mode_names[] = {
> > { VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
> > { VC4_VEC_TV_MODE_PAL_N, "PAL-N", },
> > { VC4_VEC_TV_MODE_SECAM, "SECAM", },
> > + { VC4_VEC_TV_MODE_MONOCHROME, "Mono", },
> > };
> >
> > static enum drm_connector_status
> > @@ -406,6 +424,10 @@ vc4_vec_connector_set_property(struct drm_connector
> > *connector,
> > state->tv.mode = DRM_MODE_TV_MODE_SECAM;
> > break;
> >
> > + case VC4_VEC_TV_MODE_MONOCHROME:
> > + state->tv.mode = DRM_MODE_TV_MODE_MONOCHROME;
> > + break;
> > +
> > default:
> > return -EINVAL;
> > }
> > @@ -453,6 +475,9 @@ vc4_vec_connector_get_property(struct drm_connector
> > *connector,
> > *val = VC4_VEC_TV_MODE_SECAM;
> > break;
> >
> > + case DRM_MODE_TV_MODE_MONOCHROME:
> > + return VC4_VEC_TV_MODE_MONOCHROME;
I have got an error here - it should be
*val = VC4_VEC_TV_MODE_MONOCHROME;
break;
> > +
> > default:
> > return -EINVAL;
> > }
>
> We don't need to expose the new value here, that property is only for
> the legacy, driver-specific property. So you should only need the
> vc4_vec_tv_modes changes
As both properties share the same underlying value, that means that if
the new property selects Monochrome, the legacy one will return
-EINVAL as it is an unknown value.
modetest and kmsprint -p both bomb out in this situation as by the
looks of it we fail to get a pointer to the connector returned. That
means you can't switch it back again.
Am I missing something?
Dave
Re: [PATCH 2/3] drm/vc4: Add monochrome mode to the VEC.
Hi,
On Fri, Feb 16, 2024 at 06:48:56PM GMT, Dave Stevenson wrote:
> The VEC supports not producing colour bursts for monochrome output.
> It also has an option for disabling the chroma input to remove
> chroma from the signal.
>
> Now that there is a DRM_MODE_TV_MODE_MONOCHROME defined, plumb
> this in.
>
> Signed-off-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_vec.c | 28 +++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index 268f18b10ee0..f9e134dd1e3b 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -234,6 +234,7 @@ enum vc4_vec_tv_mode_id {
> VC4_VEC_TV_MODE_PAL_60,
> VC4_VEC_TV_MODE_PAL_N,
> VC4_VEC_TV_MODE_SECAM,
> + VC4_VEC_TV_MODE_MONOCHROME,
> };
>
> struct vc4_vec_tv_mode {
> @@ -324,6 +325,22 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] =
> {
> .config1 = VEC_CONFIG1_C_CVBS_CVBS,
> .custom_freq = 0x29c71c72,
> },
> + {
> + /* 50Hz mono */
> + .mode = DRM_MODE_TV_MODE_MONOCHROME,
> + .expected_htotal = 864,
> + .config0 = VEC_CONFIG0_PAL_BDGHI_STD | VEC_CONFIG0_BURDIS |
> +VEC_CONFIG0_CHRDIS,
> + .config1 = VEC_CONFIG1_C_CVBS_CVBS,
> + },
> + {
> + /* 60Hz mono */
> + .mode = DRM_MODE_TV_MODE_MONOCHROME,
> + .expected_htotal = 858,
> + .config0 = VEC_CONFIG0_PAL_M_STD | VEC_CONFIG0_BURDIS |
> +VEC_CONFIG0_CHRDIS,
> + .config1 = VEC_CONFIG1_C_CVBS_CVBS,
> + },
> };
>
> static inline const struct vc4_vec_tv_mode *
> @@ -351,6 +368,7 @@ static const struct drm_prop_enum_list
> legacy_tv_mode_names[] = {
> { VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
> { VC4_VEC_TV_MODE_PAL_N, "PAL-N", },
> { VC4_VEC_TV_MODE_SECAM, "SECAM", },
> + { VC4_VEC_TV_MODE_MONOCHROME, "Mono", },
> };
>
> static enum drm_connector_status
> @@ -406,6 +424,10 @@ vc4_vec_connector_set_property(struct drm_connector
> *connector,
> state->tv.mode = DRM_MODE_TV_MODE_SECAM;
> break;
>
> + case VC4_VEC_TV_MODE_MONOCHROME:
> + state->tv.mode = DRM_MODE_TV_MODE_MONOCHROME;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -453,6 +475,9 @@ vc4_vec_connector_get_property(struct drm_connector
> *connector,
> *val = VC4_VEC_TV_MODE_SECAM;
> break;
>
> + case DRM_MODE_TV_MODE_MONOCHROME:
> + return VC4_VEC_TV_MODE_MONOCHROME;
> +
> default:
> return -EINVAL;
> }
We don't need to expose the new value here, that property is only for
the legacy, driver-specific property. So you should only need the
vc4_vec_tv_modes changes
Maxime
signature.asc
Description: PGP signature
[PATCH 2/3] drm/vc4: Add monochrome mode to the VEC.
The VEC supports not producing colour bursts for monochrome output.
It also has an option for disabling the chroma input to remove
chroma from the signal.
Now that there is a DRM_MODE_TV_MODE_MONOCHROME defined, plumb
this in.
Signed-off-by: Dave Stevenson
---
drivers/gpu/drm/vc4/vc4_vec.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index 268f18b10ee0..f9e134dd1e3b 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -234,6 +234,7 @@ enum vc4_vec_tv_mode_id {
VC4_VEC_TV_MODE_PAL_60,
VC4_VEC_TV_MODE_PAL_N,
VC4_VEC_TV_MODE_SECAM,
+ VC4_VEC_TV_MODE_MONOCHROME,
};
struct vc4_vec_tv_mode {
@@ -324,6 +325,22 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = {
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
.custom_freq = 0x29c71c72,
},
+ {
+ /* 50Hz mono */
+ .mode = DRM_MODE_TV_MODE_MONOCHROME,
+ .expected_htotal = 864,
+ .config0 = VEC_CONFIG0_PAL_BDGHI_STD | VEC_CONFIG0_BURDIS |
+ VEC_CONFIG0_CHRDIS,
+ .config1 = VEC_CONFIG1_C_CVBS_CVBS,
+ },
+ {
+ /* 60Hz mono */
+ .mode = DRM_MODE_TV_MODE_MONOCHROME,
+ .expected_htotal = 858,
+ .config0 = VEC_CONFIG0_PAL_M_STD | VEC_CONFIG0_BURDIS |
+ VEC_CONFIG0_CHRDIS,
+ .config1 = VEC_CONFIG1_C_CVBS_CVBS,
+ },
};
static inline const struct vc4_vec_tv_mode *
@@ -351,6 +368,7 @@ static const struct drm_prop_enum_list
legacy_tv_mode_names[] = {
{ VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
{ VC4_VEC_TV_MODE_PAL_N, "PAL-N", },
{ VC4_VEC_TV_MODE_SECAM, "SECAM", },
+ { VC4_VEC_TV_MODE_MONOCHROME, "Mono", },
};
static enum drm_connector_status
@@ -406,6 +424,10 @@ vc4_vec_connector_set_property(struct drm_connector
*connector,
state->tv.mode = DRM_MODE_TV_MODE_SECAM;
break;
+ case VC4_VEC_TV_MODE_MONOCHROME:
+ state->tv.mode = DRM_MODE_TV_MODE_MONOCHROME;
+ break;
+
default:
return -EINVAL;
}
@@ -453,6 +475,9 @@ vc4_vec_connector_get_property(struct drm_connector
*connector,
*val = VC4_VEC_TV_MODE_SECAM;
break;
+ case DRM_MODE_TV_MODE_MONOCHROME:
+ return VC4_VEC_TV_MODE_MONOCHROME;
+
default:
return -EINVAL;
}
@@ -754,7 +779,8 @@ static int vc4_vec_bind(struct device *dev, struct device
*master, void *data)
BIT(DRM_MODE_TV_MODE_PAL) |
BIT(DRM_MODE_TV_MODE_PAL_M) |
BIT(DRM_MODE_TV_MODE_PAL_N) |
- BIT(DRM_MODE_TV_MODE_SECAM));
+ BIT(DRM_MODE_TV_MODE_SECAM) |
+ BIT(DRM_MODE_TV_MODE_MONOCHROME));
if (ret)
return ret;
--
2.25.1
