Re: [PATCH] uxa/glamor: Refine CloseScreen and InitScreen process.

["Zhigang Gong" <zhigang.gong@linux.intel.com>]

> -----Original Message-----
> From:
> intel-gfx-bounces+zhigang.gong=linux.intel.com@lists.freedesktop.org
> [mailto:intel-gfx-bounces+zhigang.gong=linux.intel.com@lists.freedesktop.
> org] On Behalf Of Zhigang Gong
> Sent: Thursday, February 02, 2012 5:45 PM
> To: chris@chris-wilson.co.uk
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] uxa/glamor: Refine CloseScreen and
> InitScreen process.
> 
> > -----Original Message-----
> > From: zhigang.gong@linux.intel.com
> > [mailto:zhigang.gong@linux.intel.com]
> > Sent: Wednesday, February 01, 2012 7:47 PM
> > To: chris@chris-wilson.co.uk
> > Cc: intel-gfx@lists.freedesktop.org; zhigang.gong@linux.intel.com
> > Subject: [PATCH] uxa/glamor: Refine CloseScreen and InitScreen
> process.
> >
> > From: Zhigang Gong <zhigang.gong@linux.intel.com>
> >
> > The previous version calls glamor_egl_close_screen and
> > glamor_egl_free_screen manually which is not align with standard
> process.
> > Now glamor change the way to follow standard method:
> >
> > glamor layer and glamor egl layer both have their internal CloseScreens.
> > The correct sequence is after the I830CloseScreen is registered, then
> > register glamor_egl_close_screen and the last one is
> > glamor_close_screen. So we move out the intel_glamor_init from the
> > intel_uxa_init to I830ScreenInit and just after the registration of
> > I830CloseScreen.
> >
> > As the glamor interfaces changed, we need to check the glamor version
> > when load the glamor egl module to make sure we are loading the right
> > glamor module. If failed, it will switch back to UXA path.
> >
> > Signed-off-by: Zhigang Gong <zhigang.gong@linux.intel.com>
> > ---
> >  src/intel_driver.c |    3 +-
> >  src/intel_glamor.c |   66
> > +++++++++++++++++++++++++++------------------------
> >  src/intel_uxa.c    |    2 -
> >  uxa/uxa.h          |   10 ++++++-
> >  4 files changed, 44 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/intel_driver.c b/src/intel_driver.c index
> 9d1c4e8..d66a8fd
> > 100644
> > --- a/src/intel_driver.c
> > +++ b/src/intel_driver.c
> > @@ -1051,6 +1051,7 @@ I830ScreenInit(int scrnIndex, ScreenPtr
> screen,
> > int argc, char **argv)
> >  	intel->CreateScreenResources = screen->CreateScreenResources;
> >  	screen->CreateScreenResources = i830CreateScreenResources;
> >
> > +	intel_glamor_init(screen);
> >  	if (!xf86CrtcScreenInit(screen))
> >  		return FALSE;
> >
> > @@ -1124,8 +1125,6 @@ static void I830FreeScreen(int scrnIndex, int
> > flags)
> >  	ScrnInfoPtr scrn = xf86Screens[scrnIndex];
> >  	intel_screen_private *intel = intel_get_screen_private(scrn);
> >
> > -	intel_glamor_free_screen(scrnIndex, flags);
> > -
> >  	if (intel) {
> >  		intel_mode_fini(intel);
> >  		intel_close_drm_master(intel);
> > diff --git a/src/intel_glamor.c b/src/intel_glamor.c index
> > e96daa6..446dd3d 100644
> > --- a/src/intel_glamor.c
> > +++ b/src/intel_glamor.c
> > @@ -61,20 +61,31 @@
> > intel_glamor_create_screen_resources(ScreenPtr screen)  Bool
> > intel_glamor_pre_init(ScrnInfoPtr scrn)  {
> > +	pointer glamor_module;
> >  	intel_screen_private *intel;
> > +	CARD32 version;
> >  	intel = intel_get_screen_private(scrn);
> >
> >  	/* Load glamor module */
> > -	if (xf86LoadSubModule(scrn, "glamor_egl") &&
> > -	    glamor_egl_init(scrn, intel->drmSubFD)) {
> > -		xf86DrvMsg(scrn->scrnIndex, X_INFO,
> > -			   "glamor detected, initialising\n");
> > -		intel->uxa_flags |= UXA_USE_GLAMOR;
> > -	} else {
> > +	if ((glamor_module = xf86LoadSubModule(scrn, "glamor_egl"))) {
> > +		version = xf86GetModuleVersion(glamor_module);
> > +		if (version < MODULE_VERSION_NUMERIC(0,3,0)) {
> > +			xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> > +			"Incompatible glamor version, required >=
> 0.3.0.\n");
> > +		}
> > +		else {
> > +			if (glamor_egl_init(scrn, intel->drmSubFD)) {
> > +				xf86DrvMsg(scrn->scrnIndex, X_INFO,
> > +					   "glamor detected, initialising
> egl layer.\n");
> > +				intel->uxa_flags =
> UXA_GLAMOR_EGL_INITIALIZED;
> > +			}
> > +			else
> > +				xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> > +					   "glamor detected, failed to
> initialize egl.\n");
> > +		}
> > +	} else
> >  		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> >  			   "glamor not available\n");
> > -		intel->uxa_flags &= ~UXA_USE_GLAMOR;
> > -	}
> >
> >  	return TRUE;
> >  }
> > @@ -83,7 +94,13 @@ PixmapPtr
> >  intel_glamor_create_pixmap(ScreenPtr screen, int w, int h,
> >  			   int depth, unsigned int usage)
> >  {
> > -	return glamor_create_pixmap(screen, w, h, depth, usage);
> > +	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
> > +	intel_screen_private *intel = intel_get_screen_private(scrn);
> > +
> > +	if (intel->uxa_flags & UXA_USE_GLAMOR)
> > +		return glamor_create_pixmap(screen, w, h, depth, usage);
> > +	else
> > +		return NULL;
> >  }
> >
> >  Bool
> > @@ -145,17 +162,16 @@ intel_glamor_finish_access(PixmapPtr
> pixmap,
> > uxa_access_t access)
> >  	return;
> >  }
> >
> > -
> >  Bool
> >  intel_glamor_init(ScreenPtr screen)
> >  {
> >  	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
> >  	intel_screen_private *intel = intel_get_screen_private(scrn);
> >
> > -	if ((intel->uxa_flags & UXA_USE_GLAMOR) == 0)
> > -		return TRUE;
> > +	if ((intel->uxa_flags & UXA_GLAMOR_EGL_INITIALIZED) == 0)
> > +		goto fail;
> >
> > -	if (!glamor_init(screen, GLAMOR_INVERTED_Y_AXIS)) {
> > +	if (!glamor_init(screen, GLAMOR_INVERTED_Y_AXIS |
> > +GLAMOR_USE_EGL_SCREEN)) {
> >  		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> >  			   "Failed to initialize glamor\n");
> >  		goto fail;
> > @@ -168,7 +184,7 @@ intel_glamor_init(ScreenPtr screen)
> >  	}
> >
> >  	intel->uxa_driver->flags |= UXA_USE_GLAMOR;
> > -	intel->uxa_flags = intel->uxa_driver->flags;
> > +	intel->uxa_flags |= intel->uxa_driver->flags;
> >
> >  	intel->uxa_driver->finish_access = intel_glamor_finish_access;
> >
> > @@ -177,8 +193,8 @@ intel_glamor_init(ScreenPtr screen)
> >  	return TRUE;
> >
> >    fail:
> > -	xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> > -		   "Use standard UXA acceleration.");
> > +	xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> > +		   "Use standard UXA acceleration.\n");
> >  	return FALSE;
> >  }
> >
> > @@ -196,21 +212,9 @@ Bool
> >  intel_glamor_close_screen(ScreenPtr screen)  {
> >  	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
> > -	intel_screen_private * intel;
> > +	intel_screen_private *intel = intel_get_screen_private(scrn);
> >
> > -	intel = intel_get_screen_private(scrn);
> > -	if (intel && (intel->uxa_flags & UXA_USE_GLAMOR))
> > -		return glamor_egl_close_screen(screen);
> > +	if (intel->uxa_flags & UXA_USE_GLAMOR)
> > +		intel->uxa_flags &= ~UXA_USE_GLAMOR;
> >  	return TRUE;
> >  }
> > -
> > -void
> > -intel_glamor_free_screen(int scrnIndex, int flags) -{
> > -	ScrnInfoPtr scrn = xf86Screens[scrnIndex];
> > -	intel_screen_private * intel;
> > -
> > -	intel = intel_get_screen_private(scrn);
> > -	if (intel && (intel->uxa_flags & UXA_USE_GLAMOR))
> > -		glamor_egl_free_screen(scrnIndex,
> > GLAMOR_EGL_EXTERNAL_BUFFER);
> > -}
> > diff --git a/src/intel_uxa.c b/src/intel_uxa.c index f04a2ef..a11846d
> > 100644
> > --- a/src/intel_uxa.c
> > +++ b/src/intel_uxa.c
> > @@ -1391,7 +1391,5 @@ Bool intel_uxa_init(ScreenPtr screen)
> >  	uxa_set_fallback_debug(screen, intel->fallback_debug);
> >  	uxa_set_force_fallback(screen, intel->force_fallback);
> >
> > -	intel_glamor_init(screen);
> > -
> >  	return TRUE;
> >  }
> > diff --git a/uxa/uxa.h b/uxa/uxa.h
> > index 66b5f1e..b8569f0 100644
> > --- a/uxa/uxa.h
> > +++ b/uxa/uxa.h
> > @@ -548,12 +548,18 @@ typedef struct _UxaDriver {
> >  /**
> >   * UXA_USE_GLAMOR indicates to use glamor acceleration to perform
> > rendering.
> >   * And if glamor fail to accelerate the rendering, then goto fallback
> > to
> > - * use CPU to do the rendering.
> > + * use CPU to do the rendering. This flag will be set only when
> > + glamor get
> > + * initialized successfully.
> > + * Note, in ddx close screen, this bit need to be cleared.
> >   */
> >  #define UXA_USE_GLAMOR			(1 << 3)
> >
> > -/** @} */
> > +/* UXA_GLAMOR_EGL_INITIALIZED indicates glamor egl layer get
> > +initialized
> > + * successfully. UXA layer does not use this flag, before call to
> > + * glamor_init, ddx need to check this flag. */
> > +#define UXA_GLAMOR_EGL_INITIALIZED	(1 << 4)
> >
> > +/** @} */
> >  /** @name UXA CreatePixmap hint flags
> >   * @{
> >   */
> > --
> > 1.7.4.4
> This commit depends on the following glamor patch which is also under
> reviewing:
> http://lists.freedesktop.org/archives/glamor/2012-February/000066.html

The corresponding glamor patch has been pushed to glamor master tree,
commit id is 1bc8bf... 

> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx