[RESEND,RFC,06/18] drm/display/dp_mst: Add some missing kdocs for atomic MST structs

Commit Message

Lyude Paul June 7, 2022, 7:29 p.m. UTC

Since we're about to start adding some stuff here, we may as well fill in
any missing documentation that we forgot to write.

Signed-off-by: Lyude Paul <[e-mail]>
Cc: Wayne Lin <[e-mail]>
Cc: Ville Syrjälä <[e-mail]>
Cc: Fangzhi Zuo <[e-mail]>
Cc: Jani Nikula <[e-mail]>
Cc: Imre Deak <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: Sean Paul <[e-mail]>
---
 include/drm/display/drm_dp_mst_helper.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Wayne Lin June 15, 2022, 4:43 a.m. UTC | #1

[Public]



> -----Original Message-----
> From: Lyude Paul <[e-mail]>
> Sent: Wednesday, June 8, 2022 3:29 AM
> To: [e-mail]; [e-mail]; amd-
> [e-mail]
> Cc: Lin, Wayne <[e-mail]>; Ville Syrjälä
> <[e-mail]>; Zuo, Jerry <[e-mail]>; Jani Nikula
> <[e-mail]>; Imre Deak <[e-mail]>; Daniel Vetter
> <[e-mail]>; Sean Paul <[e-mail]>; David Airlie
> <[e-mail]>; Daniel Vetter <[e-mail]>; Thomas Zimmermann
> <[e-mail]>; Deucher, Alexander
> <[e-mail]>; Lakha, Bhawanpreet
> <[e-mail]>; open list <[e-mail]>
> Subject: [RESEND RFC 06/18] drm/display/dp_mst: Add some missing kdocs
> for atomic MST structs
> 
> Since we're about to start adding some stuff here, we may as well fill in any
> missing documentation that we forgot to write.
> 
> Signed-off-by: Lyude Paul <[e-mail]>
> Cc: Wayne Lin <[e-mail]>
> Cc: Ville Syrjälä <[e-mail]>
> Cc: Fangzhi Zuo <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Imre Deak <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> ---
>  include/drm/display/drm_dp_mst_helper.h | 24
> ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/include/drm/display/drm_dp_mst_helper.h
> b/include/drm/display/drm_dp_mst_helper.h
> index 8ab4f14f2344..eb0ea578b227 100644
> --- a/include/drm/display/drm_dp_mst_helper.h
> +++ b/include/drm/display/drm_dp_mst_helper.h
> @@ -542,19 +542,43 @@ struct drm_dp_payload {
> 
>  #define to_dp_mst_topology_state(x) container_of(x, struct
> drm_dp_mst_topology_state, base)
> 
> +/**
> + * struct drm_dp_mst_atomic_payload - Atomic state struct for an MST
> +payload
> + *
> + * The primary atomic state structure for a given MST payload. Stores
> +information like current
> + * bandwidth allocation, intended action for this payload, etc.
> + */
>  struct drm_dp_mst_atomic_payload {
> +	/** @port: The MST port assigned to this payload */
>  	struct drm_dp_mst_port *port;
> +	/** @time_slots: The number of timeslots allocated to this payload

Hi Lyude,
Would like to also indicate that the timeslots is allocated from source DP Tx
to immediate downstream DP Rx.

> */
>  	int time_slots;
> +	/** @pbn: The payload bandwidth for this payload */
>  	int pbn;
> +	/** @dsc_enabled: Whether or not this payload has DSC enabled */
>  	bool dsc_enabled;
> +
> +	/** @next: The list node for this payload */
>  	struct list_head next;
>  };
> 
> +/**
> + * struct drm_dp_mst_topology_state - DisplayPort MST topology atomic
> +state
> + *
> + * This struct represents the atomic state of the toplevel DisplayPort
> +MST manager  */
>  struct drm_dp_mst_topology_state {
> +	/** @base: Base private state for atomic */
>  	struct drm_private_state base;
> +
> +	/** @payloads: The list of payloads being created/destroyed in this
> +state */
>  	struct list_head payloads;
> +	/** @mgr: The topology manager */
>  	struct drm_dp_mst_topology_mgr *mgr;
> +
> +	/** @total_avail_slots: The total number of slots this topology can
> +handle (63 or 64) */
>  	u8 total_avail_slots;

I would prefer not using the term "available" which is a bit conflicting with
the idea in the reply of ENUM_PATH_RESOURCES - full PBN & available PBN.
Maybe better to change to use "full_slots"?

Not yet finish all the patches. Will try to go through all the patches recently : )

> +	/** @start_slot: The first usable time slot in this topology (1 or 0)
> +*/
>  	u8 start_slot;
>  };
> 
> --
> 2.35.3

--
Regards,
Wayne Lin

Lyude Paul Aug. 8, 2022, 11:07 p.m. UTC | #2

Also JFYI - I did see this comment but didn't address it in my respin, but
only because I figured this might be better for a followup patch considering
how much work is already in here

On Wed, 2022-06-15 at 04:43 +0000, Lin, Wayne wrote:
> I would prefer not using the term "available" which is a bit conflicting with
> the idea in the reply of ENUM_PATH_RESOURCES - full PBN & available PBN.
> Maybe better to change to use "full_slots"?
> 
> Not yet finish all the patches. Will try to go through all the patches recently : )

Patch

diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index 8ab4f14f2344..eb0ea578b227 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -542,19 +542,43 @@  struct drm_dp_payload {
 
 #define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base)
 
+/**
+ * struct drm_dp_mst_atomic_payload - Atomic state struct for an MST payload
+ *
+ * The primary atomic state structure for a given MST payload. Stores information like current
+ * bandwidth allocation, intended action for this payload, etc.
+ */
 struct drm_dp_mst_atomic_payload {
+	/** @port: The MST port assigned to this payload */
 	struct drm_dp_mst_port *port;
+	/** @time_slots: The number of timeslots allocated to this payload */
 	int time_slots;
+	/** @pbn: The payload bandwidth for this payload */
 	int pbn;
+	/** @dsc_enabled: Whether or not this payload has DSC enabled */
 	bool dsc_enabled;
+
+	/** @next: The list node for this payload */
 	struct list_head next;
 };
 
+/**
+ * struct drm_dp_mst_topology_state - DisplayPort MST topology atomic state
+ *
+ * This struct represents the atomic state of the toplevel DisplayPort MST manager
+ */
 struct drm_dp_mst_topology_state {
+	/** @base: Base private state for atomic */
 	struct drm_private_state base;
+
+	/** @payloads: The list of payloads being created/destroyed in this state */
 	struct list_head payloads;
+	/** @mgr: The topology manager */
 	struct drm_dp_mst_topology_mgr *mgr;
+
+	/** @total_avail_slots: The total number of slots this topology can handle (63 or 64) */
 	u8 total_avail_slots;
+	/** @start_slot: The first usable time slot in this topology (1 or 0) */
 	u8 start_slot;
 };