diff mbox series

[v2] drm/i915: Add missing docbook chapters for i915 uapi.

Message ID 20210715120842.806605-1-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: Add missing docbook chapters for i915 uapi. | expand

Commit Message

Maarten Lankhorst July 15, 2021, 12:08 p.m. UTC
I noticed when grepping for DOC: that those were defined
in the header, but not actually used. Fix it by removing
all chapters and the internal annotation, so the docbook
generated chapters are used.

Changes since v1:
- Just remove the chapters, no need for those.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210709113842.651140-1-maarten.lankhorst@linux.intel.com
---
 Documentation/gpu/driver-uapi.rst | 21 ---------------------
 1 file changed, 21 deletions(-)

Comments

Matthew Auld July 15, 2021, 12:19 p.m. UTC | #1
On Thu, 15 Jul 2021 at 13:08, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>
> I noticed when grepping for DOC: that those were defined
> in the header, but not actually used. Fix it by removing
> all chapters and the internal annotation, so the docbook
> generated chapters are used.
>
> Changes since v1:
> - Just remove the chapters, no need for those.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210709113842.651140-1-maarten.lankhorst@linux.intel.com

Indeed, it seems to still render correctly here.

Maybe update the commit title when pushing,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> ---
>  Documentation/gpu/driver-uapi.rst | 21 ---------------------
>  1 file changed, 21 deletions(-)
>
> diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
> index 27d0fbe33e87..4411e6919a3d 100644
> --- a/Documentation/gpu/driver-uapi.rst
> +++ b/Documentation/gpu/driver-uapi.rst
> @@ -5,25 +5,4 @@ DRM Driver uAPI
>  drm/i915 uAPI
>  =============
>
> -Engine Discovery uAPI
> ----------------------
> -
> -.. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :doc: Engine Discovery uAPI
> -
> -Context Engine Map uAPI
> ------------------------
> -
> -.. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :doc: Context Engine Map uAPI
> -
> -Virtual Engine uAPI
> --------------------
> -
> -.. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :doc: Virtual Engine uAPI
> -
> -i915_drm.h
> -----------
>  .. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :internal:
> --
> 2.31.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst July 16, 2021, 9:34 a.m. UTC | #2
Op 15-07-2021 om 14:19 schreef Matthew Auld:
> On Thu, 15 Jul 2021 at 13:08, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> I noticed when grepping for DOC: that those were defined
>> in the header, but not actually used. Fix it by removing
>> all chapters and the internal annotation, so the docbook
>> generated chapters are used.
>>
>> Changes since v1:
>> - Just remove the chapters, no need for those.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Link: https://patchwork.freedesktop.org/patch/msgid/20210709113842.651140-1-maarten.lankhorst@linux.intel.com
> Indeed, it seems to still render correctly here.
>
> Maybe update the commit title when pushing,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
>> ---
>>  Documentation/gpu/driver-uapi.rst | 21 ---------------------
>>  1 file changed, 21 deletions(-)
>>
>> diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
>> index 27d0fbe33e87..4411e6919a3d 100644
>> --- a/Documentation/gpu/driver-uapi.rst
>> +++ b/Documentation/gpu/driver-uapi.rst
>> @@ -5,25 +5,4 @@ DRM Driver uAPI
>>  drm/i915 uAPI
>>  =============
>>
>> -Engine Discovery uAPI
>> ----------------------
>> -
>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :doc: Engine Discovery uAPI
>> -
>> -Context Engine Map uAPI
>> ------------------------
>> -
>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :doc: Context Engine Map uAPI
>> -
>> -Virtual Engine uAPI
>> --------------------
>> -
>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :doc: Virtual Engine uAPI
>> -
>> -i915_drm.h
>> -----------
>>  .. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :internal:
>> --
>> 2.31.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Thanks, pushed!
Tvrtko Ursulin July 16, 2021, 1:07 p.m. UTC | #3
On 15/07/2021 13:08, Maarten Lankhorst wrote:
> I noticed when grepping for DOC: that those were defined
> in the header, but not actually used. Fix it by removing
> all chapters and the internal annotation, so the docbook
> generated chapters are used.
> 
> Changes since v1:
> - Just remove the chapters, no need for those.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210709113842.651140-1-maarten.lankhorst@linux.intel.com
> ---
>   Documentation/gpu/driver-uapi.rst | 21 ---------------------
>   1 file changed, 21 deletions(-)
> 
> diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
> index 27d0fbe33e87..4411e6919a3d 100644
> --- a/Documentation/gpu/driver-uapi.rst
> +++ b/Documentation/gpu/driver-uapi.rst
> @@ -5,25 +5,4 @@ DRM Driver uAPI
>   drm/i915 uAPI
>   =============
>   
> -Engine Discovery uAPI
> ----------------------
> -
> -.. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :doc: Engine Discovery uAPI
> -
> -Context Engine Map uAPI
> ------------------------
> -
> -.. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :doc: Context Engine Map uAPI
> -
> -Virtual Engine uAPI
> --------------------
> -
> -.. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :doc: Virtual Engine uAPI
> -

