[PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Nirmoy Das]

This also replaces old artifacts with a correct one in drm_sched_entity_init()
declaration

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/scheduler/sched_entity.c |  2 +-
 include/drm/gpu_scheduler.h              | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index 33e2cd1089a2..ec79e8e5ad3c 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -45,7 +45,7 @@
  * @guilty: atomic_t set to 1 when a job on this queue
  *          is found to be guilty causing a timeout
  *
- * Note: the rq_list should have atleast one element to schedule
+ * Note: the sched_list should have atleast one element to schedule
  *       the entity
  *
  * Returns 0 on success or a negative error code on failure.
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 537f7a4655a5..9e71be129c30 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -52,9 +52,9 @@ enum drm_sched_priority {
  * @list: used to append this struct to the list of entities in the
  *        runqueue.
  * @rq: runqueue on which this entity is currently scheduled.
- * @rq_list: a list of run queues on which jobs from this entity can
- *           be scheduled
- * @num_rq_list: number of run queues in the rq_list
+ * @sched_list: a list of drm_gpu_schedulers on which jobs from this entity can
+ *              be scheduled
+ * @num_sched_list: number of drm_gpu_schedulers in the sched_list.
  * @rq_lock: lock to modify the runqueue to which this entity belongs.
  * @job_queue: the list of jobs of this entity.
  * @fence_seq: a linearly increasing seqno incremented with each
@@ -81,8 +81,8 @@ enum drm_sched_priority {
 struct drm_sched_entity {
 	struct list_head		list;
 	struct drm_sched_rq		*rq;
-	unsigned int                    num_sched_list;
 	struct drm_gpu_scheduler        **sched_list;
+	unsigned int                    num_sched_list;
 	enum drm_sched_priority         priority;
 	spinlock_t			rq_lock;
 
@@ -315,7 +315,7 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
 int drm_sched_entity_init(struct drm_sched_entity *entity,
 			  enum drm_sched_priority priority,
 			  struct drm_gpu_scheduler **sched_list,
-			  unsigned int num_rq_list,
+			  unsigned int num_sched_list,
 			  atomic_t *guilty);
 long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout);
 void drm_sched_entity_fini(struct drm_sched_entity *entity);
-- 
2.24.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Christian König]

Am 16.01.20 um 15:43 schrieb Nirmoy Das:
> This also replaces old artifacts with a correct one in drm_sched_entity_init()
> declaration
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/scheduler/sched_entity.c |  2 +-
>   include/drm/gpu_scheduler.h              | 10 +++++-----
>   2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 33e2cd1089a2..ec79e8e5ad3c 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -45,7 +45,7 @@
>    * @guilty: atomic_t set to 1 when a job on this queue
>    *          is found to be guilty causing a timeout
>    *
> - * Note: the rq_list should have atleast one element to schedule
> + * Note: the sched_list should have atleast one element to schedule
>    *       the entity
>    *
>    * Returns 0 on success or a negative error code on failure.
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 537f7a4655a5..9e71be129c30 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -52,9 +52,9 @@ enum drm_sched_priority {
>    * @list: used to append this struct to the list of entities in the
>    *        runqueue.
>    * @rq: runqueue on which this entity is currently scheduled.
> - * @rq_list: a list of run queues on which jobs from this entity can
> - *           be scheduled
> - * @num_rq_list: number of run queues in the rq_list
> + * @sched_list: a list of drm_gpu_schedulers on which jobs from this entity can
> + *              be scheduled
> + * @num_sched_list: number of drm_gpu_schedulers in the sched_list.
>    * @rq_lock: lock to modify the runqueue to which this entity belongs.
>    * @job_queue: the list of jobs of this entity.
>    * @fence_seq: a linearly increasing seqno incremented with each
> @@ -81,8 +81,8 @@ enum drm_sched_priority {
>   struct drm_sched_entity {
>   	struct list_head		list;
>   	struct drm_sched_rq		*rq;
> -	unsigned int                    num_sched_list;
>   	struct drm_gpu_scheduler        **sched_list;
> +	unsigned int                    num_sched_list;
>   	enum drm_sched_priority         priority;
>   	spinlock_t			rq_lock;
>   
> @@ -315,7 +315,7 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
>   int drm_sched_entity_init(struct drm_sched_entity *entity,
>   			  enum drm_sched_priority priority,
>   			  struct drm_gpu_scheduler **sched_list,
> -			  unsigned int num_rq_list,
> +			  unsigned int num_sched_list,
>   			  atomic_t *guilty);
>   long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout);
>   void drm_sched_entity_fini(struct drm_sched_entity *entity);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Luben Tuikov]

On 2020-01-16 09:43, Nirmoy Das wrote:
> This also replaces old artifacts with a correct one in drm_sched_entity_init()
> declaration
> 
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>  drivers/gpu/drm/scheduler/sched_entity.c |  2 +-
>  include/drm/gpu_scheduler.h              | 10 +++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 33e2cd1089a2..ec79e8e5ad3c 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -45,7 +45,7 @@
>   * @guilty: atomic_t set to 1 when a job on this queue
>   *          is found to be guilty causing a timeout
>   *
> - * Note: the rq_list should have atleast one element to schedule
> + * Note: the sched_list should have atleast one element to schedule

"atleast" --> "at least".

>   *       the entity
>   *
>   * Returns 0 on success or a negative error code on failure.
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 537f7a4655a5..9e71be129c30 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -52,9 +52,9 @@ enum drm_sched_priority {
>   * @list: used to append this struct to the list of entities in the
>   *        runqueue.
>   * @rq: runqueue on which this entity is currently scheduled.
> - * @rq_list: a list of run queues on which jobs from this entity can
> - *           be scheduled
> - * @num_rq_list: number of run queues in the rq_list
> + * @sched_list: a list of drm_gpu_schedulers on which jobs from this entity can
> + *              be scheduled

I had to read this a few times to understand it. I wonder if splitting
it into two sentences would make it clearer:

"A list of schedulers (drm_gpu_schedulers). Jobs from this entity,
 can be scheduled on any scheduler on this list."

Regards,
Luben


> + * @num_sched_list: number of drm_gpu_schedulers in the sched_list.
>   * @rq_lock: lock to modify the runqueue to which this entity belongs.
>   * @job_queue: the list of jobs of this entity.
>   * @fence_seq: a linearly increasing seqno incremented with each
> @@ -81,8 +81,8 @@ enum drm_sched_priority {
>  struct drm_sched_entity {
>  	struct list_head		list;
>  	struct drm_sched_rq		*rq;
> -	unsigned int                    num_sched_list;
>  	struct drm_gpu_scheduler        **sched_list;
> +	unsigned int                    num_sched_list;
>  	enum drm_sched_priority         priority;
>  	spinlock_t			rq_lock;
>  
> @@ -315,7 +315,7 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
>  int drm_sched_entity_init(struct drm_sched_entity *entity,
>  			  enum drm_sched_priority priority,
>  			  struct drm_gpu_scheduler **sched_list,
> -			  unsigned int num_rq_list,
> +			  unsigned int num_sched_list,
>  			  atomic_t *guilty);
>  long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout);
>  void drm_sched_entity_fini(struct drm_sched_entity *entity);
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Nirmoy]

Hi Luben,

On 1/16/20 6:13 PM, Luben Tuikov wrote:
>> - * Note: the rq_list should have atleast one element to schedule
>> + * Note: the sched_list should have atleast one element to schedule
> "atleast" --> "at least".
Always a tricky one to catch, Thanks!
I will create a patch and ask Alex to squash with this one.
>
>> - * @num_rq_list: number of run queues in the rq_list
>> + * @sched_list: a list of drm_gpu_schedulers on which jobs from this entity can
>> + *              be scheduled
> I had to read this a few times to understand it. I wonder if splitting
> it into two sentences would make it clearer:
>
> "A list of schedulers (drm_gpu_schedulers). Jobs from this entity,
>   can be scheduled on any scheduler on this list."

I don't know, both feels right to me. Please create a patch if you think 
this splitting makes it more clearer.

Regards,

Nirmoy

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Luben Tuikov]

On 2020-01-17 5:59 a.m., Nirmoy wrote:
> Hi Luben,
> 
> On 1/16/20 6:13 PM, Luben Tuikov wrote:
>>> - * Note: the rq_list should have atleast one element to schedule
>>> + * Note: the sched_list should have atleast one element to schedule
>> "atleast" --> "at least".
> Always a tricky one to catch, Thanks!
> I will create a patch and ask Alex to squash with this one.
>>
>>> - * @num_rq_list: number of run queues in the rq_list
>>> + * @sched_list: a list of drm_gpu_schedulers on which jobs from this entity can
>>> + *              be scheduled
>> I had to read this a few times to understand it. I wonder if splitting
>> it into two sentences would make it clearer:
>>
>> "A list of schedulers (drm_gpu_schedulers). Jobs from this entity,
>>   can be scheduled on any scheduler on this list."
> 
> I don't know, both feels right to me. Please create a patch if you think 
> this splitting makes it more clearer.

Oh, god, since you're submitting a patch to fix "atleast" to "at least",
it would've been good to also fix this.

But you chose not to do it. Sure, you understand it, but making it more
clear, surely shows attention to detail and thinking process. And it
would help others to maintain the driver and contribute. Making
it more obfuscated, by bunching up what acts and what, makes it difficult
to understand.

I had to read this sentence a few times to separate the entities,
what acts on what, in order to understand the description.

Bunching this up into a single sentence, invariably makes
the documentation *more difficult* to understand. Breaking it up
into two sentences, makes it much easier to understand. For instance,

The first sentences describes a single object: the list:
"A list of schedulers (drm_gpu_schedulers)."
The second sentence describes the jobs, that they can be
scheduled on any scheduler of the previously talked about list:
"Jobs from this entity, can be scheduled on any scheduler on this list."

Why would you refuse to also update the documentation, as part of your
spelling mistake patch, is beyond me. It only makes sense to make
things better for other people to understand.

And this is how well-written documentation dies.

Regards,
Luben
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Nirmoy Das]

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/scheduler/sched_entity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index 2e3a058fc239..62bcca855c69 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -45,7 +45,7 @@
  * @guilty: atomic_t set to 1 when a job on this queue
  *          is found to be guilty causing a timeout
  *
- * Note: the rq_list should have atleast one element to schedule
+ * Note: the sched_list should have atleast one element to schedule
  *       the entity
  *
  * Returns 0 on success or a negative error code on failure.
-- 
2.24.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Christian König]

