On Wed, Sep 27, 2023 at 05:50:10PM +0300, Jani Nikula wrote: > On Wed, 27 Sep 2023, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > On Tue, Sep 26, 2023 at 12:34:35PM +0300, Jani Nikula wrote: > >> On Tue, 26 Sep 2023, "Manna, Animesh" <animesh.manna@xxxxxxxxx> wrote: > >> >> -----Original Message----- > >> >> From: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > >> >> Sent: Monday, September 25, 2023 6:00 PM > >> >> To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel- > >> >> gfx@xxxxxxxxxxxxxxxxxxxxx > >> >> Subject: Re: [PATCH] drm/i915/dsb: DSB code refactoring > >> >> > >> >> On Sat, 23 Sep 2023, Animesh Manna <animesh.manna@xxxxxxxxx> 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. obj+vma is a pretty reasonable API IMO. And we're not doing anything weird with their guts IIRC. But apparently xe decided to not give us that, and instead of adding a single wrapper to bridge the gap we now have several different ad-hoc wrappers for whatever reason. > > 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. Nothing preventing anyone from coming up with the single wrapper and upstreaming the i915 side (assuming we even want some kind of extra wrapper for i915 given it already uses a reasonable approach). > > Part of the reason we have these ad-hoc wrappers is that they also serve > as the todo list of stuff to fix properly. Feels more like we are trying to polish these to the point where they are supposed to be permanent solutions. -- Ville Syrjälä Intel