Hmm my idea was to have the above three laid out sequentially in the doc 
so the narrative makes sense. Otherwise they are at random locations in 
i915_drm.h so reader cannot get the nice story around high level 
operation and interactions between then. Initially I had this narrative 
right in this file but Daniel wanted it moved into i915_drm.h.

I didn't really understand the commit message to understand what wasn't 
right?

> -i915_drm.h
> -----------
>   .. kernel-doc:: include/uapi/drm/i915_drm.h
> -   :internal:

This one I added to avoid duplicate sections. It won't be a problem if 
they are not referenced from driver-uapi.rst though.

Regards,

Tvrtko
Maarten Lankhorst July 20, 2021, 9:06 a.m. UTC | #4
Op 16-07-2021 om 15:07 schreef Tvrtko Ursulin:
>
> On 15/07/2021 13:08, Maarten Lankhorst wrote:
>> I noticed when grepping for DOC: that those were defined
>> in the header, but not actually used. Fix it by removing
>> all chapters and the internal annotation, so the docbook
>> generated chapters are used.
>>
>> Changes since v1:
>> - Just remove the chapters, no need for those.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Link: https://patchwork.freedesktop.org/patch/msgid/20210709113842.651140-1-maarten.lankhorst@linux.intel.com
>> ---
>>   Documentation/gpu/driver-uapi.rst | 21 ---------------------
>>   1 file changed, 21 deletions(-)
>>
>> diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
>> index 27d0fbe33e87..4411e6919a3d 100644
>> --- a/Documentation/gpu/driver-uapi.rst
>> +++ b/Documentation/gpu/driver-uapi.rst
>> @@ -5,25 +5,4 @@ DRM Driver uAPI
>>   drm/i915 uAPI
>>   =============
>>   -Engine Discovery uAPI
>> ----------------------
>> -
>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :doc: Engine Discovery uAPI
>> -
>> -Context Engine Map uAPI
>> ------------------------
>> -
>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :doc: Context Engine Map uAPI
>> -
>> -Virtual Engine uAPI
>> --------------------
>> -
>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :doc: Virtual Engine uAPI
>> -
>
> Hmm my idea was to have the above three laid out sequentially in the doc so the narrative makes sense. Otherwise they are at random locations in i915_drm.h so reader cannot get the nice story around high level operation and interactions between then. Initially I had this narrative right in this file but Daniel wanted it moved into i915_drm.h.
>
> I didn't really understand the commit message to understand what wasn't right?

Some chapters went missing, and instead of defining the same chapters in multiple places, I felt we could just revert the change and get it right.

include/uapi/drm/i915_drm.h: * DOC: uevents generated by i915 on it's device node
include/uapi/drm/i915_drm.h: * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
include/uapi/drm/i915_drm.h: * DOC: Virtual Engine uAPI
include/uapi/drm/i915_drm.h: * DOC: Context Engine Map uAPI
include/uapi/drm/i915_drm.h: * DOC: Engine Discovery uAPI

You added the last 3, but forgot the first 2. Since removing worked, I felt we could just leave it at that. :)

No need to define twice.

