[Intel-gfx] [PATCH] drm/i915/dsb: DSB code refactoring

[Jani Nikula]

On Wed, 27 Sep 2023, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Tue, Sep 26, 2023 at 12:34:35PM +0300, Jani Nikula wrote:
>> On Tue, 26 Sep 2023, "Manna, Animesh" <animesh.manna at intel.com> wrote:
>> >> -----Original Message-----
>> >> From: Jani Nikula <jani.nikula at linux.intel.com>
>> >> Sent: Monday, September 25, 2023 6:00 PM
>> >> To: Manna, Animesh <animesh.manna at intel.com>; intel-
>> >> gfx at lists.freedesktop.org
>> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsb: DSB code refactoring
>> >> 
>> >> On Sat, 23 Sep 2023, Animesh Manna <animesh.manna at intel.com> wrote:
>> >> > Refactor DSB implementation to be compatible with Xe driver.
>> >> 
>> >> Sad trombone.
>> >> 
>> >> struct intel_dsb should remain an opaque type. I put effort into hiding its
>> >> definition, so its guts wouldn't be accessed nilly-willy all over the place. If it's
>> >> not hidden, it just will get accessed.
>> >
>> > Hi Jani,
>> >
>> > Xe driver need access to intel_dsb structure, so I can create a new header file intel_dsb_ops.h and keep intel_dsb structure in it. Is it ok?
>> 
>> I just think you need to find a different abstraction level that doesn't
>> involve exposing struct intel_dsb.
>
> I hate the fact that we seem to be adding these ad-hoc wrappers all
> over the place. Someone should just fix xe to give us the same API as
> i915, or a single wrapper should do whatever conversion is needed.

I think one of the problems is that i915 doesn't really give us a proper
API either, but requires us to fiddle with the objects' guts, and thus
have access to the struct definitions. In i915, with the include
hierarchies, that effectively means including absolutely
everything. Can't have that in xe.

Having the same API for both i915 and xe requires turning it into an
actual API that doesn't depend on either i915 or xe specific types. But
that's kind of tough before xe is upstream. Catch-22.

Part of the reason we have these ad-hoc wrappers is that they also serve
as the todo list of stuff to fix properly.


BR,
Jani.


-- 
Jani Nikula, Intel