[Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
Coelho, Luciano
luciano.coelho at intel.com
Thu Jun 22 09:50:19 UTC 2023
On Tue, 2023-06-20 at 10:30 +0000, Kandpal, Suraj wrote:
> > When intel_display_device_probe() (and, subsequently,
> > probe_gmdid_display()) returns, the caller expects ver, rel and
> > step to be
> > initialized. Since there's no way to check that there was a
> > failure and
> > no_display was returned without some further refactoring, pre-
> > initiliaze all
> > these values to zero to keep it simple and safe.
> >
> > Signed-off-by: Luca Coelho <luciano.coelho at intel.com>
>
> Looks okay to me just a small suggestion/question below.
>
> > ---
> > drivers/gpu/drm/i915/display/intel_display_device.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
> > b/drivers/gpu/drm/i915/display/intel_display_device.c
> > index 464df1764a86..fb6354e9e704 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> > @@ -731,6 +731,15 @@ probe_gmdid_display(struct drm_i915_private
> > *i915, u16 *ver, u16 *rel, u16 *step
> > u32 val;
> > int i;
> >
> > + /* The caller expects to ver, rel and step to be
> > initialized
> > + * here, and there's no good way to check when there was a
> > + * failure and no_display was returned. So initialize all
> > these
> > + * values here zero, to be sure.
> > + */
> > + *ver = 0;
> > + *rel = 0;
> > + *step = 0;
> > +
>
> From what I can see this is only called from
> intel_display_device_probe() which is in turn
> called from intel_device_info_driver_create() where the above
> variables are defined maybe
> we initialize these values there itself.
Thanks for the review!
I thought about initializing the variables on the caller side, but
reckoned that it would be more intuitive to initialize them in the
probe_gmdid_display() function instead, because the caller expects
those values to be set in successful cases and there's no way for it to
know whether there was a failure or not (because we return a pointer to
local no_display structure that the caller doesn't know about).
Obviously with the current code in the caller, that doesn't make much
difference, but I thought it was cleaner as I did.
But I'm okay to change it and initialize them at the caller, so just
let me know if you want that.
--
Cheers,
Luca.
More information about the Intel-gfx
mailing list