[PATCH] A simple doc fix

[PATCH] A simple doc fix

[Anup K Parikh]

Fix two warnings during doc build which also results in corresponding
additions in generated docs

Warnings Fixed:
1. include/drm/gpu_scheduler.h:462: warning: Function parameter or member
   'dev' not described in 'drm_gpu_scheduler'
2. drivers/gpu/drm/scheduler/sched_main.c:1005: warning: Function
   parameter or member 'dev' not described in 'drm_sched_init'

Signed-off-by: Anup K Parikh <parikhanupk.foss@gmail.com>
---
 drivers/gpu/drm/scheduler/sched_main.c | 1 +
 include/drm/gpu_scheduler.h            | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 68317d3a7a27..875d00213849 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -994,6 +994,7 @@ static int drm_sched_main(void *param)
  *		used
  * @score: optional score atomic shared with other schedulers
  * @name: name used for debugging
+ * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
  *
  * Return 0 on success, otherwise error code.
  */
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index addb135eeea6..920b91fd1719 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -435,6 +435,7 @@ struct drm_sched_backend_ops {
  * @_score: score used when the driver doesn't provide one
  * @ready: marks if the underlying HW is ready to work
  * @free_guilty: A hit to time out handler to free the guilty job.
+ * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
  *
  * One scheduler is implemented for each hardware ring.
  */
-- 
2.35.1

Re: [PATCH] A simple doc fix

[Andrey Grodzovsky]

On 2022-09-14 06:36, Anup K Parikh wrote:
> Fix two warnings during doc build which also results in corresponding
> additions in generated docs
>
> Warnings Fixed:
> 1. include/drm/gpu_scheduler.h:462: warning: Function parameter or member
>     'dev' not described in 'drm_gpu_scheduler'
> 2. drivers/gpu/drm/scheduler/sched_main.c:1005: warning: Function
>     parameter or member 'dev' not described in 'drm_sched_init'
>
> Signed-off-by: Anup K Parikh <parikhanupk.foss@gmail.com>
> ---
>   drivers/gpu/drm/scheduler/sched_main.c | 1 +
>   include/drm/gpu_scheduler.h            | 1 +
>   2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 68317d3a7a27..875d00213849 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -994,6 +994,7 @@ static int drm_sched_main(void *param)
>    *		used
>    * @score: optional score atomic shared with other schedulers
>    * @name: name used for debugging
> + * @dev: A device pointer for use in error reporting in a multiple GPU scenario.


Why multiple GPUs scenario ? It's also used in single GPU scenario.

Andrey


>    *
>    * Return 0 on success, otherwise error code.
>    */
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index addb135eeea6..920b91fd1719 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -435,6 +435,7 @@ struct drm_sched_backend_ops {
>    * @_score: score used when the driver doesn't provide one
>    * @ready: marks if the underlying HW is ready to work
>    * @free_guilty: A hit to time out handler to free the guilty job.
> + * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
>    *
>    * One scheduler is implemented for each hardware ring.
>    */

Re: [PATCH] A simple doc fix

[Anup K Parikh]

On Wed, Sep 14, 2022 at 10:24:36AM -0400, Andrey Grodzovsky wrote:
> 
> On 2022-09-14 06:36, Anup K Parikh wrote:
> > Fix two warnings during doc build which also results in corresponding
> > additions in generated docs
> > 
> > Warnings Fixed:
> > 1. include/drm/gpu_scheduler.h:462: warning: Function parameter or member
> >     'dev' not described in 'drm_gpu_scheduler'
> > 2. drivers/gpu/drm/scheduler/sched_main.c:1005: warning: Function
> >     parameter or member 'dev' not described in 'drm_sched_init'
> > 
> > Signed-off-by: Anup K Parikh <parikhanupk.foss@gmail.com>
> > ---
> >   drivers/gpu/drm/scheduler/sched_main.c | 1 +
> >   include/drm/gpu_scheduler.h            | 1 +
> >   2 files changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> > index 68317d3a7a27..875d00213849 100644
> > --- a/drivers/gpu/drm/scheduler/sched_main.c
> > +++ b/drivers/gpu/drm/scheduler/sched_main.c
> > @@ -994,6 +994,7 @@ static int drm_sched_main(void *param)
> >    *		used
> >    * @score: optional score atomic shared with other schedulers
> >    * @name: name used for debugging
> > + * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
> 
> 
> Why multiple GPUs scenario ? It's also used in single GPU scenario.
> 
> Andrey
> 
> 
Hello Mr. Andrey Grodzovsky,

Thanks for the quick review and response.

My documentation string (same for both files) is based on commit id
8ab62eda177bc350f34fea4fcea23603b8184bfd. It seemed that both warnings
might've been introduced by the addition of that device pointer.

Also, the commit message specifically mentions this addition for use with
DRM_DEV_ERROR() to make life easier under a multiple GPU scenario. So, I
used cscope to look for DRM_DEV_ERROR() and then for drm_dev_printk(). I
also checked previous versions of both files and noticed DRM_ERROR() in
drivers/gpu/drm/scheduler/sched_main.c changed to DRM_DEV_ERROR().

Perhaps, I wrongly correlated my cscope/history findings with the commit
message and used absolute wording. I guess that this might be (I usually
avoid absolute wording) useful not only in a single GPU scenario (to print
better standardized messages with dev when available) but also in non-error
printing such as with KERN_NOTICE, KERN_INFO, etc. I'm still not sure if
the added device pointer could be used for something else besides printing.

Please let me know if my understanding is correct and whether I should
change the wording to:

A device pointer - primarily useful for printing standardized messages with
DRM_DEV_ERROR().
> >    *
> >    * Return 0 on success, otherwise error code.
> >    */
> > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> > index addb135eeea6..920b91fd1719 100644
> > --- a/include/drm/gpu_scheduler.h
> > +++ b/include/drm/gpu_scheduler.h
> > @@ -435,6 +435,7 @@ struct drm_sched_backend_ops {
> >    * @_score: score used when the driver doesn't provide one
> >    * @ready: marks if the underlying HW is ready to work
> >    * @free_guilty: A hit to time out handler to free the guilty job.
> > + * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
> >    *
> >    * One scheduler is implemented for each hardware ring.
> >    */

Re: [PATCH] A simple doc fix

[Andrey Grodzovsky]

On 2022-09-14 15:26, Anup K Parikh wrote:
> On Wed, Sep 14, 2022 at 10:24:36AM -0400, Andrey Grodzovsky wrote:
>> On 2022-09-14 06:36, Anup K Parikh wrote:
>>> Fix two warnings during doc build which also results in corresponding
>>> additions in generated docs
>>>
>>> Warnings Fixed:
>>> 1. include/drm/gpu_scheduler.h:462: warning: Function parameter or member
>>>      'dev' not described in 'drm_gpu_scheduler'
>>> 2. drivers/gpu/drm/scheduler/sched_main.c:1005: warning: Function
>>>      parameter or member 'dev' not described in 'drm_sched_init'
>>>
>>> Signed-off-by: Anup K Parikh <parikhanupk.foss@gmail.com>
>>> ---
>>>    drivers/gpu/drm/scheduler/sched_main.c | 1 +
>>>    include/drm/gpu_scheduler.h            | 1 +
>>>    2 files changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>>> index 68317d3a7a27..875d00213849 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -994,6 +994,7 @@ static int drm_sched_main(void *param)
>>>     *		used
>>>     * @score: optional score atomic shared with other schedulers
>>>     * @name: name used for debugging
>>> + * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
>>
>> Why multiple GPUs scenario ? It's also used in single GPU scenario.
>>
>> Andrey
>>
>>
> Hello Mr. Andrey Grodzovsky,
>
> Thanks for the quick review and response.
>
> My documentation string (same for both files) is based on commit id
> 8ab62eda177bc350f34fea4fcea23603b8184bfd. It seemed that both warnings
> might've been introduced by the addition of that device pointer.
>
> Also, the commit message specifically mentions this addition for use with
> DRM_DEV_ERROR() to make life easier under a multiple GPU scenario. So, I
> used cscope to look for DRM_DEV_ERROR() and then for drm_dev_printk(). I
> also checked previous versions of both files and noticed DRM_ERROR() in
> drivers/gpu/drm/scheduler/sched_main.c changed to DRM_DEV_ERROR().
>
> Perhaps, I wrongly correlated my cscope/history findings with the commit
> message and used absolute wording. I guess that this might be (I usually
> avoid absolute wording) useful not only in a single GPU scenario (to print
> better standardized messages with dev when available) but also in non-error
> printing such as with KERN_NOTICE, KERN_INFO, etc. I'm still not sure if
> the added device pointer could be used for something else besides printing.
>
> Please let me know if my understanding is correct and whether I should
> change the wording to:
>
> A device pointer - primarily useful for printing standardized messages with
> DRM_DEV_ERROR().

It could be used for many other things but in this case in deed used 
only for the print.
So yep - looks good.

Andrey


>>>     *
>>>     * Return 0 on success, otherwise error code.
>>>     */
>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>>> index addb135eeea6..920b91fd1719 100644
>>> --- a/include/drm/gpu_scheduler.h
>>> +++ b/include/drm/gpu_scheduler.h
>>> @@ -435,6 +435,7 @@ struct drm_sched_backend_ops {
>>>     * @_score: score used when the driver doesn't provide one
>>>     * @ready: marks if the underlying HW is ready to work
>>>     * @free_guilty: A hit to time out handler to free the guilty job.
>>> + * @dev: A device pointer for use in error reporting in a multiple GPU scenario.
>>>     *
>>>     * One scheduler is implemented for each hardware ring.
>>>     */