On Wed, Dec 13, 2023 at 06:26:13PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 13, 2023 at 05:05:21PM +0100, Andrzej Hajda wrote:
> > On 13.12.2023 01:42, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > Dump the details about every memory region into dmesg at probe time.
> > > Avoids having to dig those out from random places when debugging stuff.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_memory_region.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
> > > index b2708f8cac2a..52d998e5c21a 100644
> > > --- a/drivers/gpu/drm/i915/intel_memory_region.c
> > > +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> > > @@ -372,6 +372,24 @@ int intel_memory_regions_hw_probe(struct drm_i915_private *i915)
> > > i915->mm.regions[i] = mem;
> > > }
> > >
> > > + for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
> > > + struct intel_memory_region *mem = i915->mm.regions[i];
> > > + u64 region_size, io_size;
> > > +
> > > + if (!mem)
> > > + continue;
> > > +
> > > + region_size = resource_size(&mem->region) >> 20;
> > > + io_size = resource_size(&mem->io) >> 20;
> > > +
> > > + if (resource_size(&mem->io))
> > > + drm_dbg(&i915->drm, "Memory region(%d): %s: %llu MiB %pR, io: %llu MiB %pR\n",
> > > + mem->id, mem->name, region_size, &mem->region, io_size, &mem->io);
> > > + else
> > > + drm_dbg(&i915->drm, "Memory region(%d): %s: %llu MiB %pR, io: n/a\n",
> > > + mem->id, mem->name, region_size, &mem->region);
> >
> > Doesn't printk handle properly 0-length resources?
>
> Not without extra help. Apparently there is IORESOURCE_UNSET
> that would just print the size, but you have to explicitly
> set that yourself, which I suppose we could do.
This is what we get with nothing:
Memory region(6): stolen-local: 20 MiB [mem 0x3fe800000-0x3ffbfffff], io: 0 MiB [mem 0x00000000-0xffffffffffffffff]
This is with IORESOURCE_UNSET:
Memory region(6): stolen-local: 20 MiB [mem 0x3fe800000-0x3ffbfffff], io: 0 MiB [mem size 0x00000000]
And this is the original:
Memory region(6): stolen-local: 20 MiB [mem 0x3fe800000-0x3ffbfffff], io: n/a
I kinda like the explicit 'n/a' to make it obvious there is noting
there.
Also using IORESOURCE_UNSET would be a bit more annoying as we'd have
to explicitly preserve it in places where we resize the resource
(stolen reseved handling and selftests). But I suppose it would be
just a matter of using the full DEFINE_RES_NAMED() instead of
DEFINE_RES_MEM(). Hmm, maybe I should do that anyway just to be
safe in case someone starts to use some extra flags on these...
>
> >
> > Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
> >
> > Regards
> > Andrzej
> >
> >
> > > + }
> > > +
> > > return 0;
> > >
> > > out_cleanup:
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