Am 14.01.20 um 10:58 schrieb Nirmoy Das:
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/scheduler/sched_entity.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 2e3a058fc239..62bcca855c69 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -45,7 +45,7 @@
>    * @guilty: atomic_t set to 1 when a job on this queue
>    *          is found to be guilty causing a timeout
>    *
> - * Note: the rq_list should have atleast one element to schedule
> + * Note: the sched_list should have atleast one element to schedule
>    *       the entity
>    *
>    * Returns 0 on success or a negative error code on failure.

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Yuan, Xiaojie]

[AMD Official Use Only - Internal Distribution Only]

Hi Nirmoy,

Seems like documentation for struct drm_sched_entity@gpu_scheduler.h need update as well.

BR,
Xiaojie

________________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Nirmoy Das <nirmoy.aiemd@gmail.com>
Sent: Tuesday, January 14, 2020 5:58 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander; Ho, Kenny; Das, Nirmoy; Koenig, Christian
Subject: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/scheduler/sched_entity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index 2e3a058fc239..62bcca855c69 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -45,7 +45,7 @@
  * @guilty: atomic_t set to 1 when a job on this queue
  *          is found to be guilty causing a timeout
  *
- * Note: the rq_list should have atleast one element to schedule
+ * Note: the sched_list should have atleast one element to schedule
  *       the entity
  *
  * Returns 0 on success or a negative error code on failure.
