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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 20, 2023 at 12:39:19PM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Nikula, Jani <jani.nikula@intel.com>
> > Sent: Wednesday, September 20, 2023 3:38 PM
> > To: Shankar, Uma <uma.shankar@intel.com>; intel-xe@lists.freedesktop.org
> > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@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@intel.com> wrote:
> > >> -----Original Message-----
> > >> From: Nikula, Jani <jani.nikula@intel.com>
> > >> Sent: Tuesday, September 19, 2023 4:00 PM
> > >> To: Shankar, Uma <uma.shankar@intel.com>;
> > >> intel-xe@lists.freedesktop.org
> > >> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> > >> Shankar, Uma <uma.shankar@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@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@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@linux.intel.com Do you know how critical this is or can we defer its enabling and
> add support later directly in upstream.

That whole code is mostly nonsense since that register is normally
locked by the BIOS. So despite what the code thinks it cannot realibly
be used to enable/disable VGA decoding. I have some old branch somewhere
that tries to use other means to control this, but IIRC for some reason
I had to resort to stop_machine() to make it not suck in some way.

-- 
Ville Syrjälä
Intel