Browsing the DRM documentation shows that drm_crtc_state.no_vblank
is not turned into a reference to the no_vblank field, but rather a
reference to `struct drm_crtc_state`. The only difference with other
field references is that the struct name is prefixed by the literal
`struct` tag, despite also already having a `&` reference prefix in two
of the three cases. Remove the `struct` prefix to turn these references
into proper links to the designated field.
Fixes: 7beb691f1e6f ("drm: Initialize struct drm_crtc_state.no_vblank from device settings")
Signed-off-by: Marijn Suijten <[email protected]>
---
Note that a simple regex like "&struct \w+\.\w+" shows that there are
only a handful of violators, most of them inside DRM files. Let me know
if you'd like a v2 that addresses all of them in one go (in separate
patches or one combined change)?
Kind regards,
Marijn
---
drivers/gpu/drm/drm_vblank.c | 2 +-
include/drm/drm_crtc.h | 2 +-
include/drm/drm_simple_kms_helper.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 702a12bc93bd..45504732f98e 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -140,7 +140,7 @@
* must not call drm_vblank_init(). For such drivers, atomic helpers will
* automatically generate fake vblank events as part of the display update.
* This functionality also can be controlled by the driver by enabling and
- * disabling struct drm_crtc_state.no_vblank.
+ * disabling &drm_crtc_state.no_vblank.
*/
/* Retry timestamp calculation up to 3 times to satisfy
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8b48a1974da3..eb75d0aec170 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -342,7 +342,7 @@ struct drm_crtc_state {
* that case.
*
* For very simple hardware without VBLANK interrupt, enabling
- * &struct drm_crtc_state.no_vblank makes DRM's atomic commit helpers
+ * &drm_crtc_state.no_vblank makes DRM's atomic commit helpers
* send a fake VBLANK event at the end of the display update after all
* hardware changes have been applied. See
* drm_atomic_helper_fake_vblank().
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index b2486d073763..6e64d91819e7 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -102,7 +102,7 @@ struct drm_simple_display_pipe_funcs {
* drm_crtc_arm_vblank_event(), when the driver supports vblank
* interrupt handling, or drm_crtc_send_vblank_event() for more
* complex case. In case the hardware lacks vblank support entirely,
- * drivers can set &struct drm_crtc_state.no_vblank in
+ * drivers can set &drm_crtc_state.no_vblank in
* &struct drm_simple_display_pipe_funcs.check and let DRM's
* atomic helper fake a vblank event.
*/
---
base-commit: 6bd343537461b57f3efe5dfc5fc193a232dfef1e
change-id: 20240416-drm-no_vblank-kdoc-link-fea1b53008a3
Best regards,
--
Marijn Suijten <[email protected]>
Hi,
On 4/16/24 12:29 PM, Marijn Suijten wrote:
> Browsing the DRM documentation shows that drm_crtc_state.no_vblank
> is not turned into a reference to the no_vblank field, but rather a
> reference to `struct drm_crtc_state`. The only difference with other
> field references is that the struct name is prefixed by the literal
> `struct` tag, despite also already having a `&` reference prefix in two
> of the three cases. Remove the `struct` prefix to turn these references
> into proper links to the designated field.
>
> Fixes: 7beb691f1e6f ("drm: Initialize struct drm_crtc_state.no_vblank from device settings")
> Signed-off-by: Marijn Suijten <[email protected]>
> ---
> Note that a simple regex like "&struct \w+\.\w+" shows that there are
> only a handful of violators, most of them inside DRM files. Let me know
> if you'd like a v2 that addresses all of them in one go (in separate
> patches or one combined change)?
>
> Kind regards,
> Marijn
> ---
> drivers/gpu/drm/drm_vblank.c | 2 +-
> include/drm/drm_crtc.h | 2 +-
> include/drm/drm_simple_kms_helper.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 702a12bc93bd..45504732f98e 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -140,7 +140,7 @@
> * must not call drm_vblank_init(). For such drivers, atomic helpers will
> * automatically generate fake vblank events as part of the display update.
> * This functionality also can be controlled by the driver by enabling and
> - * disabling struct drm_crtc_state.no_vblank.
> + * disabling &drm_crtc_state.no_vblank.
> */
>
> /* Retry timestamp calculation up to 3 times to satisfy
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8b48a1974da3..eb75d0aec170 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -342,7 +342,7 @@ struct drm_crtc_state {
> * that case.
> *
> * For very simple hardware without VBLANK interrupt, enabling
> - * &struct drm_crtc_state.no_vblank makes DRM's atomic commit helpers
> + * &drm_crtc_state.no_vblank makes DRM's atomic commit helpers
> * send a fake VBLANK event at the end of the display update after all
> * hardware changes have been applied. See
> * drm_atomic_helper_fake_vblank().
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index b2486d073763..6e64d91819e7 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -102,7 +102,7 @@ struct drm_simple_display_pipe_funcs {
> * drm_crtc_arm_vblank_event(), when the driver supports vblank
> * interrupt handling, or drm_crtc_send_vblank_event() for more
> * complex case. In case the hardware lacks vblank support entirely,
> - * drivers can set &struct drm_crtc_state.no_vblank in
> + * drivers can set &drm_crtc_state.no_vblank in
> * &struct drm_simple_display_pipe_funcs.check and let DRM's
> * atomic helper fake a vblank event.
> */
>
> ---
> base-commit: 6bd343537461b57f3efe5dfc5fc193a232dfef1e
> change-id: 20240416-drm-no_vblank-kdoc-link-fea1b53008a3
Do you see differences in the generated html for these changes?
"&struct somestruct" and "&somestruct" should both be OK AFAIK, although
Documentation/doc-guide/kernel-doc.rst seems to say that using
"&struct somestruct" is preferred.
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
Hi Randy,
[..]
> Do you see differences in the generated html for these changes?
I have not yet generated the HTML locally to test this patch, but will surely do
if that's a requirement.
> "&struct somestruct" and "&somestruct" should both be OK AFAIK, although
> Documentation/doc-guide/kernel-doc.rst seems to say that using
> "&struct somestruct" is preferred.
Keep in mind that this patch is about field/member references. Quoting the
relevant paragraph under "Highlights and cross-references":
``&struct_name->member`` or ``&struct_name.member``
Structure or union member reference. The cross-reference will be to the struct
or union definition, not the member directly.
This lacks the struct tag entirely, and observation shows that links with them
don't highlight correctly (hence this patch) while member links without struct
tag are clickable and have an anchor link to their parent struct.
- Marijn
On 4/16/24 2:00 PM, Marijn Suijten wrote:
> Hi Randy,
>
> [..]
>
>> Do you see differences in the generated html for these changes?
>
> I have not yet generated the HTML locally to test this patch, but will surely do
> if that's a requirement.
>
>> "&struct somestruct" and "&somestruct" should both be OK AFAIK, although
>> Documentation/doc-guide/kernel-doc.rst seems to say that using
>> "&struct somestruct" is preferred.
>
> Keep in mind that this patch is about field/member references. Quoting the
> relevant paragraph under "Highlights and cross-references":
>
> ``&struct_name->member`` or ``&struct_name.member``
> Structure or union member reference. The cross-reference will be to the struct
> or union definition, not the member directly.
>
> This lacks the struct tag entirely, and observation shows that links with them
> don't highlight correctly (hence this patch) while member links without struct
> tag are clickable and have an anchor link to their parent struct.
Alrigthy. Thank you.
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
On 4/16/24 2:02 PM, Randy Dunlap wrote:
>
>
> On 4/16/24 2:00 PM, Marijn Suijten wrote:
>> Hi Randy,
>>
>> [..]
>>
>>> Do you see differences in the generated html for these changes?
>>
>> I have not yet generated the HTML locally to test this patch, but will surely do
>> if that's a requirement.
>>
>>> "&struct somestruct" and "&somestruct" should both be OK AFAIK, although
>>> Documentation/doc-guide/kernel-doc.rst seems to say that using
>>> "&struct somestruct" is preferred.
>>
>> Keep in mind that this patch is about field/member references. Quoting the
>> relevant paragraph under "Highlights and cross-references":
>>
>> ``&struct_name->member`` or ``&struct_name.member``
>> Structure or union member reference. The cross-reference will be to the struct
>> or union definition, not the member directly.
>>
>> This lacks the struct tag entirely, and observation shows that links with them
>> don't highlight correctly (hence this patch) while member links without struct
>> tag are clickable and have an anchor link to their parent struct.
>
> Alrigthy. Thank you.
>
That means:
Reviewed-by: Randy Dunlap <[email protected]>
Thanks.
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html