[PATCH v2 4/5] drm/udl: Untangle .get_modes() and .detect_ctx()

[Thomas Zimmermann]

Hi

Am 10.05.24 um 14:17 schrieb Jani Nikula:

>> +	/*
>> +	 * The adapter sends all-zeros if no monitor has been
>> +	 * connected. We consider anything else a connection.
>> +	 */
>> +	return memcmp(no_edid, hdr, 8) != 0;
> Nitpick, this works, but you can drop the no_edid buf by using:
>
> 	return memchr_inv(hdr, 0, sizeof(hdr));

Makes sense to me. Thanks for reviewing.

Best regards
Thomas

>
> Up to you,
>
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>
>
>> +}
>> +
>> +const struct drm_edid *udl_edid_read(struct drm_connector *connector)
>> +{
>> +	struct udl_device *udl = to_udl(connector->dev);
>> +
>> +	return drm_edid_read_custom(connector, udl_read_edid_block, udl);
>> +}
>> diff --git a/drivers/gpu/drm/udl/udl_edid.h b/drivers/gpu/drm/udl/udl_edid.h
>> new file mode 100644
>> index 0000000000000..fe15ff3752b7d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/udl/udl_edid.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef UDL_EDID_H
>> +#define UDL_EDID_H
>> +
>> +#include <linux/types.h>
>> +
>> +struct drm_connector;
>> +struct drm_edid;
>> +struct udl_device;
>> +
>> +bool udl_probe_edid(struct udl_device *udl);
>> +const struct drm_edid *udl_edid_read(struct drm_connector *connector);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
>> index 3df9fc38388b4..4236ce57f5945 100644
>> --- a/drivers/gpu/drm/udl/udl_modeset.c
>> +++ b/drivers/gpu/drm/udl/udl_modeset.c
>> @@ -25,6 +25,7 @@
>>   #include <drm/drm_vblank.h>
>>   
>>   #include "udl_drv.h"
>> +#include "udl_edid.h"
>>   #include "udl_proto.h"
>>   
>>   /*
>> @@ -415,97 +416,44 @@ static const struct drm_encoder_funcs udl_encoder_funcs = {
>>   
>>   static int udl_connector_helper_get_modes(struct drm_connector *connector)
>>   {
>> -	struct udl_connector *udl_connector = to_udl_connector(connector);
>> +	const struct drm_edid *drm_edid;
>> +	int count;
>>   
>> -	drm_connector_update_edid_property(connector, udl_connector->edid);
>> -	if (udl_connector->edid)
>> -		return drm_add_edid_modes(connector, udl_connector->edid);
>> +	drm_edid = udl_edid_read(connector);
>> +	drm_edid_connector_update(connector, drm_edid);
>> +	count = drm_edid_connector_add_modes(connector);
>> +	drm_edid_free(drm_edid);
>>   
>> -	return 0;
>> +	return count;
>>   }
>>   
>> -static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
>> -	.get_modes = udl_connector_helper_get_modes,
>> -};
>> -
>> -static int udl_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
>> +static int udl_connector_helper_detect_ctx(struct drm_connector *connector,
>> +					   struct drm_modeset_acquire_ctx *ctx,
>> +					   bool force)
>>   {
>> -	struct udl_device *udl = data;
>> -	struct drm_device *dev = &udl->drm;
>> -	struct usb_device *udev = udl_to_usb_device(udl);
>> -	u8 *read_buff;
>> -	int idx, ret;
>> -	size_t i;
>> -
>> -	read_buff = kmalloc(2, GFP_KERNEL);
>> -	if (!read_buff)
>> -		return -ENOMEM;
>> +	struct udl_device *udl = to_udl(connector->dev);
>>   
>> -	if (!drm_dev_enter(dev, &idx)) {
>> -		ret = -ENODEV;
>> -		goto err_kfree;
>> -	}
>> -
>> -	for (i = 0; i < len; i++) {
>> -		int bval = (i + block * EDID_LENGTH) << 8;
>> -
>> -		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>> -				      0x02, (0x80 | (0x02 << 5)), bval,
>> -				      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_drm_dev_exit;
>> -		} else if (ret < 1) {
>> -			ret = -EIO;
>> -			drm_err(dev, "Read EDID byte %zu failed\n", i);
>> -			goto err_drm_dev_exit;
>> -		}
>> -
>> -		buf[i] = read_buff[1];
>> -	}
>> +	if (udl_probe_edid(udl))
>> +		return connector_status_connected;
>>   
>> -	drm_dev_exit(idx);
>> -	kfree(read_buff);
>> -
>> -	return 0;
>> -
>> -err_drm_dev_exit:
>> -	drm_dev_exit(idx);
>> -err_kfree:
>> -	kfree(read_buff);
>> -	return ret;
>> +	return connector_status_disconnected;
>>   }
>>   
>> -static enum drm_connector_status udl_connector_detect(struct drm_connector *connector, bool force)
>> -{
>> -	struct drm_device *dev = connector->dev;
>> -	struct udl_device *udl = to_udl(dev);
>> -	struct udl_connector *udl_connector = to_udl_connector(connector);
>> -	enum drm_connector_status status = connector_status_disconnected;
>> -
>> -	/* cleanup previous EDID */
>> -	kfree(udl_connector->edid);
>> -	udl_connector->edid = NULL;
>> -
>> -	udl_connector->edid = drm_do_get_edid(connector, udl_get_edid_block, udl);
>> -	if (udl_connector->edid)
>> -		status = connector_status_connected;
>> -
>> -	return status;
>> -}
>> +static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
>> +	.get_modes = udl_connector_helper_get_modes,
>> +	.detect_ctx = udl_connector_helper_detect_ctx,
>> +};
>>   
>>   static void udl_connector_destroy(struct drm_connector *connector)
>>   {
>>   	struct udl_connector *udl_connector = to_udl_connector(connector);
>>   
>>   	drm_connector_cleanup(connector);
>> -	kfree(udl_connector->edid);
>>   	kfree(udl_connector);
>>   }
>>   
>>   static const struct drm_connector_funcs udl_connector_funcs = {
>>   	.reset = drm_atomic_helper_connector_reset,
>> -	.detect = udl_connector_detect,
>>   	.fill_modes = drm_helper_probe_single_connector_modes,
>>   	.destroy = udl_connector_destroy,
>>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)