Re: [PATCH 2/6] drm/{i915, xe}/display: Add display runtime pm parent interface

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

On Wed, 22 Oct 2025, Jouni Högander <jouni.hogander@intel.com> wrote:
> We have differing implementations for display runtime pm in i915 and xe
> drivers. Add struct of function pointers into display_parent_interface
> which will contain used implementation of runtime pm.
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  include/drm/intel/display_parent_interface.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> index 28c976815327a..7ad0a27d503c8 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -8,6 +8,22 @@
>  
>  struct drm_device;
>  
> +struct intel_display_rpm {

I'm wondering if it would better emphasize the point if we named these
structs with _interface suffix.

> +	struct ref_tracker *(*get)(const struct drm_device *drm);

I think we should have the struct ref_tracker forward declaration above.

> +	struct ref_tracker *(*get_raw)(const struct drm_device *drm);
> +	struct ref_tracker *(*get_if_in_use)(const struct drm_device *drm);
> +	struct ref_tracker *(*get_noresume)(const struct drm_device *drm);
> +
> +	void (*put)(const struct drm_device *drm, struct ref_tracker *wakeref);
> +	void (*put_raw)(const struct drm_device *drm, struct ref_tracker *wakeref);
> +	void (*put_unchecked)(const struct drm_device *drm);
> +
> +	bool (*suspended)(const struct drm_device *drm);
> +	void (*assert_held)(const struct drm_device *drm);
> +	void (*assert_block)(const struct drm_device *drm);
> +	void (*assert_unblock)(const struct drm_device *drm);
> +};
> +
>  /**
>   * struct intel_display_parent_interface - services parent driver provides to display
>   *
> @@ -21,6 +37,8 @@ struct drm_device;
>   * check the optional pointers.
>   */
>  struct intel_display_parent_interface {
> +	/* Runtime PM functions */

I think these should be kernel-doc comments, i.e. /** @rpm: ... */. They
can still be one-liners.

BR,
Jani.

> +	const struct intel_display_rpm *rpm;
>  };
>  
>  #endif

-- 
Jani Nikula, Intel