[PATCH v6 1/2] drm/ast: Add BMC virtual connector

[Thomas Zimmermann]

Hi

Am 26.07.23 um 10:41 schrieb Jocelyn Falempe:
> On 13/07/2023 15:41, Jocelyn Falempe wrote:
>> Most aspeed devices have a BMC, which allows to remotely see the screen.
>> Also in the common use case, those servers don't have a display 
>> connected.
>> So add a Virtual connector, to reflect that even if no display is
>> connected, the framebuffer can still be seen remotely.
>> This prepares the work to implement a detect_ctx() for the Display port
>> connector.
>>
>> v4: call drm_add_modes_noedid() with 4096x4096 (Thomas Zimmermann)
>>      remove useless struct field init to 0 (Thomas Zimmermann)
>>      don't use drm_simple_encoder_init() (Thomas Zimmermann)
>>      inline ast_bmc_connector_init() (Thomas Zimmermann)
>>
>> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID 
>> on DP")
>> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
>> ---
>>   drivers/gpu/drm/ast/ast_drv.h  |  4 +++
>>   drivers/gpu/drm/ast/ast_mode.c | 58 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
>> b/drivers/gpu/drm/ast/ast_drv.h
>> index 3f6e0c984523..c9659e72002f 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -214,6 +214,10 @@ struct ast_device {
>>               struct drm_encoder encoder;
>>               struct drm_connector connector;
>>           } astdp;
>> +        struct {
>> +            struct drm_encoder encoder;
>> +            struct drm_connector connector;
>> +        } bmc;
>>       } output;
>>       bool support_wide_screen;
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>> b/drivers/gpu/drm/ast/ast_mode.c
>> index f711d592da52..1a8293162fec 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -1735,6 +1735,61 @@ static int ast_astdp_output_init(struct 
>> ast_device *ast)
>>       return 0;
>>   }
>> +/*
>> + * BMC virtual Connector
>> + */
>> +
>> +static int ast_bmc_connector_helper_get_modes(struct drm_connector 
>> *connector)
>> +{
>> +    return drm_add_modes_noedid(connector, 4096, 4096);
>> +}
>> +
>> +static const struct drm_connector_helper_funcs 
>> ast_bmc_connector_helper_funcs = {
>> +    .get_modes = ast_bmc_connector_helper_get_modes,
>> +};
>> +
>> +static const struct drm_connector_funcs ast_bmc_connector_funcs = {
>> +    .reset = drm_atomic_helper_connector_reset,
>> +    .fill_modes = drm_helper_probe_single_connector_modes,
>> +    .destroy = drm_connector_cleanup,
>> +    .atomic_duplicate_state = 
>> drm_atomic_helper_connector_duplicate_state,
>> +    .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static const struct drm_encoder_funcs ast_bmc_encoder_funcs = {
>> +    .destroy = drm_encoder_cleanup,
>> +};

Can you please move the encoder_funcs struct before the connector stuff?

>> +
>> +static int ast_bmc_output_init(struct ast_device *ast)
>> +{
>> +    struct drm_device *dev = &ast->base;
>> +    struct drm_crtc *crtc = &ast->crtc;
>> +    struct drm_encoder *encoder = &ast->output.bmc.encoder;
>> +    struct drm_connector *connector = &ast->output.bmc.connector;
>> +    int ret;
>> +
>> +
>> +    ret = drm_encoder_init(dev, encoder,
>> +                &ast_bmc_encoder_funcs,
>> +                DRM_MODE_ENCODER_VIRTUAL, "ast_bmc");
>> +    if (ret)
>> +        return ret;
>> +    encoder->possible_crtcs = drm_crtc_mask(crtc);
>> +
>> +    ret = drm_connector_init(dev, connector, &ast_bmc_connector_funcs,
>> +                 DRM_MODE_CONNECTOR_VIRTUAL);
>> +    if (ret)
>> +        return ret;
>> +
>> +    drm_connector_helper_add(connector, 
>> &ast_bmc_connector_helper_funcs);
>> +
>> +    ret = drm_connector_attach_encoder(connector, encoder);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return 0;
>> +}
>> +
>>   /*
>>    * Mode config
>>    */
>> @@ -1842,6 +1897,9 @@ int ast_mode_config_init(struct ast_device *ast)
>>           if (ret)
>>               return ret;
>>       }
>> +    ret = ast_bmc_output_init(ast);
>> +    if (ret)
>> +        return ret;
>>       drm_mode_config_reset(dev);
>>
>> base-commit: b32d5a51f3c21843011d68a58e6ac0b897bce9f2
> 
> 
> I'm missing a review on this patch. The VGA CRD0[7] register check 
> doesn't work on my server, so I'm adding the BMC virtual connector for 
> all aspeed hardware, but as discussed, it shouldn't have a negative impact.

Sorry. I simply missed this patch. With my little nitpick fixed, you can add

Reviewed-by: Thomas Zimmermann <tzimmermann at suse.de>

Thanks for the fix.

Best regards
Thomas

> 
> Thanks,
> 

-- 
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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230726/9fe780ef/attachment.sig>