[PATCH 2/5] drm/mgag200: vga-bmc: Transparently handle BMC

[Thomas Zimmermann]

Hi

Am 27.09.24 um 16:08 schrieb Jani Nikula:
> On Mon, 05 Aug 2024, Thomas Zimmermann <tzimmermann at suse.de> wrote:
>> The VGA-BMC connector selects the VGA output if a display has been
>> attached to the physical connector. Otherwise it selects the BMC
>> output. In any case, the connector status is set to 'detected', so
>> that the userspace compositor displays to it.
>>
>> Depending on the setting, the connector's display modes either come
>> from the VGA monitor's EDID or from an internal list of BMC-compatible
>> modes.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>   drivers/gpu/drm/mgag200/mgag200_vga_bmc.c | 50 ++++++++++++++++++++++-
>>   1 file changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c b/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
>> index b6b90632b3c6..3a958c3587ac 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   
>>   #include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_edid.h>
>>   #include <drm/drm_modeset_helper_vtables.h>
>>   #include <drm/drm_probe_helper.h>
>>   
>> @@ -11,9 +12,54 @@ static const struct drm_encoder_funcs mgag200_dac_encoder_funcs = {
>>   	.destroy = drm_encoder_cleanup
>>   };
>>   
>> +static int mgag200_vga_bmc_connector_helper_get_modes(struct drm_connector *connector)
>> +{
>> +	struct mga_device *mdev = to_mga_device(connector->dev);
>> +	const struct mgag200_device_info *minfo = mdev->info;
>> +	int count;
>> +
>> +	count = drm_connector_helper_get_modes(connector);
>> +
>> +	if (!count) {
>> +		/*
>> +		 * There's no EDID data without a connected monitor. Set BMC-
>> +		 * compatible modes in this case. The XGA default resolution
>> +		 * should work well for all BMCs.
>> +		 */
>> +		count = drm_add_modes_noedid(connector, minfo->max_hdisplay, minfo->max_vdisplay);
>> +		if (count)
>> +			drm_set_preferred_mode(connector, 1024, 768);
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +/*
>> + * There's no monitor connected if the DDC did not return an EDID. Still
>> + * return 'connected' as there's always a BMC. Incrementing the connector's
>> + * epoch counter triggers an update of the related properties.
>> + */
>> +static int mgag200_vga_bmc_connector_helper_detect_ctx(struct drm_connector *connector,
>> +						       struct drm_modeset_acquire_ctx *ctx,
>> +						       bool force)
>> +{
>> +	enum drm_connector_status old_status, status;
>> +
>> +	if (connector->edid_blob_ptr)
> This is now the only place outside of drm_edid.c that uses edid_blob_ptr
> for anything.
>
> Seems like you're using it as a proxy for "had a display connected".
>
> I wish it could be kept private to the EDID code.

No problem. I'd also prefer to change this to work like in ast, [1] 
where that function tests for the connector's 'physical status'.

But I'd like to store the physical status in struct drm_connector and 
have an optional update helper that detects the actual (logical) status, 
as outlined at [2]. Can we talk about that?

Best regards
Thomas

[1] 
https://gitlab.freedesktop.org/drm/kernel/-/blob/ae2c6d8b3b88c176dff92028941a4023f1b4cb91/drivers/gpu/drm/ast/ast_vga.c#L29
[2] 
https://lore.kernel.org/dri-devel/1d16c1ae-2e27-4daa-b8a6-5eab179ef551@suse.de/

>
>
> BR,
> Jani.
>
>
>> +		old_status = connector_status_connected;
>> +	else
>> +		old_status = connector_status_disconnected;
>> +
>> +	status = drm_connector_helper_detect_from_ddc(connector, ctx, force);
>> +
>> +	if (status != old_status)
>> +		++connector->epoch_counter;
>> +	return connector_status_connected;
>> +}
>> +
>>   static const struct drm_connector_helper_funcs mgag200_vga_connector_helper_funcs = {
>> -	.get_modes = drm_connector_helper_get_modes,
>> -	.detect_ctx = drm_connector_helper_detect_from_ddc
>> +	.get_modes = mgag200_vga_bmc_connector_helper_get_modes,
>> +	.detect_ctx = mgag200_vga_bmc_connector_helper_detect_ctx,
>>   };
>>   
>>   static const struct drm_connector_funcs mgag200_vga_connector_funcs = {

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