Re: [PATCH 1/2] drm/doc: document how userspace should find out CRTC index

[Leandro Ribeiro <leandro.ribeiro@collabora.com>]

On 5/12/21 10:04 AM, Pekka Paalanen wrote:
> On Wed, 12 May 2021 09:50:14 -0300
> Leandro Ribeiro <leandro.ribeiro@collabora.com> wrote:
> 
>> On 5/6/21 5:50 AM, Pekka Paalanen wrote:
>>> On Wed, 28 Apr 2021 18:36:50 -0300
>>> Leandro Ribeiro <leandro.ribeiro@collabora.com> wrote:
>>>   
>>>> In this patch we add a section to document what userspace should do to
>>>> find out the CRTC index. This is important as there are multiple places
>>>> in the documentation that need this, so it's better to just point to
>>>> this section and avoid repetition.
>>>>
>>>> Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
>>>> ---
>>>>  Documentation/gpu/drm-uapi.rst    | 14 ++++++++++++++
>>>>  drivers/gpu/drm/drm_debugfs_crc.c |  9 +++++----
>>>>  include/uapi/drm/drm.h            |  3 ++-
>>>>  3 files changed, 21 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
>>>> index 04bdc7a91d53..1aa52a6ac567 100644
>>>> --- a/Documentation/gpu/drm-uapi.rst
>>>> +++ b/Documentation/gpu/drm-uapi.rst
>>>> @@ -457,6 +457,20 @@ Userspace API Structures
>>>>  .. kernel-doc:: include/uapi/drm/drm_mode.h
>>>>     :doc: overview
>>>>
>>>> +.. _crtc_index:
>>>> +
>>>> +CRTC index
>>>> +----------
>>>> +
>>>> +In some situations, it is important for userspace to find out the index of a  
>>>
>>> That could be said about everything, so this sentence has no
>>> information value. Instead, you could start by stating that CRTCs have
>>> both an object ID and an index, and they are not the same thing. CRTC
>>> index is used in cases where a densely packed identifier for a CRTC is
>>> needed, e.g. in bit-for-crtc masks, where using the object ID would not
>>> work.
>>>  
>>>> +CRTC. The CRTC index should not be confused with its object id.
>>>> +
>>>> +In order to do this, userspace should first query the resources object  
>>>
>>> Instead of saying what userspace must do, you could just explain where
>>> it can be observed.
>>>   
>>>> +from the device that owns the CRTC (using the DRM_IOCTL_MODE_GETRESOURCES  
>>>
>>> So here you might start with: DRM_IOCTL_MODE_GETRESOURCES populates a
>>> structure with an array of CRTC IDs. CRTC's index is its index in that
>>> array.
>>>   
>>>> +ioctl). The resources object contains a pointer to an array of CRTC's, and also
>>>> +the number of entries of the array. The index of the CRTC is the same as its
>>>> +position in this array.  
>>>
>>> Anyway, the idea here is right.
>>>   
>>
>> So what about:
>>
>> CRTC's have both an object ID and an index, and they should not be
>> confused. The index is used in cases where a densely packed identifier
>> for a CRTC is needed, for instance in a bitmask of CRTC's. (maybe a link
>> to the possible_crtcs field of struct drm_mode_get_plane? as example)
>>
>> DRM_IOCTL_MODE_GETRESOURCES populates a structure with an array of CRTC
>> id's, and the CRTC index is its position in this array.
> 
> Sure, sounds good.
> 
> Capitalized 'ID'?
> 

Sure, I'll add this change.

Thanks,
Leandro