* [PATCH] drm: improve the documentation of connector hpd ops
@ 2023-09-20 20:13 Abhinav Kumar
2023-12-03 14:24 ` Laurent Pinchart
2023-12-04 14:16 ` Dmitry Baryshkov
0 siblings, 2 replies; 5+ messages in thread
From: Abhinav Kumar @ 2023-09-20 20:13 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter
Cc: Abhinav Kumar, freedreno, quic_jesszhan, quic_parellan,
laurent.pinchart, andersson, jani.nikula, linux-kernel
While making the changes in [1], it was noted that the documentation
of the enable_hpd() and disable_hpd() does not make it clear that
these ops should not try to do hpd state maintenance and should only
attempt to enable/disable hpd related hardware for the connector.
The state management of these calls to make sure these calls are
balanced is handled by the DRM core and we should keep it that way
to minimize the overhead in the drivers which implement these ops.
[1]: https://patchwork.freedesktop.org/patch/558387/
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
include/drm/drm_modeset_helper_vtables.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index e3c3ac615909..a33cf7488737 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -1154,6 +1154,11 @@ struct drm_connector_helper_funcs {
* This operation is optional.
*
* This callback is used by the drm_kms_helper_poll_enable() helpers.
+ *
+ * This operation does not need to perform any hpd state tracking as
+ * the DRM core handles that maintenance and ensures the calls to enable
+ * and disable hpd are balanced.
+ *
*/
void (*enable_hpd)(struct drm_connector *connector);
@@ -1165,6 +1170,11 @@ struct drm_connector_helper_funcs {
* This operation is optional.
*
* This callback is used by the drm_kms_helper_poll_disable() helpers.
+ *
+ * This operation does not need to perform any hpd state tracking as
+ * the DRM core handles that maintenance and ensures the calls to enable
+ * and disable hpd are balanced.
+ *
*/
void (*disable_hpd)(struct drm_connector *connector);
};
--
2.40.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm: improve the documentation of connector hpd ops
2023-09-20 20:13 [PATCH] drm: improve the documentation of connector hpd ops Abhinav Kumar
@ 2023-12-03 14:24 ` Laurent Pinchart
2023-12-03 18:14 ` [Freedreno] " Dmitry Baryshkov
2023-12-04 14:16 ` Dmitry Baryshkov
1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2023-12-03 14:24 UTC (permalink / raw)
To: Abhinav Kumar
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, freedreno, quic_jesszhan,
quic_parellan, andersson, jani.nikula, linux-kernel
Hi Abhinav,
Thank you for the patch (and thank to Dmitry for pinging me on IRC, this
patch got burried in my inbox).
On Wed, Sep 20, 2023 at 01:13:58PM -0700, Abhinav Kumar wrote:
> While making the changes in [1], it was noted that the documentation
> of the enable_hpd() and disable_hpd() does not make it clear that
> these ops should not try to do hpd state maintenance and should only
> attempt to enable/disable hpd related hardware for the connector.
s/attempt to //
>
> The state management of these calls to make sure these calls are
> balanced is handled by the DRM core and we should keep it that way
> to minimize the overhead in the drivers which implement these ops.
>
> [1]: https://patchwork.freedesktop.org/patch/558387/
>
You could add a
Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> include/drm/drm_modeset_helper_vtables.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index e3c3ac615909..a33cf7488737 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -1154,6 +1154,11 @@ struct drm_connector_helper_funcs {
> * This operation is optional.
> *
> * This callback is used by the drm_kms_helper_poll_enable() helpers.
> + *
> + * This operation does not need to perform any hpd state tracking as
> + * the DRM core handles that maintenance and ensures the calls to enable
> + * and disable hpd are balanced.
> + *
> */
> void (*enable_hpd)(struct drm_connector *connector);
>
> @@ -1165,6 +1170,11 @@ struct drm_connector_helper_funcs {
> * This operation is optional.
> *
> * This callback is used by the drm_kms_helper_poll_disable() helpers.
> + *
> + * This operation does not need to perform any hpd state tracking as
> + * the DRM core handles that maintenance and ensures the calls to enable
> + * and disable hpd are balanced.
> + *
> */
> void (*disable_hpd)(struct drm_connector *connector);
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Freedreno] [PATCH] drm: improve the documentation of connector hpd ops
2023-12-03 14:24 ` Laurent Pinchart
@ 2023-12-03 18:14 ` Dmitry Baryshkov
2023-12-04 17:45 ` Abhinav Kumar
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Baryshkov @ 2023-12-03 18:14 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Abhinav Kumar, jani.nikula, Thomas Zimmermann, freedreno,
andersson, Maarten Lankhorst, linux-kernel, Maxime Ripard,
quic_parellan, dri-devel, Daniel Vetter, quic_jesszhan,
David Airlie
On Sun, 3 Dec 2023 at 16:24, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Abhinav,
>
> Thank you for the patch (and thank to Dmitry for pinging me on IRC, this
> patch got burried in my inbox).
>
> On Wed, Sep 20, 2023 at 01:13:58PM -0700, Abhinav Kumar wrote:
> > While making the changes in [1], it was noted that the documentation
> > of the enable_hpd() and disable_hpd() does not make it clear that
> > these ops should not try to do hpd state maintenance and should only
> > attempt to enable/disable hpd related hardware for the connector.
>
> s/attempt to //
I can probably fix this while applying the patch.
>
> >
> > The state management of these calls to make sure these calls are
> > balanced is handled by the DRM core and we should keep it that way
> > to minimize the overhead in the drivers which implement these ops.
> >
> > [1]: https://patchwork.freedesktop.org/patch/558387/
> >
>
> You could add a
>
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > ---
> > include/drm/drm_modeset_helper_vtables.h | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> > index e3c3ac615909..a33cf7488737 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -1154,6 +1154,11 @@ struct drm_connector_helper_funcs {
> > * This operation is optional.
> > *
> > * This callback is used by the drm_kms_helper_poll_enable() helpers.
> > + *
> > + * This operation does not need to perform any hpd state tracking as
> > + * the DRM core handles that maintenance and ensures the calls to enable
> > + * and disable hpd are balanced.
> > + *
> > */
> > void (*enable_hpd)(struct drm_connector *connector);
> >
> > @@ -1165,6 +1170,11 @@ struct drm_connector_helper_funcs {
> > * This operation is optional.
> > *
> > * This callback is used by the drm_kms_helper_poll_disable() helpers.
> > + *
> > + * This operation does not need to perform any hpd state tracking as
> > + * the DRM core handles that maintenance and ensures the calls to enable
> > + * and disable hpd are balanced.
> > + *
> > */
> > void (*disable_hpd)(struct drm_connector *connector);
> > };
>
> --
> Regards,
>
> Laurent Pinchart
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Freedreno] [PATCH] drm: improve the documentation of connector hpd ops
2023-12-03 18:14 ` [Freedreno] " Dmitry Baryshkov
@ 2023-12-04 17:45 ` Abhinav Kumar
0 siblings, 0 replies; 5+ messages in thread
From: Abhinav Kumar @ 2023-12-04 17:45 UTC (permalink / raw)
To: Dmitry Baryshkov, Laurent Pinchart
Cc: jani.nikula, Thomas Zimmermann, freedreno, andersson,
Maarten Lankhorst, linux-kernel, Maxime Ripard, quic_parellan,
dri-devel, Daniel Vetter, quic_jesszhan, David Airlie
On 12/3/2023 10:14 AM, Dmitry Baryshkov wrote:
> On Sun, 3 Dec 2023 at 16:24, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Hi Abhinav,
>>
>> Thank you for the patch (and thank to Dmitry for pinging me on IRC, this
>> patch got burried in my inbox).
>>
>> On Wed, Sep 20, 2023 at 01:13:58PM -0700, Abhinav Kumar wrote:
>>> While making the changes in [1], it was noted that the documentation
>>> of the enable_hpd() and disable_hpd() does not make it clear that
>>> these ops should not try to do hpd state maintenance and should only
>>> attempt to enable/disable hpd related hardware for the connector.
>>
>> s/attempt to //
>
> I can probably fix this while applying the patch.
>
Thank you Laurent and Dmitry.
>>
>>>
>>> The state management of these calls to make sure these calls are
>>> balanced is handled by the DRM core and we should keep it that way
>>> to minimize the overhead in the drivers which implement these ops.
>>>
>>> [1]: https://patchwork.freedesktop.org/patch/558387/
>>>
>>
>> You could add a
>>
>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>>> ---
>>> include/drm/drm_modeset_helper_vtables.h | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
>>> index e3c3ac615909..a33cf7488737 100644
>>> --- a/include/drm/drm_modeset_helper_vtables.h
>>> +++ b/include/drm/drm_modeset_helper_vtables.h
>>> @@ -1154,6 +1154,11 @@ struct drm_connector_helper_funcs {
>>> * This operation is optional.
>>> *
>>> * This callback is used by the drm_kms_helper_poll_enable() helpers.
>>> + *
>>> + * This operation does not need to perform any hpd state tracking as
>>> + * the DRM core handles that maintenance and ensures the calls to enable
>>> + * and disable hpd are balanced.
>>> + *
>>> */
>>> void (*enable_hpd)(struct drm_connector *connector);
>>>
>>> @@ -1165,6 +1170,11 @@ struct drm_connector_helper_funcs {
>>> * This operation is optional.
>>> *
>>> * This callback is used by the drm_kms_helper_poll_disable() helpers.
>>> + *
>>> + * This operation does not need to perform any hpd state tracking as
>>> + * the DRM core handles that maintenance and ensures the calls to enable
>>> + * and disable hpd are balanced.
>>> + *
>>> */
>>> void (*disable_hpd)(struct drm_connector *connector);
>>> };
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: improve the documentation of connector hpd ops
2023-09-20 20:13 [PATCH] drm: improve the documentation of connector hpd ops Abhinav Kumar
2023-12-03 14:24 ` Laurent Pinchart
@ 2023-12-04 14:16 ` Dmitry Baryshkov
1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Baryshkov @ 2023-12-04 14:16 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Abhinav Kumar
Cc: freedreno, quic_jesszhan, quic_parellan, laurent.pinchart,
andersson, jani.nikula, linux-kernel
On Wed, 20 Sep 2023 13:13:58 -0700, Abhinav Kumar wrote:
> While making the changes in [1], it was noted that the documentation
> of the enable_hpd() and disable_hpd() does not make it clear that
> these ops should not try to do hpd state maintenance and should only
> attempt to enable/disable hpd related hardware for the connector.
>
> The state management of these calls to make sure these calls are
> balanced is handled by the DRM core and we should keep it that way
> to minimize the overhead in the drivers which implement these ops.
>
> [...]
Applied to drm-misc-next, thanks!
[1/1] drm: improve the documentation of connector hpd ops
commit: a5b2dcb96d6acb286459612a142371b0d74543bf
Best regards,
--
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-04 17:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-20 20:13 [PATCH] drm: improve the documentation of connector hpd ops Abhinav Kumar
2023-12-03 14:24 ` Laurent Pinchart
2023-12-03 18:14 ` [Freedreno] " Dmitry Baryshkov
2023-12-04 17:45 ` Abhinav Kumar
2023-12-04 14:16 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®