[Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()

Kandpal, Suraj suraj.kandpal at intel.com
Tue Jun 20 10:30:29 UTC 2023


> 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.

Regards,
Suraj Kandpal

>  	addr = pci_iomap_range(pdev, 0,
> i915_mmio_reg_offset(GMD_ID_DISPLAY), sizeof(u32));
>  	if (!addr) {
>  		drm_err(&i915->drm, "Cannot map MMIO BAR to read
> display GMD_ID\n");
> --
> 2.39.2



More information about the Intel-gfx mailing list