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

[Borah, Chaitanya Kumar]

Hello Jani,

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Wednesday, September 20, 2023 3:22 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>; intel-
> xe at lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna at intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah at intel.com>; Shankar, Uma
> <uma.shankar at intel.com>
> Subject: Re: [RFC 1/3] drm/i915: Add wrapper for xe compatibility
> 
> On Wed, 20 Sep 2023, Chaitanya Kumar Borah
> <chaitanya.kumar.borah at 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 at intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna at 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.
> 

Thank you for the inputs.
Please find the patch for i915 here

https://patchwork.freedesktop.org/patch/560528/?series=124340&rev=2

For the xe part, I will float once this patch is backported.

Regarding other parts of the Fixme patch, I am planning to add compatibility headers for the macro/debug function.
Let me know if you have any early comments on these.

+#ifndef _INTEL_UC_FW_H_
+#define _INTEL_UC_FW_H_
+
+#define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
+
+#endif

+#ifndef _I915_GPU_ERROR_H_
+#define _I915_GPU_ERROR_H_
+
+struct drm_i915_error_state_buf;
+
+__printf(2, 3)
+static inline void
+i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
+{
+}
+
+#endif

Regards

Chaitanya

> 
> 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