Re: [Intel-gfx] [PATCH v2] drm/i915: Add missing docbook chapters for i915 uapi.

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx