[PATCH v2 2/5] drm/udl: Move drm_dev_{enter,exit}() into udl_get_edid_block()

[Jani Nikula]

On Wed, 10 Apr 2024, Thomas Zimmermann <tzimmermann at suse.de> wrote:
> Protect the code in udl_get_edid_block() with drm_dev_enter() and
> drm_dev_exit(), so that all callers automatically invoke it. The
> function uses hardware resources, which can be hot-unplugged at
> any time. The other code in udl_connector_detect() does not use the
> resources of the hardware device and therefore does not require
> protection.
>
> This change will allow to use udl_get_edid_block() in various
> contexts easily.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>

Reviewed-by: Jani Nikula <jani.nikula at intel.com>

> ---
>  drivers/gpu/drm/udl/udl_modeset.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
> index 751da3a294c44..3df9fc38388b4 100644
> --- a/drivers/gpu/drm/udl/udl_modeset.c
> +++ b/drivers/gpu/drm/udl/udl_modeset.c
> @@ -434,13 +434,18 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block, size_t le
>  	struct drm_device *dev = &udl->drm;
>  	struct usb_device *udev = udl_to_usb_device(udl);
>  	u8 *read_buff;
> -	int ret;
> +	int idx, ret;
>  	size_t i;
>  
>  	read_buff = kmalloc(2, GFP_KERNEL);
>  	if (!read_buff)
>  		return -ENOMEM;
>  
> +	if (!drm_dev_enter(dev, &idx)) {
> +		ret = -ENODEV;
> +		goto err_kfree;
> +	}
> +
>  	for (i = 0; i < len; i++) {
>  		int bval = (i + block * EDID_LENGTH) << 8;
>  
> @@ -449,20 +454,23 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block, size_t le
>  				      0xA1, read_buff, 2, USB_CTRL_GET_TIMEOUT);
>  		if (ret < 0) {
>  			drm_err(dev, "Read EDID byte %zu failed err %x\n", i, ret);
> -			goto err_kfree;
> +			goto err_drm_dev_exit;
>  		} else if (ret < 1) {
>  			ret = -EIO;
>  			drm_err(dev, "Read EDID byte %zu failed\n", i);
> -			goto err_kfree;
> +			goto err_drm_dev_exit;
>  		}
>  
>  		buf[i] = read_buff[1];
>  	}
>  
> +	drm_dev_exit(idx);
>  	kfree(read_buff);
>  
>  	return 0;
>  
> +err_drm_dev_exit:
> +	drm_dev_exit(idx);
>  err_kfree:
>  	kfree(read_buff);
>  	return ret;
> @@ -474,21 +482,15 @@ static enum drm_connector_status udl_connector_detect(struct drm_connector *conn
>  	struct udl_device *udl = to_udl(dev);
>  	struct udl_connector *udl_connector = to_udl_connector(connector);
>  	enum drm_connector_status status = connector_status_disconnected;
> -	int idx;
>  
>  	/* cleanup previous EDID */
>  	kfree(udl_connector->edid);
>  	udl_connector->edid = NULL;
>  
> -	if (!drm_dev_enter(dev, &idx))
> -		return connector_status_disconnected;
> -
>  	udl_connector->edid = drm_do_get_edid(connector, udl_get_edid_block, udl);
>  	if (udl_connector->edid)
>  		status = connector_status_connected;
>  
> -	drm_dev_exit(idx);
> -
>  	return status;
>  }

-- 
Jani Nikula, Intel