[Intel-xe] [PATCH 2/3] drm/xe: Add wrapper function for VGA decode setup

[Shankar, Uma]

> -----Original Message-----
> From: Shankar, Uma
> Sent: Wednesday, September 20, 2023 6:09 PM
> To: Nikula, Jani <jani.nikula at intel.com>; intel-xe at lists.freedesktop.org;
> ville.syrjala at linux.intel.com
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>
> Subject: RE: [PATCH 2/3] drm/xe: Add wrapper function for VGA decode setup
> 
> 
> 
> > -----Original Message-----
> > From: Nikula, Jani <jani.nikula at intel.com>
> > Sent: Wednesday, September 20, 2023 3:38 PM
> > To: Shankar, Uma <uma.shankar at intel.com>;
> > intel-xe at lists.freedesktop.org
> > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>
> > Subject: RE: [PATCH 2/3] drm/xe: Add wrapper function for VGA decode
> > setup
> >
> > On Wed, 20 Sep 2023, "Shankar, Uma" <uma.shankar at intel.com> wrote:
> > >> -----Original Message-----
> > >> From: Nikula, Jani <jani.nikula at intel.com>
> > >> Sent: Tuesday, September 19, 2023 4:00 PM
> > >> To: Shankar, Uma <uma.shankar at intel.com>;
> > >> intel-xe at lists.freedesktop.org
> > >> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>;
> > >> Shankar, Uma <uma.shankar at intel.com>
> > >> Subject: Re: [PATCH 2/3] drm/xe: Add wrapper function for VGA
> > >> decode setup
> > >>
> > >> On Mon, 18 Sep 2023, Uma Shankar <uma.shankar at intel.com> wrote:
> > >> > Some of the VGA functionality is not needed by the Intel Xe driver.
> > >> > Adding a wrapper function for VGA decode setup.
> > >> >
> > >> > Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> > >> > ---
> > >> >  drivers/gpu/drm/xe/display/ext/i915_utils.c | 5 +++++
> > >> >  drivers/gpu/drm/xe/xe_display.h             | 1 +
> > >> >  2 files changed, 6 insertions(+)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/xe/display/ext/i915_utils.c
> > >> > b/drivers/gpu/drm/xe/display/ext/i915_utils.c
> > >> > index 981edc2788bc..aa3e9ad718c3 100644
> > >> > --- a/drivers/gpu/drm/xe/display/ext/i915_utils.c
> > >> > +++ b/drivers/gpu/drm/xe/display/ext/i915_utils.c
> > >> > @@ -20,3 +20,8 @@ int __i915_inject_probe_error(struct
> > >> > drm_i915_private *i915, int err,  {
> > >> >     return 0;
> > >> >  }
> > >> > +
> > >> > +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev,
> > >> > +bool
> > >> > +enable_decode) {
> > >> > +   return -EIO;
> > >> > +}
> > >>
> > >> We don't really want to add anything new to i915_utils.c. It's just
> > >> temporary cruft that had to be taken from i915 to make the rest compile.
> > >
> > > Hi Jani,
> > > Thanks for the feedback.
> > >
> > > I was looking at any good place to stack it together. There are some
> > > misc stuff like this which we may need for some other stuff as well.
> > > Should I add a xe_disp_misc file for this or any other suggestion on
> > > file which
> > we can use.
> >
> > Let's take a step back, though.
> >
> > Why is this call needed on i915 but not on xe?
> 
> Ideally it should be needed even here. Not sure on the exact usage though, the
> GMCHGraphicsControl is there on current platforms as well. Trying to gather
> some info to get clarity.
> 
> @ville.syrjala at linux.intel.com Do you know how critical this is or can we defer its
> enabling and add support later directly in upstream.

Hi Jani,
Tried to check this and it seems Xe is managing this per tile and doing it at tile initialization (xe_tile_init_noalloc)
So, we should need this for Xe separately. Hope my understanding is correct.

Regards,
Uma Shankar

> > BR,
> > Jani.
> >
> >
> > >
> > > Regards,
> > > Uma Shankar
> > >
> > >> > diff --git a/drivers/gpu/drm/xe/xe_display.h
> > >> > b/drivers/gpu/drm/xe/xe_display.h index
> > >> > 03b7ea9268d4..d7a200e6ec30
> > >> > 100644
> > >> > --- a/drivers/gpu/drm/xe/xe_display.h
> > >> > +++ b/drivers/gpu/drm/xe/xe_display.h
> > >> > @@ -38,6 +38,7 @@ void xe_display_pm_suspend(struct xe_device
> > >> > *xe); void xe_display_pm_suspend_late(struct xe_device *xe);
> > >> > void xe_display_pm_resume_early(struct xe_device *xe);  void
> > >> > xe_display_pm_resume(struct xe_device *xe);
> > >> > +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev,
> > >> > +bool enable_decode);
> > >>
> > >> We also don't want to add random declarations in xe_display.h either...
> > >
> > > Yeah, looking deeper seems we don't need it as compat-header is
> > > already
> > including the same.
> > > Will drop this declaration.
> > >
> > > Regards,
> > > Uma Shankar
> > >
> > >> BR,
> > >> Jani.
> > >>
> > >> >
> > >> >  #else
> > >>
> > >> --
> > >> Jani Nikula, Intel
> >
> > --
> > Jani Nikula, Intel