--
2.24.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CXiaojie.Yuan%40amd.com%7Cb5b5e9e4c2a34ac6b50308d798d81903%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637145926254946780&amp;sdata=ZgkmwrForbuXnPGGHz2UVE67anyILWedAMNlcSA%2BvUw%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

[Nirmoy]

On 1/15/20 3:14 AM, Yuan, Xiaojie wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Nirmoy,
>
> Seems like documentation for struct drm_sched_entity@gpu_scheduler.h need update as well.

Yes that needs fixing too,

Thanks,

Nirmoy

>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Nirmoy Das <nirmoy.aiemd@gmail.com>
> Sent: Tuesday, January 14, 2020 5:58 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander; Ho, Kenny; Das, Nirmoy; Koenig, Christian
> Subject: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/scheduler/sched_entity.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 2e3a058fc239..62bcca855c69 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -45,7 +45,7 @@
>    * @guilty: atomic_t set to 1 when a job on this queue
>    *          is found to be guilty causing a timeout
>    *
> - * Note: the rq_list should have atleast one element to schedule
> + * Note: the sched_list should have atleast one element to schedule
>    *       the entity
>    *
>    * Returns 0 on success or a negative error code on failure.
> --
> 2.24.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CXiaojie.Yuan%40amd.com%7Cb5b5e9e4c2a34ac6b50308d798d81903%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637145926254946780&amp;sdata=ZgkmwrForbuXnPGGHz2UVE67anyILWedAMNlcSA%2BvUw%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx