Re: [PATCH 5/6] drm/i915/display: Use display parent interface for i915 runtime pm

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

On Wed, 22 Oct 2025, Jouni Högander <jouni.hogander@intel.com> wrote:
> Start using  display parent interface for i915 runtime pm. Doing the same
> for xe is done in coming changes.
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_display_rpm.c  | 43 +++++++++++--------
>  1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_rpm.c b/drivers/gpu/drm/i915/display/intel_display_rpm.c
> index 56c4024201c16..622646814e0bf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_rpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_rpm.c
> @@ -4,66 +4,71 @@
>  #include "i915_drv.h"
>  #include "intel_display_core.h"
>  #include "intel_display_rpm.h"
> -#include "intel_runtime_pm.h"
> -
> -static struct intel_runtime_pm *display_to_rpm(struct intel_display *display)
> -{
> -	struct drm_i915_private *i915 = to_i915(display->drm);
> -
> -	return &i915->runtime_pm;
> -}
> +#include "drm/intel/display_parent_interface.h"

This should use <> for include, as it's in include/drm, and placed
appropriately in the include list.

>  
>  struct ref_tracker *intel_display_rpm_get_raw(struct intel_display *display)
>  {
> -	return intel_runtime_pm_get_raw(display_to_rpm(display));
> +	return display->parent->rpm && display->parent->rpm->get_raw ?
> +		display->parent->rpm->get_raw(display->drm) : NULL;

I think we should require and assume ->rpm is set, and also all the
function pointers are set. It should be a rare documented exception if
something can be NULL.

I'm a bit divided about keeping the wrappers for calling the
interface. If we assume the pointers are valid, we could switch to
calling e.g.

        - wakeref = intel_display_rpm_get_raw();
	+ wakeref = display->parent->rpm->get_raw(display->drm);

but since that's a bunch of churn, it's okay to leave that be for
now. We can decide on it later. But let's not have all those NULL
checks.

BR,
Jani.

>  }
>  
>  void intel_display_rpm_put_raw(struct intel_display *display, struct ref_tracker *wakeref)
>  {
> -	intel_runtime_pm_put_raw(display_to_rpm(display), wakeref);
> +	if (display->parent->rpm && display->parent->rpm->put_raw)
> +		display->parent->rpm->put_raw(display->drm, wakeref);
>  }
>  
>  struct ref_tracker *intel_display_rpm_get(struct intel_display *display)
>  {
> -	return intel_runtime_pm_get(display_to_rpm(display));
> +	return display->parent->rpm && display->parent->rpm->get ?
> +		display->parent->rpm->get(display->drm) : NULL;
> +
>  }
>  
>  struct ref_tracker *intel_display_rpm_get_if_in_use(struct intel_display *display)
>  {
> -	return intel_runtime_pm_get_if_in_use(display_to_rpm(display));
> +	return display->parent->rpm && display->parent->rpm->get_if_in_use ?
> +		display->parent->rpm->get_if_in_use(display->drm) : NULL;
>  }
>  
>  struct ref_tracker *intel_display_rpm_get_noresume(struct intel_display *display)
>  {
> -	return intel_runtime_pm_get_noresume(display_to_rpm(display));
> +	return display->parent->rpm && display->parent->rpm->get_noresume ?
> +		display->parent->rpm->get_noresume(display->drm) : NULL;
>  }
>  
>  void intel_display_rpm_put(struct intel_display *display, struct ref_tracker *wakeref)
>  {
> -	intel_runtime_pm_put(display_to_rpm(display), wakeref);
> +	if (display->parent->rpm && display->parent->rpm->put)
> +		display->parent->rpm->put(display->drm, wakeref);
>  }
>  
>  void intel_display_rpm_put_unchecked(struct intel_display *display)
>  {
> -	intel_runtime_pm_put_unchecked(display_to_rpm(display));
> +	if (display->parent->rpm && display->parent->rpm->put_unchecked)
> +		display->parent->rpm->put_unchecked(display->drm);
>  }
>  
>  bool intel_display_rpm_suspended(struct intel_display *display)
>  {
> -	return intel_runtime_pm_suspended(display_to_rpm(display));
> +	return display->parent->rpm && display->parent->rpm->suspended ?
> +		display->parent->rpm->suspended(display->drm) : false;
>  }
>  
>  void assert_display_rpm_held(struct intel_display *display)
>  {
> -	assert_rpm_wakelock_held(display_to_rpm(display));
> +	if (display->parent->rpm && display->parent->rpm->assert_held)
> +		display->parent->rpm->assert_held(display->drm);
>  }
>  
>  void intel_display_rpm_assert_block(struct intel_display *display)
>  {
> -	disable_rpm_wakeref_asserts(display_to_rpm(display));
> +	if (display->parent->rpm && display->parent->rpm->assert_block)
> +		display->parent->rpm->assert_block(display->drm);
>  }
>  
>  void intel_display_rpm_assert_unblock(struct intel_display *display)
>  {
> -	enable_rpm_wakeref_asserts(display_to_rpm(display));
> +	if (display->parent->rpm && display->parent->rpm->assert_block)
> +		display->parent->rpm->assert_unblock(display->drm);
>  }

-- 
Jani Nikula, Intel