Re: [PATCH v4 1/3] gpu: drm: separate panel orientation property creating and value setting

On Wed, Apr 28, 2021 at 12:55 AM Hsin-Yi Wang hsinyi@chromium.org wrote:

Thank you for revising the patchset, this looks much better to me!

/snip

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 7631f76e7f34..cda83314d7ad 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c

/snip

@@ -2173,8 +2173,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);

• @connector: connector for which to set the panel-orientation property.
• @panel_orientation: drm_panel_orientation value to set

• • This function sets the connector's panel_orientation and attaches
• • a "panel orientation" property to the connector.

• • This function sets the connector's panel_orientation value. If the property
• • doesn't exist, it will create one first.

I still don't think this function should overload initialization and assignment. I'd prefer we just removed the creation from set_panel_orientation entirely so this WARN can't be hit.

• Calling this function on a connector where the panel_orientation has
• already been set is a no-op (e.g. the orientation has been overridden with

@@ -2206,18 +2206,16 @@ int drm_connector_set_panel_orientation(

    prop = dev->mode_config.panel_orientation_property;
    if (!prop) {

•           prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
  

•                           "panel orientation",
  

•                           drm_panel_orientation_enum_list,
  

•                           ARRAY_SIZE(drm_panel_orientation_enum_list));
  

•           if (!prop)
  

•                   return -ENOMEM;
  

•           int ret;
  
  

•           dev->mode_config.panel_orientation_property = prop;
  

•           ret = drm_connector_init_panel_orientation_property(connector);
  

•           if (ret)
  

•                   return ret;
  

•           prop = dev->mode_config.panel_orientation_property;
    }
  
  

•   drm_object_attach_property(&connector->base, prop,
  

•                              info->panel_orientation);
  

•   drm_object_property_set_value(&connector->base, prop,
  

•                                 info->panel_orientation);
    return 0;
  

} EXPORT_SYMBOL(drm_connector_set_panel_orientation); @@ -2225,7 +2223,7 @@ EXPORT_SYMBOL(drm_connector_set_panel_orientation); /**

• drm_connector_set_panel_orientation_with_quirk -

• set the connector's panel_orientation after checking for quirks
  

• • @connector: connector for which to init the panel-orientation property.

• • @connector: connector for which to set the panel-orientation property.
  • @panel_orientation: drm_panel_orientation value to set
  • @width: width in pixels of the panel, used for panel quirk detection
  • @height: height in pixels of the panel, used for panel quirk detection

@@ -2252,6 +2250,40 @@ int drm_connector_set_panel_orientation_with_quirk( } EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);

+/**

• • drm_connector_init_panel_orientation_property -

• • create the connector's panel orientation property
    

• • This function attaches a "panel orientation" property to the connector
• • and initializes its value to DRM_MODE_PANEL_ORIENTATION_UNKNOWN.
• • The value of the property can be set by drm_connector_set_panel_orientation()
• • or drm_connector_set_panel_orientation_with_quirk() later.
• • Returns:
• • Zero on success, negative errno on failure.
• */

+int drm_connector_init_panel_orientation_property(

•   struct drm_connector *connector)
  

+{

•   struct drm_device *dev = connector->dev;
  

•   struct drm_property *prop;
  

•   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
  

•                   "panel orientation",
  

•                   drm_panel_orientation_enum_list,
  

•                   ARRAY_SIZE(drm_panel_orientation_enum_list));
  

•   if (!prop)
  

•           return -ENOMEM;
  

•   dev->mode_config.panel_orientation_property = prop;
  

•   drm_object_attach_property(&connector->base, prop,
  

•                              DRM_MODE_PANEL_ORIENTATION_UNKNOWN);
  

•   return 0;
  

+} +EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);

int drm_connector_set_obj_prop(struct drm_mode_object *obj, struct drm_property *property, uint64_t value) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 1922b278ffad..4396c1c4a5db 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1696,6 +1696,8 @@ int drm_connector_set_panel_orientation_with_quirk( struct drm_connector *connector, enum drm_panel_orientation panel_orientation, int width, int height); +int drm_connector_init_panel_orientation_property(

•   struct drm_connector *connector);
  

int drm_connector_attach_max_bpc_property(struct drm_connector *connector, int min, int max);

-- 2.31.1.498.g6c1eba8ee3d-goog