Thomas Zimmermann
2022-Sep-23 08:19 UTC
[Nouveau] [PATCH v2 06/33] drm/connector: Rename legacy TV property
Hi Am 22.09.22 um 16:25 schrieb Maxime Ripard:> The current tv_mode has driver-specific values that don't allow to > easily share code using it, either at the userspace or kernel level. > > Since we're going to introduce a new, generic, property that fit the > same purpose, let's rename this one to legacy_tv_mode to make it > obvious we should move away from it. > > Signed-off-by: Maxime Ripard <maxime at cerno.tech>It's not wrong, but 'legacy' is already overloaded with meaning. If you can, maybe name it 'driver_tv_mode_property' or 'custom_tv_mode_property' instead. Acked-by: Thomas Zimmermann <tzimmermann at suse.de>> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index c06d0639d552..7f2b9a07fbdf 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -698,8 +698,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, > state->tv.margins.top = val; > } else if (property == config->tv_bottom_margin_property) { > state->tv.margins.bottom = val; > - } else if (property == config->tv_mode_property) { > - state->tv.mode = val; > + } else if (property == config->legacy_tv_mode_property) { > + state->tv.legacy_mode = val; > } else if (property == config->tv_brightness_property) { > state->tv.brightness = val; > } else if (property == config->tv_contrast_property) { > @@ -808,8 +808,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, > *val = state->tv.margins.top; > } else if (property == config->tv_bottom_margin_property) { > *val = state->tv.margins.bottom; > - } else if (property == config->tv_mode_property) { > - *val = state->tv.mode; > + } else if (property == config->legacy_tv_mode_property) { > + *val = state->tv.legacy_mode; > } else if (property == config->tv_brightness_property) { > *val = state->tv.brightness; > } else if (property == config->tv_contrast_property) { > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index e3142c8142b3..ede6025638d7 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1686,14 +1686,14 @@ int drm_mode_create_tv_properties(struct drm_device *dev, > if (drm_mode_create_tv_margin_properties(dev)) > goto nomem; > > - dev->mode_config.tv_mode_property > + dev->mode_config.legacy_tv_mode_property > drm_property_create(dev, DRM_MODE_PROP_ENUM, > "mode", num_modes); > - if (!dev->mode_config.tv_mode_property) > + if (!dev->mode_config.legacy_tv_mode_property) > goto nomem; > > for (i = 0; i < num_modes; i++) > - drm_property_add_enum(dev->mode_config.tv_mode_property, > + drm_property_add_enum(dev->mode_config.legacy_tv_mode_property, > i, modes[i]); > > dev->mode_config.tv_brightness_property > diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c > index fa636206f232..86e992b2108b 100644 > --- a/drivers/gpu/drm/gud/gud_connector.c > +++ b/drivers/gpu/drm/gud/gud_connector.c > @@ -303,7 +303,7 @@ static int gud_connector_atomic_check(struct drm_connector *connector, > old_state->tv.margins.right != new_state->tv.margins.right || > old_state->tv.margins.top != new_state->tv.margins.top || > old_state->tv.margins.bottom != new_state->tv.margins.bottom || > - old_state->tv.mode != new_state->tv.mode || > + old_state->tv.legacy_mode != new_state->tv.legacy_mode || > old_state->tv.brightness != new_state->tv.brightness || > old_state->tv.contrast != new_state->tv.contrast || > old_state->tv.flicker_reduction != new_state->tv.flicker_reduction || > @@ -424,7 +424,7 @@ gud_connector_property_lookup(struct drm_connector *connector, u16 prop) > case GUD_PROPERTY_TV_BOTTOM_MARGIN: > return config->tv_bottom_margin_property; > case GUD_PROPERTY_TV_MODE: > - return config->tv_mode_property; > + return config->legacy_tv_mode_property; > case GUD_PROPERTY_TV_BRIGHTNESS: > return config->tv_brightness_property; > case GUD_PROPERTY_TV_CONTRAST: > @@ -454,7 +454,7 @@ static unsigned int *gud_connector_tv_state_val(u16 prop, struct drm_tv_connecto > case GUD_PROPERTY_TV_BOTTOM_MARGIN: > return &state->margins.bottom; > case GUD_PROPERTY_TV_MODE: > - return &state->mode; > + return &state->legacy_mode; > case GUD_PROPERTY_TV_BRIGHTNESS: > return &state->brightness; > case GUD_PROPERTY_TV_CONTRAST: > diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c > index b91e48d2190d..d29b63fd6178 100644 > --- a/drivers/gpu/drm/i2c/ch7006_drv.c > +++ b/drivers/gpu/drm/i2c/ch7006_drv.c > @@ -264,7 +264,7 @@ static int ch7006_encoder_create_resources(struct drm_encoder *encoder, > priv->hmargin); > drm_object_attach_property(&connector->base, conf->tv_bottom_margin_property, > priv->vmargin); > - drm_object_attach_property(&connector->base, conf->tv_mode_property, > + drm_object_attach_property(&connector->base, conf->legacy_tv_mode_property, > priv->norm); > drm_object_attach_property(&connector->base, conf->tv_brightness_property, > priv->brightness); > @@ -315,7 +315,7 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder, > ch7006_load_reg(client, state, CH7006_POV); > ch7006_load_reg(client, state, CH7006_VPOS); > > - } else if (property == conf->tv_mode_property) { > + } else if (property == conf->legacy_tv_mode_property) { > if (connector->dpms != DRM_MODE_DPMS_OFF) > return -EINVAL; > > diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c > index 9379f3463344..abaf9ded942d 100644 > --- a/drivers/gpu/drm/i915/display/intel_tv.c > +++ b/drivers/gpu/drm/i915/display/intel_tv.c > @@ -1986,7 +1986,8 @@ intel_tv_init(struct drm_i915_private *dev_priv) > } > drm_mode_create_tv_properties(dev, i, tv_format_names); > > - drm_object_attach_property(&connector->base, dev->mode_config.tv_mode_property, > + drm_object_attach_property(&connector->base, > + dev->mode_config.legacy_tv_mode_property, > state->tv.mode); > drm_object_attach_property(&connector->base, > dev->mode_config.tv_left_margin_property, > diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c > index be28e7bd7490..1a15534adc60 100644 > --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c > +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c > @@ -662,7 +662,7 @@ static int nv17_tv_create_resources(struct drm_encoder *encoder, > conf->tv_subconnector_property, > tv_enc->subconnector); > drm_object_attach_property(&connector->base, > - conf->tv_mode_property, > + conf->legacy_tv_mode_property, > tv_enc->tv_norm); > drm_object_attach_property(&connector->base, > conf->tv_flicker_reduction_property, > @@ -722,7 +722,7 @@ static int nv17_tv_set_property(struct drm_encoder *encoder, > if (encoder->crtc) > nv17_tv_update_rescaler(encoder); > > - } else if (property == conf->tv_mode_property) { > + } else if (property == conf->legacy_tv_mode_property) { > if (connector->dpms != DRM_MODE_DPMS_OFF) > return -EINVAL; > > diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c > index 0b3333865702..77c50ecb0309 100644 > --- a/drivers/gpu/drm/vc4/vc4_vec.c > +++ b/drivers/gpu/drm/vc4/vc4_vec.c > @@ -274,7 +274,7 @@ static int vc4_vec_connector_get_modes(struct drm_connector *connector) > struct drm_display_mode *mode; > > mode = drm_mode_duplicate(connector->dev, > - vc4_vec_tv_modes[state->tv.mode].mode); > + vc4_vec_tv_modes[state->tv.legacy_mode].mode); > if (!mode) { > DRM_ERROR("Failed to create a new display mode\n"); > return -ENOMEM; > @@ -312,7 +312,7 @@ static int vc4_vec_connector_init(struct drm_device *dev, struct vc4_vec *vec) > drm_connector_helper_add(connector, &vc4_vec_connector_helper_funcs); > > drm_object_attach_property(&connector->base, > - dev->mode_config.tv_mode_property, > + dev->mode_config.legacy_tv_mode_property, > VC4_VEC_TV_MODE_NTSC); > > drm_connector_attach_encoder(connector, &vec->encoder.base); > @@ -449,7 +449,7 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, > { > const struct vc4_vec_tv_mode *vec_mode; > > - vec_mode = &vc4_vec_tv_modes[conn_state->tv.mode]; > + vec_mode = &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; > > if (conn_state->crtc && > !drm_mode_equal(vec_mode->mode, &crtc_state->adjusted_mode)) > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 1d5e3cccb9e3..5cfad8b6ad83 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -695,7 +695,7 @@ struct drm_connector_tv_margins { > * @select_subconnector: selected subconnector > * @subconnector: detected subconnector > * @margins: TV margins > - * @mode: TV mode > + * @legacy_mode: Legacy TV mode, driver specific value > * @brightness: brightness in percent > * @contrast: contrast in percent > * @flicker_reduction: flicker reduction in percent > @@ -707,7 +707,7 @@ struct drm_tv_connector_state { > enum drm_mode_subconnector select_subconnector; > enum drm_mode_subconnector subconnector; > struct drm_connector_tv_margins margins; > - unsigned int mode; > + unsigned int legacy_mode; > unsigned int brightness; > unsigned int contrast; > unsigned int flicker_reduction; > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 6b5e01295348..35a827175c24 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -714,11 +714,13 @@ struct drm_mode_config { > * between different TV connector types. > */ > struct drm_property *tv_select_subconnector_property; > + > /** > - * @tv_mode_property: Optional TV property to select > + * @legacy_tv_mode_property: Optional TV property to select > * the output TV mode. > */ > - struct drm_property *tv_mode_property; > + struct drm_property *legacy_tv_mode_property; > + > /** > * @tv_left_margin_property: Optional TV property to set the left > * margin (expressed in pixels). >-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 N?rnberg, Germany (HRB 36809, AG N?rnberg) Gesch?ftsf?hrer: Ivo Totev -------------- next part -------------- A non-text attachment was scrubbed... Name: OpenPGP_signature Type: application/pgp-signature Size: 840 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20220923/073bf1ff/attachment.sig>
Maxime Ripard
2022-Sep-26 09:50 UTC
[Nouveau] [PATCH v2 06/33] drm/connector: Rename legacy TV property
Hi Thomas, On Fri, Sep 23, 2022 at 10:19:08AM +0200, Thomas Zimmermann wrote:> Hi > > Am 22.09.22 um 16:25 schrieb Maxime Ripard: > > The current tv_mode has driver-specific values that don't allow to > > easily share code using it, either at the userspace or kernel level. > > > > Since we're going to introduce a new, generic, property that fit the > > same purpose, let's rename this one to legacy_tv_mode to make it > > obvious we should move away from it. > > > > Signed-off-by: Maxime Ripard <maxime at cerno.tech> > > It's not wrong, but 'legacy' is already overloaded with meaning. If you can, > maybe name it 'driver_tv_mode_property' or 'custom_tv_mode_property' > instead. > > Acked-by: Thomas Zimmermann <tzimmermann at suse.de>I'd really like to point out that new drivers shouldn't be using this. If we're using either of your proposals then writers might get the impression that this is ok to us. Would you prefer deprecated to legacy? Maxime -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20220926/8fb6a18b/attachment-0001.sig>