Re: [Intel-gfx] [PATCH 2/3] drm/i915: Name our BARs based on the spec

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

On Wed, Oct 05, 2022 at 05:02:58PM +0100, Matthew Auld wrote:
> On Wed, 5 Oct 2022 at 16:42, Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We use all kinds of weird names for our base address registers.
> > Take the names from the spec and stick to them to avoid confusing
> > everyone.
> >
> > The only exceptions are IOBAR and LMEMBAR since naming them
> > IOBAR_BAR and LMEMBAR_BAR looks too funny, and yet I think
> > that adding the _BAR to GTTMMADR & co. (which don't have one
> > in the spec name) does make it more clear what they are.
> > And IOBAR vs. GTTMMADR_BAR also looks a bit too inconsistent
> > for my taste.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  .../gpu/drm/i915/display/intel_lpe_audio.c    |  4 ++--
> >  drivers/gpu/drm/i915/gt/intel_ggtt.c          | 12 +++++-----
> >  drivers/gpu/drm/i915/gvt/cfg_space.c          |  2 +-
> >  drivers/gpu/drm/i915/intel_pci_config.h       | 23 +++++++++++++------
> >  4 files changed, 25 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> > index dca6003ccac8..389ccdc46a1e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> > @@ -101,9 +101,9 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >         rsc[0].flags    = IORESOURCE_IRQ;
> >         rsc[0].name     = "hdmi-lpe-audio-irq";
> >
> > -       rsc[1].start    = pci_resource_start(pdev, GTTMMADR_BAR) +
> > +       rsc[1].start    = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
> >                 I915_HDMI_LPE_AUDIO_BASE;
> > -       rsc[1].end      = pci_resource_start(pdev, GTTMMADR_BAR) +
> > +       rsc[1].end      = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
> >                 I915_HDMI_LPE_AUDIO_BASE + I915_HDMI_LPE_AUDIO_SIZE - 1;
> >         rsc[1].flags    = IORESOURCE_MEM;
> >         rsc[1].name     = "hdmi-lpe-audio-mmio";
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > index b31fe0fb013f..668131c25da7 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > @@ -871,8 +871,8 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >         u32 pte_flags;
> >         int ret;
> >
> > -       GEM_WARN_ON(pci_resource_len(pdev, GTTMMADR_BAR) != gen6_gttmmadr_size(i915));
> > -       phys_addr = pci_resource_start(pdev, GTTMMADR_BAR) + gen6_gttadr_offset(i915);
> > +       GEM_WARN_ON(pci_resource_len(pdev, GEN4_GTTMMADR_BAR) != gen6_gttmmadr_size(i915));
> > +       phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) + gen6_gttadr_offset(i915);
> >
> >         /*
> >          * On BXT+/ICL+ writes larger than 64 bit to the GTT pagetable range
> > @@ -932,10 +932,10 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
> >         u16 snb_gmch_ctl;
> >
> >         if (!HAS_LMEM(i915) && !HAS_BAR2_SMEM_STOLEN(i915)) {
> > -               if (!i915_pci_resource_valid(pdev, GTT_APERTURE_BAR))
> > +               if (!i915_pci_resource_valid(pdev, GEN4_GMADR_BAR))
> >                         return -ENXIO;
> >
> > -               ggtt->gmadr = pci_resource(pdev, GTT_APERTURE_BAR);
> > +               ggtt->gmadr = pci_resource(pdev, GEN4_GMADR_BAR);
> >                 ggtt->mappable_end = resource_size(&ggtt->gmadr);
> >         }
> >
> > @@ -1089,10 +1089,10 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
> >         unsigned int size;
> >         u16 snb_gmch_ctl;
> >
> > -       if (!i915_pci_resource_valid(pdev, GTT_APERTURE_BAR))
> > +       if (!i915_pci_resource_valid(pdev, GEN4_GMADR_BAR))
> >                 return -ENXIO;
> >
> > -       ggtt->gmadr = pci_resource(pdev, GTT_APERTURE_BAR);
> > +       ggtt->gmadr = pci_resource(pdev, GEN4_GMADR_BAR);
> >         ggtt->mappable_end = resource_size(&ggtt->gmadr);
> >
> >         /*
> > diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c b/drivers/gpu/drm/i915/gvt/cfg_space.c
> > index eef3bba8a41b..19a663f6504f 100644
> > --- a/drivers/gpu/drm/i915/gvt/cfg_space.c
> > +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
> > @@ -356,7 +356,7 @@ void intel_vgpu_init_cfg_space(struct intel_vgpu *vgpu,
> >         vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_GTTMMIO].size =
> >                 pci_resource_len(pdev, GTTMMADR_BAR);
> >         vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_APERTURE].size =
> > -               pci_resource_len(pdev, GTT_APERTURE_BAR);
> > +               pci_resource_len(pdev, GMADR_BAR);
> 
> Hmm, where is that defined? Typo?

Was supposed to be GEN4_GMADR_BAR. Apparently I have lost gvt
from my .config once again...

-- 
Ville Syrjälä
Intel