Re: [PATCHv2] drm/xe/display: Change HPD to polling on runtime suspend

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 08 Aug 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> HPD is interrupt based and on runtime suspend change it to polling as
> HPD is not a wakeup event. A worker thread is scheduled for doing this
> polling and it keeps polling for HPD live status on an internval of 10s.
> On runtime resume disable polling and fallback to interrupt mechanism.
>
> v2: move poll_enable() to xe_display(Imre)
>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 78cccbe28947..989022d21052 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -315,6 +315,10 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>  
>  	intel_hpd_cancel_work(xe);
>  
> +	/* Enable hpd polling on runtime suspend */

What information does this comment add that is not obvious from the
code?

I'm not against helpful comments, but they need to be something more
than just explaining the same code in English.

BR,
Jani.


> +	if (runtime)
> +		intel_hpd_poll_enable(xe);
> +
>  	intel_suspend_encoders(xe);
>  
>  	intel_opregion_suspend(xe, s2idle ? PCI_D1 : PCI_D3cold);

-- 
Jani Nikula, Intel