On 2024-02-21 04:07, Pekka Paalanen wrote: > On Fri, 16 Feb 2024 18:48:55 +0000 > Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> wrote: > >> From: Nick Hollinghurst <nick.hollinghurst@xxxxxxxxxxxxxxx> >> >> Add this as a value for enum_drm_connector_tv_mode, represented >> by the string "Mono", to generate video with no colour encoding >> or bursts. Define it to have no pedestal (since only NTSC-M calls >> for a pedestal). >> >> Change default mode creation to acommodate the new tv_mode value >> which comprises both 525-line and 625-line formats. >> >> Signed-off-by: Nick Hollinghurst <nick.hollinghurst@xxxxxxxxxxxxxxx> >> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > > Hi Dave and Nick, > > since no-one else commented yet, I'd like to remind of the new UAPI > requirements: > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements > > AFAIU, adding a new value to an existing enum still counts as new UAPI. > I tend to agree with Pekka. I'm getting tired of seeing new DRM properties without knowing which canonical upstream user-space project uses it. Can someone describe this for the "TV mode" property? Harry > Maybe there is no need for the full treatment here, or maybe there is, > I'm not sure. I think you should make some statement about how the new > UAPI requirements have been addressed. > > Btw. no-one has submitted a record with "TV mode" to > https://drmdb.emersion.fr/ > It only lists the radeon-specific "tv standard" property. I first > thought you had mistaken the property name in the cover letter. > > > Thanks, > pq > >> --- >> drivers/gpu/drm/drm_connector.c | 7 +++++++ >> drivers/gpu/drm/drm_modes.c | 5 ++++- >> drivers/gpu/drm/drm_probe_helper.c | 5 +++-- >> include/drm/drm_connector.h | 7 +++++++ >> 4 files changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c >> index b0516505f7ae..fe05d27f3404 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -1005,6 +1005,7 @@ static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = { >> { DRM_MODE_TV_MODE_PAL_M, "PAL-M" }, >> { DRM_MODE_TV_MODE_PAL_N, "PAL-N" }, >> { DRM_MODE_TV_MODE_SECAM, "SECAM" }, >> + { DRM_MODE_TV_MODE_MONOCHROME, "Mono" }, >> }; >> DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list) >> >> @@ -1697,6 +1698,12 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); >> * TV Mode is CCIR System B (aka 625-lines) together with >> * the SECAM Color Encoding. >> * >> + * Mono: >> + * >> + * Use timings appropriate to the DRM mode, including >> + * equalizing pulses for a 525-line or 625-line mode, >> + * with no pedestal or color encoding. >> + * >> * Drivers can set up this property by calling >> * drm_mode_create_tv_properties(). >> */ >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c >> index c4f88c3a93b7..d274e7b00b79 100644 >> --- a/drivers/gpu/drm/drm_modes.c >> +++ b/drivers/gpu/drm/drm_modes.c >> @@ -530,7 +530,8 @@ static int fill_analog_mode(struct drm_device *dev, >> * @interlace: whether to compute an interlaced mode >> * >> * This function creates a struct drm_display_mode instance suited for >> - * an analog TV output, for one of the usual analog TV mode. >> + * an analog TV output, for one of the usual analog TV modes. Where >> + * this is DRM_MODE_TV_MODE_MONOCHROME, a 625-line mode will be created. >> * >> * Note that @hdisplay is larger than the usual constraints for the PAL >> * and NTSC timings, and we'll choose to ignore most timings constraints >> @@ -568,6 +569,8 @@ struct drm_display_mode *drm_analog_tv_mode(struct drm_device *dev, >> case DRM_MODE_TV_MODE_PAL_N: >> fallthrough; >> case DRM_MODE_TV_MODE_SECAM: >> + fallthrough; >> + case DRM_MODE_TV_MODE_MONOCHROME: >> analog = DRM_MODE_ANALOG_PAL; >> break; >> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >> index d1e1ade66f81..9254dc2af873 100644 >> --- a/drivers/gpu/drm/drm_probe_helper.c >> +++ b/drivers/gpu/drm/drm_probe_helper.c >> @@ -1211,8 +1211,9 @@ int drm_connector_helper_tv_get_modes(struct drm_connector *connector) >> for (i = 0; i < tv_mode_property->num_values; i++) >> supported_tv_modes |= BIT(tv_mode_property->values[i]); >> >> - if ((supported_tv_modes & ntsc_modes) && >> - (supported_tv_modes & pal_modes)) { >> + if (((supported_tv_modes & ntsc_modes) && >> + (supported_tv_modes & pal_modes)) || >> + (supported_tv_modes & BIT(DRM_MODE_TV_MODE_MONOCHROME))) { >> uint64_t default_mode; >> >> if (drm_object_property_get_default_value(&connector->base, >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index fe88d7fc6b8f..90fd0ea0ca09 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -200,6 +200,13 @@ enum drm_connector_tv_mode { >> */ >> DRM_MODE_TV_MODE_SECAM, >> >> + /** >> + * @DRM_MODE_TV_MODE_MONOCHROME: Use timings appropriate to >> + * the DRM mode, including equalizing pulses for a 525-line >> + * or 625-line mode, with no pedestal or color encoding. >> + */ >> + DRM_MODE_TV_MODE_MONOCHROME, >> + >> /** >> * @DRM_MODE_TV_MODE_MAX: Number of analog TV output modes. >> * >
