Re: [PATCH] drm/i915/icl, x86/gpu: implement ICL stolen memory support

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qui, 2018-05-03 às 12:59 +0300, Joonas Lahtinen escreveu:
> Quoting Paulo Zanoni (2018-05-03 03:23:52)
> > ICL changes the registers and addresses to 64 bits.
> > 
> > I also briefly looked at implementing an u64 version of the PCI
> > config
> > read functions, but I concluded this wouldn't be trivial, so it's
> > not
> > worth doing it for a single user that can't have any racing
> > problems
> > while reading the register in two separate operations.
> > 
> > v2:
> >   - Adjust the patch after the i915_stolen_to_dma() changes.
> >   - Remove unused variable (Daniele).
> >   - Update commit message.
> > v3:
> >   - Fix a missing phys_addr_t->dma_addr_t forgotten in v2 (kbuild
> > bot)
> > v4:
> >   - Rebase.
> > v5:
> >   - Fix warnings in arch/x86/kernel/early-quirks.c after rebase.
> > v6:
> >  - No more TODO list.
> >  - Stay under 80 columns.
> >  - Add debug message to match the other functions.
> 
> This will only confuse most readers, so please do scrub the internal
> changelog when sending the first revision on a mailing list.

We've been doing this for years, why did our opinion change today?


> 
> > Issue: VIZ-9250
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: x86@kernel.org
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.co
> > m> # Early review, needs re-check before merging
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  arch/x86/kernel/early-quirks.c         | 18 ++++++++++++++++
> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 38
> > +++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_reg.h        |  1 +
> >  include/drm/i915_drm.h                 |  4 +++-
> >  4 files changed, 59 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/early-quirks.c
> > b/arch/x86/kernel/early-quirks.c
> > index bae0d32e327b..96228bac1c8c 100644
> > --- a/arch/x86/kernel/early-quirks.c
> > +++ b/arch/x86/kernel/early-quirks.c
> > @@ -340,6 +340,18 @@ static resource_size_t __init
> > gen3_stolen_base(int num, int slot, int func,
> >         return bsm & INTEL_BSM_MASK;
> >  }
> >  
> > +static resource_size_t __init gen11_stolen_base(int num, int slot,
> > int func,
> > +                                                resource_size_t
> > stolen_size)
> > +{
> > +       u64 bsm;
> > +
> > +       bsm = read_pci_config(num, slot, func,
> > INTEL_GEN11_BSM_DW0);
> > +       bsm &= INTEL_BSM_MASK;
> > +       bsm |= (u64)read_pci_config(num, slot, func,
> > INTEL_GEN11_BSM_DW1) << 32;
> > +
> > +       return (resource_size_t)bsm;
> 
> return bsm; will suffice.
> 
> > +}
> > +
> >  static resource_size_t __init i830_stolen_size(int num, int slot,
> > int func)
> >  {
> >         u16 gmch_ctrl;
> > @@ -500,6 +512,11 @@ static const struct intel_early_ops
> > chv_early_ops __initconst = {
> >         .stolen_size = chv_stolen_size,
> >  };
> >  
> > +static const struct intel_early_ops gen11_early_ops __initconst =
> > {
> > +       .stolen_base = gen11_stolen_base,
> > +       .stolen_size = gen9_stolen_size,
> > +};
> > +
> >  static const struct pci_device_id intel_early_ids[] __initconst =
> > {
> >         INTEL_I830_IDS(&i830_early_ops),
> >         INTEL_I845G_IDS(&i845_early_ops),
> > @@ -531,6 +548,7 @@ static const struct pci_device_id
> > intel_early_ids[] __initconst = {
> >         INTEL_CFL_IDS(&gen9_early_ops),
> >         INTEL_GLK_IDS(&gen9_early_ops),
> >         INTEL_CNL_IDS(&gen9_early_ops),
> > +       INTEL_ICL_11_IDS(&gen11_early_ops),
> >  };
> 
> Please split the patch here and add a respective Fixes: tag to when
> base Icelake support was introduced. Lacking this portion when
> running ICL will
> cause random memory corruption so it's important to have this landed
> early.

Icelake is still hidden behind the alpha_support flag, so no point in
backporting for i915.ko specifically. If we're talking about
backporting just because of the memory reservation without graphics,
how far back should we go?

> 
> Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx