Re: [PATCH 5/9] drm/xe/display: Use a single early init call for display

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Nov 13, 2024 at 11:30:36AM +0100, Maarten Lankhorst wrote:
> Hey,
> 
> Den 2024-11-12 kl. 19:39, skrev Rodrigo Vivi:
> > On Thu, Nov 07, 2024 at 11:01:36AM +0100, Maarten Lankhorst wrote:
> > > Instead of 3 different calls, it should be safe to unify to a single
> > > call now. This makes the init sequence cleaner, and display less
> > > tangled.
> > > 
> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Link: https://patchwork.freedesktop.org/patch/msgid/20241105121857.17389-5-maarten.lankhorst@linux.intel.com
> > > Signed-off-by: Maarten Lankhorst,,, <dev@lankhorst.se>
> > > ---
> > >   drivers/gpu/drm/xe/display/xe_display.c | 72 +++++++------------------
> > >   drivers/gpu/drm/xe/display/xe_display.h |  8 +--
> > >   drivers/gpu/drm/xe/xe_device.c          | 10 +---
> > >   3 files changed, 23 insertions(+), 67 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> > > index b5502f335f531..a9ce4f561e7aa 100644
> > > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > 
> > we should make every function in this file only a wrap to the i915
> > side with the 	if (!xe->info.probe_display) being the only thing
> > extra in here... so we consolidate the display side in a way
> > that we could later really split the display to a separate driver.
> > 
> > I have the feeling that this patch takes display to the other direction...
> > 
> It should not matter much. I believe i915 had more split because of display
> being more connected to hardware. In Xe the display is from the start a
> separate driver or module on the chip , so we can do the different init
> calls in one go, as there are no interrupts
> calling into the display driver yet.
> 
> So in Xe we can do a 2-stage init of display, xe_display_init_early which
> initialises everything required before enabling interrupts, completed with
> the takeover of the initial FB, and xe_display_init() for the normal init
> sequence that has to run afterwards.
> 
> Because of memirq's performing allocations by design, we really have to run
> the FB handover code without interrupts anyway.
> 
> I believe the same thing could be done on i915 on BDW+, but might be harder
> on legacy platforms.

Okay... it shouldn't matter much indeed. Fix your signed off by and
feel free to use
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> Cheers,
> ~Maarten