Re: [Intel-xe] [RFC 1/3] drm/i915: Add wrapper for xe compatibility

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

On Wed, 20 Sep 2023, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
> Add a wrapper around intel_step_name to maintain compatibility
> with xe driver. The wrapper will share the same name as the one
> to be used by xe while both drivers include different files as
> needed during compilation.
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_step.c | 5 +++++
>  drivers/gpu/drm/i915/intel_step.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
> index c02a6f156a00..6b62b5fcc792 100644
> --- a/drivers/gpu/drm/i915/intel_step.c
> +++ b/drivers/gpu/drm/i915/intel_step.c
> @@ -352,3 +352,8 @@ const char *intel_step_name(enum intel_step step)
>  		return "**";
>  	}
>  }
> +
> +const char *intel_get_step_name(struct drm_i915_private *i915)
> +{
> +	return intel_step_name(RUNTIME_INFO(i915)->step.display_step);
> +}

Okay, so I've mulled this back and forth. Here's the, uh, steps I think
should be taken:

1) Please name this intel_display_step_name() or
   intel_step_display_name() or something. It's about *display*
   stepping, and just intel_get_step_name() is a misnomer.

2) Use this in intel_dmc.c.

3) Send the above to intel-gfx, based on drm-tip. Make it into a single
   patch. Get it merged to upstream i915.

4) On xe, revert the FIXME patch in question.

5) Add a fixup to remove "#define intel_step_name xe_step_name" from
   compat-i915-headers/i915_drv.h

6) Backport the upstream patch from 3) to xe.

7) Add xe_step_display_name() or similar, mimicking the one added to
   i915. Don't name it the same as in i915.

8) In compat-i915-headers/intel_step.h, add a static inline to bridge
   the call from intel_step_display_name() to xe_step_display_name(). Or
   make it a #define.


BR,
Jani.



> diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
> index 96dfca4cba73..25a1b1948a43 100644
> --- a/drivers/gpu/drm/i915/intel_step.h
> +++ b/drivers/gpu/drm/i915/intel_step.h
> @@ -78,5 +78,6 @@ enum intel_step {
>  
>  void intel_step_init(struct drm_i915_private *i915);
>  const char *intel_step_name(enum intel_step step);
> +const char *intel_get_step_name(struct drm_i915_private *i915);
>  
>  #endif /* __INTEL_STEP_H__ */

-- 
Jani Nikula, Intel