[Intel-gfx] [RFC PATCH] drm/i915: don't treat small BAR as an error with CSS

[Matthew Auld]

On Wed, 11 May 2022 at 13:34, Das, Nirmoy <nirmoy.das at intel.com> wrote:
>
>
> On 5/11/2022 1:31 PM, Matthew Auld wrote:
> > On Wed, 11 May 2022 at 11:25, Nirmoy Das <nirmoy.das at intel.com> wrote:
> >> Determine lmem_size using ADDR_RANGE register so that module
> >> load on platfrom with small bar with css  works.
> >>
> >> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
> >> ---
> >> I once reserved a dg2 machine with small bar and module load failed on
> >> it. I can't find that machine anymore hence sending this as RFC.
> > AFAIK we currently don't want to load the driver on such dg2
> > configurations, until we first have all the uapi bits landed.
>
>
> Ok, sounds good.
>
> >   The last
> > patch in that series then turns this on, or at least that's what I
> > have locally.
> >
> >>   drivers/gpu/drm/i915/gt/intel_region_lmem.c | 11 ++++++++++-
> >>   1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> >> index f5111c0a0060..a55eecac104e 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> >> @@ -100,10 +100,19 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt)
> >>          if (!IS_DGFX(i915))
> >>                  return ERR_PTR(-ENODEV);
> >>
> >> +       if (IS_DG1(uncore->i915)) {
> >> +               lmem_size = pci_resource_len(pdev, 2);
> > We can drop this, since this is set below also.
> >
> >> +       } else {
> >> +               resource_size_t lmem_range;
> >> +
> >> +               lmem_range = intel_gt_read_register(&i915->gt0, XEHPSDV_TILE0_ADDR_RANGE) & 0xFFFF;
> >> +               lmem_size = lmem_range >> XEHPSDV_TILE_LMEM_RANGE_SHIFT;
> >> +               lmem_size *= SZ_1G;
> > We can move this under HAS_FLAT_CCS.
> >
> > I think we need another patch that then just gracefully returns
> > -EINVAL if this is a small-bar configuration, along with maybe some
> > helpful drm_err() or so, which can be removed once we properly support
> > it?
>
> I will resend with this suggestion.
>
>
> >   Also it looks like we are returning ENODEV in some places here,
> > which looks iffy.
>
>
> We do
>
>          io_start = pci_resource_start(pdev, 2);
>          io_size = min(pci_resource_len(pdev, 2), lmem_size);
>          if (!io_size)
>                  return ERR_PTR(-ENODEV);
>
> Is this return looks iffy?

Yeah, since it will only skips the lmem init, without erroring out
during module load, which I guess leads to nasty errors laters. Also
the lmem_size < flat_ccs_base check.

>
>
> Thanks,
>
> Nirmoy
>
> >
> >> +       }
> >> +
> >>          if (HAS_FLAT_CCS(i915)) {
> >>                  u64 tile_stolen, flat_ccs_base;
> >>
> >> -               lmem_size = pci_resource_len(pdev, 2);
> >>                  flat_ccs_base = intel_gt_read_register(gt, XEHPSDV_FLAT_CCS_BASE_ADDR);
> >>                  flat_ccs_base = (flat_ccs_base >> XEHPSDV_CCS_BASE_SHIFT) * SZ_64K;
> >>
> >> --
> >> 2.35.1
> >>