[PATCH V2 1/3] drm/xe: Move enable host l2 VRAM post MCR init

[Matt Roper]

On Wed, Aug 14, 2024 at 11:34:21AM -0500, Lucas De Marchi wrote:
> On Wed, Aug 14, 2024 at 03:26:12PM GMT, Tejas Upadhyay wrote:
> > enable host l2 VRAM is reading MCR register thus
> > should be moved after MCR init is done.
> 
> This commit message skips a lot of the details on what was discussed in
> the other thread. Let's improve it and for future please be more verbose
> in your commit messages. Suggestion:
> 
> 	xe_gt_enable_host_l2_vram() is reading the XE2_GAMREQSTRM_CTRL register
> 	that is currently missing the MCR annotation. However, just adding the
> 	annotation doesn't work as this function is called before MCR handling
> 	is initialized in xe_gt_mcr_init().
> 
> 	xe_gt_enable_host_l2_vram() is used to implement WA 16023588340 that
> 	needs to be done as early as possible during initialization in order
> 	to be effective since the MMIO writes impact it. In the failure
> 	scenario, driver would simply not be able to bind sucesfully.
> 
> 	Moving xe_gt_enable_host_l2_vram() later, after MCR initialization is
> 	done, only incurs a few additional HW accesses, particularly when
> 	loading GuC for hwconfig. Binding/unbinding the driver 100 times in
> 	BMG still works so it should be ok to start handling the WA a little
> 	bit later. This is sufficient to allow adding the MCR annotation to
> 	XE2_GAMREQSTRM_CTRL.
> 
> > 
> > V2(Matt):
> > - Reword commit message
> > V1(Lucas):
> > - Reorder patch and reorder flow of L2 VRAM enable
> > 
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> 
> Matt Roper / Matthew Auld, do the paragraphs above capture
> correctly the discussion we had in the previous version?

Yeah, that expanded description looks good to me.  With that,

        Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


Matt

> 
> with that,
> 
> // Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> thanks
> Lucas De Marchi
> 
> > ---
> > drivers/gpu/drm/xe/xe_gt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index 58895ed22f6e..238c7d1053f0 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -557,7 +557,6 @@ int xe_gt_init_hwconfig(struct xe_gt *gt)
> > 
> > 	xe_gt_mcr_init_early(gt);
> > 	xe_pat_init(gt);
> > -	xe_gt_enable_host_l2_vram(gt);
> > 
> > 	err = xe_uc_init(&gt->uc);
> > 	if (err)
> > @@ -569,6 +568,7 @@ int xe_gt_init_hwconfig(struct xe_gt *gt)
> > 
> > 	xe_gt_topology_init(gt);
> > 	xe_gt_mcr_init(gt);
> > +	xe_gt_enable_host_l2_vram(gt);
> > 
> > out_fw:
> > 	xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > -- 
> > 2.25.1
> > 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation