Re: [PATCHv2] drm/xe/display: check for error on drmm_mutex_init

From: Andi Shyti <andi.shyti@linux.intel.com>
To: Arun R Murthy <arun.r.murthy@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCHv2] drm/xe/display: check for error on drmm_mutex_init
Date: Thu, 28 Mar 2024 09:31:31 +0100	[thread overview]
Message-ID: <ZgUq47D313cr2VYp@ashyti-mobl2.lan> (raw)
In-Reply-To: <20240328080005.410961-1-arun.r.murthy@intel.com>

Hi Arun,

...

> -	drmm_mutex_init(&xe->drm, &xe->sb_lock);
> -	drmm_mutex_init(&xe->drm, &xe->display.backlight.lock);
> -	drmm_mutex_init(&xe->drm, &xe->display.audio.mutex);
> -	drmm_mutex_init(&xe->drm, &xe->display.wm.wm_mutex);
> -	drmm_mutex_init(&xe->drm, &xe->display.pps.mutex);
> -	drmm_mutex_init(&xe->drm, &xe->display.hdcp.hdcp_mutex);
> +	if (drmm_mutex_init(&xe->drm, &xe->sb_lock) ||
> +	    drmm_mutex_init(&xe->drm, &xe->display.backlight.lock) ||
> +	    drmm_mutex_init(&xe->drm, &xe->display.audio.mutex) ||
> +	    drmm_mutex_init(&xe->drm, &xe->display.wm.wm_mutex) ||
> +	    drmm_mutex_init(&xe->drm, &xe->display.pps.mutex) ||
> +	    drmm_mutex_init(&xe->drm, &xe->display.hdcp.hdcp_mutex))
> +		return -ENOMEM;

why not extract the value from drmm_mutex_init()? it would make
the code a bit more complex, but better than forcing a -ENOMEM
return.

	err = drmm_mutex_init(...)
	if (err)
		return err;

	err = drmm_mutex_init(...)
	if (err)
		return err;

	err = drmm_mutex_init(...)
	if (err)
		return err;

	...

On the other hand drmm_mutex_init(), as of now returns only
-ENOMEM, but it's a bad practice to assume it will always do. I'd
rather prefer not to check the error value at all.

Andi

>  	xe->enabled_irq_mask = ~0;
>  
>  	err = drmm_add_action_or_reset(&xe->drm, display_destroy, NULL);
> -- 
> 2.25.1