>> -i915_drm.h
>> -----------
>>   .. kernel-doc:: include/uapi/drm/i915_drm.h
>> -   :internal:
>
> This one I added to avoid duplicate sections. It won't be a problem if they are not referenced from driver-uapi.rst though.
Tvrtko Ursulin July 20, 2021, 9:39 a.m. UTC | #5
On 20/07/2021 10:06, Maarten Lankhorst wrote:
> Op 16-07-2021 om 15:07 schreef Tvrtko Ursulin:
>>
>> On 15/07/2021 13:08, Maarten Lankhorst wrote:
>>> I noticed when grepping for DOC: that those were defined
>>> in the header, but not actually used. Fix it by removing
>>> all chapters and the internal annotation, so the docbook
>>> generated chapters are used.
>>>
>>> Changes since v1:
>>> - Just remove the chapters, no need for those.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Link: https://patchwork.freedesktop.org/patch/msgid/20210709113842.651140-1-maarten.lankhorst@linux.intel.com
>>> ---
>>>    Documentation/gpu/driver-uapi.rst | 21 ---------------------
>>>    1 file changed, 21 deletions(-)
>>>
>>> diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
>>> index 27d0fbe33e87..4411e6919a3d 100644
>>> --- a/Documentation/gpu/driver-uapi.rst
>>> +++ b/Documentation/gpu/driver-uapi.rst
>>> @@ -5,25 +5,4 @@ DRM Driver uAPI
>>>    drm/i915 uAPI
>>>    =============
>>>    -Engine Discovery uAPI
>>> ----------------------
>>> -
>>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>>> -   :doc: Engine Discovery uAPI
>>> -
>>> -Context Engine Map uAPI
>>> ------------------------
>>> -
>>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>>> -   :doc: Context Engine Map uAPI
>>> -
>>> -Virtual Engine uAPI
>>> --------------------
>>> -
>>> -.. kernel-doc:: include/uapi/drm/i915_drm.h
>>> -   :doc: Virtual Engine uAPI
>>> -
>>
>> Hmm my idea was to have the above three laid out sequentially in the doc so the narrative makes sense. Otherwise they are at random locations in i915_drm.h so reader cannot get the nice story around high level operation and interactions between then. Initially I had this narrative right in this file but Daniel wanted it moved into i915_drm.h.
>>
>> I didn't really understand the commit message to understand what wasn't right?
> 
> Some chapters went missing, and instead of defining the same chapters in multiple places, I felt we could just revert the change and get it right.
> 
> include/uapi/drm/i915_drm.h: * DOC: uevents generated by i915 on it's device node
> include/uapi/drm/i915_drm.h: * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
> include/uapi/drm/i915_drm.h: * DOC: Virtual Engine uAPI
> include/uapi/drm/i915_drm.h: * DOC: Context Engine Map uAPI
> include/uapi/drm/i915_drm.h: * DOC: Engine Discovery uAPI
> 
> You added the last 3, but forgot the first 2. Since removing worked, I felt we could just leave it at that. :)

Hmm.. I thought the ":internal:" keyword will avoid duplicating chapters 
which were externally referenced but it seems it's not what it does.

No idea then how to do what I want. Docs are a bit hard to find for some 
of this. Oh well..

I could pull my chapters back out but Daniel explicitly wanted them in 
i915_drm.h so I don't see a way to reconcile both mine and his wishes 
here. So it can stay a bit of a mess.

Regards,

Tvrtko

> No need to define twice.
> 
>>> -i915_drm.h
>>> -----------
>>>    .. kernel-doc:: include/uapi/drm/i915_drm.h
>>> -   :internal:
>>
>> This one I added to avoid duplicate sections. It won't be a problem if they are not referenced from driver-uapi.rst though.
>
diff mbox series

Patch

diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
index 27d0fbe33e87..4411e6919a3d 100644
--- a/Documentation/gpu/driver-uapi.rst
+++ b/Documentation/gpu/driver-uapi.rst
@@ -5,25 +5,4 @@  DRM Driver uAPI
 drm/i915 uAPI
 =============
 
-Engine Discovery uAPI
----------------------
-
-.. kernel-doc:: include/uapi/drm/i915_drm.h
-   :doc: Engine Discovery uAPI
-
-Context Engine Map uAPI
------------------------
-
-.. kernel-doc:: include/uapi/drm/i915_drm.h
-   :doc: Context Engine Map uAPI
-
-Virtual Engine uAPI
--------------------
-
-.. kernel-doc:: include/uapi/drm/i915_drm.h
-   :doc: Virtual Engine uAPI
-
-i915_drm.h
-----------
 .. kernel-doc:: include/uapi/drm/i915_drm.h
-   :internal: