On Mon, Jul 09, 2018 at 10:40:05AM +0200, Daniel Vetter wrote:
> - switch everything over to inline comments
> - add notes about locking, links to functions and other related stuff
> - also include a note about Ville's soon-to-be-merged
> drm_connector_for_each_possible_encoder().
>
> Also check that all the hyperlinks in drm_connector.h work and fix
> them as needed.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> ---
> include/drm/drm_connector.h | 191 ++++++++++++++++++++++--------------
> 1 file changed, 120 insertions(+), 71 deletions(-)
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 6028638f3108..a0300e3468cb 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -557,8 +557,7 @@ struct drm_connector_funcs {
> * received for this output connector->edid must be NULL.
> *
> * Drivers using the probe helpers should use
> - * drm_helper_probe_single_connector_modes() or
> - * drm_helper_probe_single_connector_modes_nomerge() to implement this
> + * drm_helper_probe_single_connector_modes() to implement this
> * function.
> *
> * RETURNS:
> @@ -769,45 +768,6 @@ struct drm_cmdline_mode {
>
> /**
> * struct drm_connector - central DRM connector control structure
> - * @dev: parent DRM device
> - * @kdev: kernel device for sysfs attributes
> - * @attr: sysfs attributes
> - * @head: list management
> - * @base: base KMS object
> - * @name: human readable name, can be overwritten by the driver
> - * @connector_type: one of the DRM_MODE_CONNECTOR_<foo> types from drm_mode.h
> - * @connector_type_id: index into connector type enum
> - * @interlace_allowed: can this connector handle interlaced modes?
> - * @doublescan_allowed: can this connector handle doublescan?
> - * @stereo_allowed: can this connector handle stereo modes?
> - * @funcs: connector control functions
> - * @edid_blob_ptr: DRM property containing EDID if present
> - * @properties: property tracking for this connector
> - * @dpms: current dpms state
> - * @helper_private: mid-layer private data
> - * @cmdline_mode: mode line parsed from the kernel cmdline for this connector
> - * @force: a DRM_FORCE_<foo> state for forced mode sets
> - * @override_edid: has the EDID been overwritten through debugfs for testing?
> - * @encoder_ids: valid encoders for this connector
> - * @eld: EDID-like data, if present
> - * @latency_present: AV delay info from ELD, if found
> - * @video_latency: video latency info from ELD, if found
> - * @audio_latency: audio latency info from ELD, if found
> - * @null_edid_counter: track sinks that give us all zeros for the EDID
> - * @bad_edid_counter: track sinks that give us an EDID with invalid checksum
> - * @edid_corrupt: indicates whether the last read EDID was corrupt
> - * @debugfs_entry: debugfs directory for this connector
> - * @has_tile: is this connector connected to a tiled monitor
> - * @tile_group: tile group for the connected monitor
> - * @tile_is_single_monitor: whether the tile is one monitor housing
> - * @num_h_tile: number of horizontal tiles in the tile group
> - * @num_v_tile: number of vertical tiles in the tile group
> - * @tile_h_loc: horizontal location of this tile
> - * @tile_v_loc: vertical location of this tile
> - * @tile_h_size: horizontal size of this tile.
> - * @tile_v_size: vertical size of this tile.
> - * @scaling_mode_property: Optional atomic property to control the upscaling.
> - * @content_protection_property: Optional property to control content protection
> *
> * Each connector may be connected to one or more CRTCs, or may be clonable by
> * another connector if they can share a CRTC. Each connector also has a specific
> @@ -815,13 +775,27 @@ struct drm_cmdline_mode {
> * span multiple monitors).
> */
> struct drm_connector {
> + /** @dev: parent DRM device */
> struct drm_device *dev;
> + /** @kdev: kernel device for sysfs attributes */
> struct device *kdev;
> + /** @attr: sysfs attributes */
> struct device_attribute *attr;
> +
> + /**
> + * @head:
> + *
> + * List of all connectors on a @dev, linked from
> + * &drm_mode_config.connector_list. Protected by
> + * &drm_mode_config.connector_list_lock, but please only use
> + * &drm_connector_list_iter to walk this list.
> + */
> struct list_head head;
>
> + /** @base: base KMS object */
> struct drm_mode_object base;
>
> + /** @name: human readable name, can be overwritten by the driver */
> char *name;
>
> /**
> @@ -839,10 +813,30 @@ struct drm_connector {
> */
> unsigned index;
>
> + /**
> + * @connector_type:
> + * one of the DRM_MODE_CONNECTOR_<foo> types from drm_mode.h
> + */
> int connector_type;
> + /** @connector_type_id: index into connector type enum */
> int connector_type_id;
> + /**
> + * @interlace_allowed:
> + * Can this connector handle interlaced modes? Only used by
> + * drm_helper_probe_single_connector_modes() for mode filtering.
> + */
> bool interlace_allowed;
> + /**
> + * @doublescan_allowed:
> + * Can this connector handle doublescan? Only used by
> + * drm_helper_probe_single_connector_modes() for mode filtering.
> + */
> bool doublescan_allowed;
> + /**
> + * @stereo_allowed:
> + * Can this connector handle stereo modes? Only used by
> + * drm_helper_probe_single_connector_modes() for mode filtering.
> + */
> bool stereo_allowed;
>
> /**
> @@ -891,45 +885,42 @@ struct drm_connector {
> * Protected by &drm_mode_config.mutex.
> */
> struct drm_display_info display_info;
> +
> + /** @funcs: connector control functions */
> const struct drm_connector_funcs *funcs;
>
> + /**
> + * @edid_blob_ptr: DRM property containing EDID if present. Protected by
> + * &drm_mode_config.mutex. This should be updated only by calling
> + * drm_mode_connector_update_edid_property().
> + */
> struct drm_property_blob *edid_blob_ptr;
> +
> + /** @properties: property tracking for this connector */
> struct drm_object_properties properties;
>
> + /**
> + * @scaling_mode_property: Optional atomic property to control the
> + * upscaling. See drm_connector_attach_content_protection_property().
> + */
> struct drm_property *scaling_mode_property;
>
> /**
> * @content_protection_property: DRM ENUM property for content
> - * protection
> + * protection. See drm_connector_attach_content_protection_property().
> */
> struct drm_property *content_protection_property;
>
> /**
> * @path_blob_ptr:
> *
> - * DRM blob property data for the DP MST path property.
> + * DRM blob property data for the DP MST path property. This should only
> + * be updated by calling drm_mode_connector_set_path_property().
> */
> struct drm_property_blob *path_blob_ptr;
>
> - /**
> - * @tile_blob_ptr:
> - *
> - * DRM blob property data for the tile property (used mostly by DP MST).
> - * This is meant for screens which are driven through separate display
> - * pipelines represented by &drm_crtc, which might not be running with
> - * genlocked clocks. For tiled panels which are genlocked, like
> - * dual-link LVDS or dual-link DSI, the driver should try to not expose
> - * the tiling and virtualize both &drm_crtc and &drm_plane if needed.
> - */
> - struct drm_property_blob *tile_blob_ptr;
> -
> -/* should we poll this connector for connects and disconnects */
> -/* hot plug detectable */
> #define DRM_CONNECTOR_POLL_HPD (1 << 0)
> -/* poll for connections */
> #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
> -/* can cleanly poll for disconnections without flickering the screen */
> -/* DACs should rarely do this without a lot of testing */
> #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
>
> /**
> @@ -946,25 +937,40 @@ struct drm_connector {
> * Periodically poll the connector for connection.
> *
> * DRM_CONNECTOR_POLL_DISCONNECT
> - * Periodically poll the connector for disconnection.
> + * Periodically poll the connector for disconnection, without
> + * causing flickering even when the connector is in use. DACs should
> + * rarely do this without a lot of testing.
> *
> * Set to 0 for connectors that don't support connection status
> * discovery.
> */
> uint8_t polled;
>
> - /* requested DPMS state */
> + /**
> + * @dpms: Current dpms state. For legacy drivers the
> + * &drm_connector_funcs.dpms callback must update this. For atomic
> + * drivers, this is handled by the core atomic code, and drivers must
> + * only take &drm_crtc_state.active into account.
> + */
> int dpms;
>
> + /** @helper_private: mid-layer private data */
> const struct drm_connector_helper_funcs *helper_private;
>
> - /* forced on connector */
> + /** @cmdline_mode: mode line parsed from the kernel cmdline for this connector */
> struct drm_cmdline_mode cmdline_mode;
> + /** @force: a DRM_FORCE_<foo> state for forced mode sets */
> enum drm_connector_force force;
> + /** @override_edid: has the EDID been overwritten through debugfs for testing? */
> bool override_edid;
>
> #define DRM_CONNECTOR_MAX_ENCODER 3
> + /**
> + * @encoder_ids: Valid encoders for this connector. Please only use
> + * drm_connector_for_each_possible_encoder() to enumerate these.
> + */
> uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
> +
> /**
> * @encoder: Currently bound encoder driving this connector, if any.
> * Only really meaningful for non-atomic drivers. Atomic drivers should
> @@ -974,19 +980,37 @@ struct drm_connector {
> struct drm_encoder *encoder;
>
> #define MAX_ELD_BYTES 128
> - /* EDID bits */
> + /** @eld: EDID-like data, if present */
> uint8_t eld[MAX_ELD_BYTES];
> + /** @latency_present: AV delay info from ELD, if found */
> bool latency_present[2];
> - int video_latency[2]; /* [0]: progressive, [1]: interlaced */
> + /**
> + * @video_latency: Video latency info from ELD, if found.
> + * [0]: progressive, [1]: interlaced
> + */
> + int video_latency[2];
> + /**
> + * @audio_latency: audio latency info from ELD, if found
> + * [0]: progressive, [1]: interlaced
> + */
> int audio_latency[2];
> - int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
> + /**
> + * @null_edid_counter: track sinks that give us all zeros for the EDID.
> + * Needed to workaround some HW bugs where we get all 0s
> + */
> + int null_edid_counter;
> +
> + /** @bad_edid_counter: track sinks that give us an EDID with invalid checksum */
> unsigned bad_edid_counter;
>
> - /* Flag for raw EDID header corruption - used in Displayport
> - * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
> + /**
> + * @edid_corrupt: Indicates whether the last read EDID was corrupt. Used
> + * in Displayport compliance testing - Displayport Link CTS Core 1.2
> + * rev1.1 4.2.2.6
> */
> bool edid_corrupt;
>
> + /** @debugfs_entry: debugfs directory for this connector */
> struct dentry *debugfs_entry;
>
> /**
> @@ -994,7 +1018,7 @@ struct drm_connector {
> *
> * Current atomic state for this connector.
> *
> - * This is protected by @drm_mode_config.connection_mutex. Note that
> + * This is protected by &drm_mode_config.connection_mutex. Note that
> * nonblocking atomic commits access the current connector state without
> * taking locks. Either by going through the &struct drm_atomic_state
> * pointers, see for_each_oldnew_connector_in_state(),
> @@ -1005,19 +1029,44 @@ struct drm_connector {
> */
> struct drm_connector_state *state;
>
> - /* DisplayID bits */
> + /* DisplayID bits. FIXME: Extract into a substruct? */
> +
> + /**
> + * @tile_blob_ptr:
> + *
> + * DRM blob property data for the tile property (used mostly by DP MST).
> + * This is meant for screens which are driven through separate display
> + * pipelines represented by &drm_crtc, which might not be running with
> + * genlocked clocks. For tiled panels which are genlocked, like
> + * dual-link LVDS or dual-link DSI, the driver should try to not expose
> + * the tiling and virtualize both &drm_crtc and &drm_plane if needed.
> + *
> + * This should only be updated by calling
> + * drm_mode_connector_set_tile_property().
> + */
> + struct drm_property_blob *tile_blob_ptr;
> +
> + /** @has_tile: is this connector connected to a tiled monitor */
> bool has_tile;
> + /** @tile_group: tile group for the connected monitor */
> struct drm_tile_group *tile_group;
> + /** @tile_is_single_monitor: whether the tile is one monitor housing */
> bool tile_is_single_monitor;
>
> + /** @num_h_tile: number of horizontal tiles in the tile group */
> + /** @num_v_tile: number of vertical tiles in the tile group */
> uint8_t num_h_tile, num_v_tile;
> + /** @tile_h_loc: horizontal location of this tile */
> + /** @tile_v_loc: vertical location of this tile */
> uint8_t tile_h_loc, tile_v_loc;
> + /** @tile_h_size: horizontal size of this tile. */
> + /** @tile_v_size: vertical size of this tile. */
> uint16_t tile_h_size, tile_v_size;
>
> /**
> * @free_node:
> *
> - * List used only by &drm_connector_iter to be able to clean up a
> + * List used only by &drm_connector_list_iter to be able to clean up a
> * connector from any context, in conjunction with
> * &drm_mode_config.connector_free_work.
> */
> --
> 2.18.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
