This is RFC patch for adding a connector link-status property
and making it atomic by adding it to the drm_connector_state.
This is to make sure its wired properly in drm_atomic_connector_set_property
and drm_atomic_connector_get_property functions.

Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: Daniel Vetter <daniel.vetter at intel.com>
Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
---
 drivers/gpu/drm/drm_atomic.c    |  5 ++++
 drivers/gpu/drm/drm_connector.c | 65 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_connector.h     | 12 +++++++-
 include/drm/drm_mode_config.h   |  5 ++++
 include/uapi/drm/drm_mode.h     |  4 +++
 5 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 89737e4..644d19f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
                 * now?) atomic writes to DPMS property:
                 */
                return -EINVAL;
+       } else if (property == config->link_status_property) {
+               state->link_status = val;
+               return 0;
        } else if (connector->funcs->atomic_set_property) {
                return connector->funcs->atomic_set_property(connector,
                                state, property, val);
@@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
                *val = (state->crtc) ? state->crtc->base.id : 0;
        } else if (property == config->dpms_property) {
                *val = connector->dpms;
+       } else if (property == config->link_status_property) {
+               *val = state->link_status;
        } else if (connector->funcs->atomic_get_property) {
                return connector->funcs->atomic_get_property(connector,
                                state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 5a45262..4576c9f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
        drm_object_attach_property(&connector->base,
                                      config->dpms_property, 0);

+       drm_object_attach_property(&connector->base,
+                                  config->link_status_property,
+                                  0);
+
        if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
                drm_object_attach_property(&connector->base, 
config->prop_crtc_id, 0);
        }
@@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum 
subpixel_order order)
 };
 DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)

+static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
+       { DRM_MODE_LINK_STATUS_GOOD, "Good" },
+       { DRM_MODE_LINK_STATUS_BAD, "Bad" },
+};
+DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
+
 /**
  * drm_display_info_set_bus_formats - set the supported bus formats
  * @info: display info to store bus formats in
@@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct 
drm_display_info *info,
  *     path property the MST manager created. Userspace cannot change this
  *     property.
  * TILE:
- *     Connector tile group property to indicate how a set of DRM connector
+ *      Connector tile group property to indicate how a set of DRM connector
  *     compose together into one logical screen. This is used by both high-res
  *     external screens (often only using a single cable, but exposing multiple
  *     DP MST sinks), or high-res integrated panels (like dual-link DSI) which
@@ -625,7 +635,14 @@ int drm_display_info_set_bus_formats(struct 
drm_display_info *info,
  *     tiling and virtualize both &drm_crtc and &drm_plane if needed. Drivers
  *     should update this value using drm_mode_connector_set_tile_property().
  *     Userspace cannot change this property.
- *
+ * link-status:
+ *      Connector link-status property to indicate the status of link during
+ *      the modeset. The default value of link-status is "GOOD". If something
+ *      fails during modeset, the kernel driver can set this to "BAD", prune
+ *      the mode list based on new link parameters and send a hotplug uevent
+ *      to notify userspace to re-check the valid modes through GET_CONNECTOR
+ *      IOCTL and redo a modeset. Drivers should update this value using
+ *      drm_mode_connector_set_link_status_property().
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -666,6 +683,13 @@ int drm_connector_create_standard_properties(struct 
drm_device *dev)
                return -ENOMEM;
        dev->mode_config.tile_property = prop;

+       prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC, 
"link-status",
+                                       drm_link_status_enum_list,
+                                       ARRAY_SIZE(drm_link_status_enum_list));
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.link_status_property = prop;
+
        return 0;
 }

@@ -995,6 +1019,43 @@ int drm_mode_connector_update_edid_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_mode_connector_update_edid_property);

+/**
+ * drm_mode_connector_set_link_status_property - Set link status property of a 
connector
+ * @connector: drm connector
+ * @link_status: new value of link status property (0: Good, 1: Bad)
+ *
+ * In usual working scenario, this link status property will always be set to
+ * "GOOD". If something fails during or after a mode set, the kernel driver 
should
+ * set this link status property to "BAD" and prune the mode list based on new
+ * information. The caller then needs to send a hotplug uevent for userspace to
+ *  re-check the valid modes through GET_CONNECTOR_IOCTL and retry modeset.
+ *
+ * Note that a lot of existing userspace do not handle this property.
+ * Drivers can therefore not rely on userspace to fix up everything and
+ * should try to handle issues (like just re-training a link) without
+ * userspace's intervention. This should only be used when the current mode
+ * fails and userspace must select a different display mode.
+ * The DRM driver can chose to not modify property and keep link status
+ * as "GOOD" always to keep the user experience same as it currently is.
+ *
+ * The reason for adding this property is to handle link training failures, but
+ * it is not limited to DP or link training. For example, if we implement
+ * asynchronous setcrtc, this property can be used to report any failures in 
that.
+ */
+void drm_mode_connector_set_link_status_property(struct drm_connector 
*connector,
+                                                uint64_t link_status)
+{
+       struct drm_device *dev = connector->dev;
+
+       /* Make sure the mutex is grabbed */
+       lockdep_assert_held(&dev->mode_config.mutex);
+       connector->link_status = link_status;
+       drm_object_property_set_value(&connector->base,
+                                     dev->mode_config.link_status_property,
+                                     link_status);
+}
+EXPORT_SYMBOL(drm_mode_connector_set_link_status_property);
+
 int drm_mode_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 34f9741..3d513ab 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -213,6 +213,9 @@ struct drm_connector_state {

        struct drm_encoder *best_encoder;

+       /* Connector Link status */
+       unsigned int link_status;
+
        struct drm_atomic_state *state;
 };

@@ -695,6 +698,12 @@ struct drm_connector {
        uint8_t num_h_tile, num_v_tile;
        uint8_t tile_h_loc, tile_v_loc;
        uint16_t tile_h_size, tile_v_size;
+
+       /* Connector Link status
+        * 0: If the link is Good
+        * 1: If the link is Bad
+        */
+       int link_status;
 };

 #define obj_to_connector(x) container_of(x, struct drm_connector, base)
@@ -767,12 +776,13 @@ int drm_mode_create_tv_properties(struct drm_device *dev,
 int drm_mode_create_scaling_mode_property(struct drm_device *dev);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
-
 int drm_mode_connector_set_path_property(struct drm_connector *connector,
                                         const char *path);
 int drm_mode_connector_set_tile_property(struct drm_connector *connector);
 int drm_mode_connector_update_edid_property(struct drm_connector *connector,
                                            const struct edid *edid);
+void drm_mode_connector_set_link_status_property(struct drm_connector 
*connector,
+                                                uint64_t link_status);

 /**
  * struct drm_tile_group - Tile group metadata
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index bf9991b..86faee4 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -431,6 +431,11 @@ struct drm_mode_config {
         */
        struct drm_property *tile_property;
        /**
+        * @link_status_property: Default connector property for link status
+        * of a connector
+        */
+       struct drm_property *link_status_property;
+       /**
         * @plane_type_property: Default plane property to differentiate
         * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
         */
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 728790b..309c478 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -123,6 +123,10 @@
 #define DRM_MODE_DIRTY_ON       1
 #define DRM_MODE_DIRTY_ANNOTATE 2

+/* Link Status options */
+#define DRM_MODE_LINK_STATUS_GOOD      0
+#define DRM_MODE_LINK_STATUS_BAD       1
+
 struct drm_mode_modeinfo {
        __u32 clock;
        __u16 hdisplay;
-- 
1.9.1

Reply via email to