| Message ID | 20240416-drm-no_vblank-kdoc-link-v1-1-a1d8d1e9ff34@somainline.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Fix no_vblank field references in documentation | expand |
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 <marijn.suijten@somainline.org> > --- > 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.
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.
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 <rdunlap@infradead.org> Thanks.
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. */
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 <marijn.suijten@somainline.org> --- 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(-) --- base-commit: 6bd343537461b57f3efe5dfc5fc193a232dfef1e change-id: 20240416-drm-no_vblank-kdoc-link-fea1b53008a3 Best regards,