[PATCH 1/2] drm/i915/gem: Return -EINVAL instead of '0'

[Andi Shyti]

On Mon, Jun 17, 2024 at 10:46:07AM -0500, Lucas De Marchi wrote:
> On Mon, Jun 17, 2024 at 04:22:11PM GMT, Andi Shyti wrote:
> > On Mon, Jun 17, 2024 at 07:55:10AM -0500, Lucas De Marchi wrote:
> > > On Sun, Jun 16, 2024 at 09:03:48AM GMT, Andi Shyti wrote:
> > > > Commit 05da7d9f717b ("drm/i915/gem: Downgrade stolen lmem setup
> > > > warning") returns '0' from i915_gem_stolen_lmem_setup(), but it's
> > > > supposed to return a pointer to the intel_memory_region
> > > > structure.
> > > >
> > > > Sparse complains with the following message:
> > > >
> > > > > > drivers/gpu/drm/i915/gem/i915_gem_stolen.c:943:32: sparse: sparse:
> > > >   Using plain integer as NULL pointer
> > > >
> > > > The caller checks for errors, and if no error is returned, it
> > > > stores the address of the stolen memory. Therefore, we can't
> > > > return NULL. Since we are handling a case of out-of-bounds, it's
> > > > appropriate to treat the "lmem_size < dsm_base" case as an error.
> > > 
> > > which completely invalidates the point of the commit that introduced this
> > > regression. That was commit was supposed to do "let's continue, just
> > > disabling stolen".
> > 
> > Yes, correct, I missed the point while fixing stuff. But patch 2
> > is still valid.
> 
> no, it's not. It's introduced by the same commit. I went to look into
> this exactly because of the second issue: it broke 32b build in xe and
> all the CI.Hooks in xe are failing.

yes, it's broken because it's using %lli, right? In 32b it should
be %li.

Patch 2 is replacing %lli with %pa which should fix the 32b
build.

I'm sending a new series now.

Andi

> Lucas De Marchi
> 
> 
> > 
> > Thanks,
> > Andi