| Message ID | 20220511102509.19927-1-nirmoy.das@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/i915: don't treat small BAR as an error with CSS | expand |
+Ram (git send-email didn't add Ram to the cc list for some reason) On 5/11/2022 12:25 PM, Nirmoy Das 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@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. > > 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); > + } 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; > + } > + > 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; >
On Wed, 11 May 2022 at 11:25, Nirmoy Das <nirmoy.das@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@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. 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? Also it looks like we are returning ENODEV in some places here, which looks iffy. > + } > + > 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 >
On 5/11/2022 1:31 PM, Matthew Auld wrote: > On Wed, 11 May 2022 at 11:25, Nirmoy Das <nirmoy.das@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@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? 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 >>
On Wed, 11 May 2022 at 13:34, Das, Nirmoy <nirmoy.das@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@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@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 > >>
On 5/11/2022 4:14 PM, Matthew Auld wrote: > On Wed, 11 May 2022 at 13:34, Das, Nirmoy <nirmoy.das@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@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@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. Yes, makes sense. Going to send patch to clean that up. Thanks, Nirmoy > >> >> 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 >>>>
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); + } 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; + } + 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;
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@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. drivers/gpu/drm/i915/gt/intel_region_lmem.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)