On 5/6/21 5:50 AM, Pekka Paalanen wrote:
> On Wed, 28 Apr 2021 18:36:50 -0300
> Leandro Ribeiro <leandro.ribeiro@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> ---
>> 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.
>> +
>> .. kernel-doc:: include/uapi/drm/drm.h
>> :internal:
>>
>> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
>> index 3dd70d813f69..9575188d97ee 100644
>> --- a/drivers/gpu/drm/drm_debugfs_crc.c
>> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
>> @@ -46,10 +46,11 @@
>> * it reached a given hardware component (a CRC sampling "source").
>> *
>> * Userspace can control generation of CRCs in a given CRTC by writing to the
>> - * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the CRTC.
>> - * Accepted values are source names (which are driver-specific) and the "auto"
>> - * keyword, which will let the driver select a default source of frame CRCs
>> - * for this CRTC.
>> + * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the
>> + * CRTC. To learn how to find out the index of a certain CRTC, please see
>> + * :ref:`crtc_index`. Accepted values are source names (which are
>
> This a bit verbose: "To learn..." It could be more concise, like
> making the words "the index" a hyperlink instead of adding a whole
> sentence.
>
Nice, I'll apply this change.
>> + * driver-specific) and the "auto" keyword, which will let the driver select a
>> + * default source of frame CRCs for this CRTC.
>> *
>> * Once frame CRC generation is enabled, userspace can capture them by reading
>> * the dri/0/crtc-N/crc/data file. Each line in that file contains the frame
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 67b94bc3c885..6944f08ab1a6 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -636,7 +636,8 @@ struct drm_gem_open {
>> * DRM_CAP_VBLANK_HIGH_CRTC
>> *
>> * If set to 1, the kernel supports specifying a CRTC index in the high bits of
>> - * &drm_wait_vblank_request.type.
>> + * &drm_wait_vblank_request.type. To learn how to find out the index of a
>> + * certain CRTC, please see :ref:`crtc_index`.
>
> The same here with "a CRTC index" turned into a hyperlink.
>
I'll change this as well.
Thanks,
Leandro
>
> Thanks,
> pq
>
>> *
>> * Starting kernel version 2.6.39, this capability is always set to 1.
>> */
>> --
>> 2.31.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
