Re: [PATCH v2 9/9] drm/ast: Remove BMC output

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 13.08.24 um 15:20 schrieb Jocelyn Falempe:
> On 12/08/2024 11:30, Thomas Zimmermann wrote:
> > Ast's BMC connector tracks the status of an underlying physical
> > connector and updates the BMC status accordingly. This functionality
> > works around GNOME's settings app, which cannot handle multiple
> > outputs on the same CRTC.
> >
> > The workaround is now obsolete as all code for physical outputs
> > handle BMC support internally. Hence, remove the driver's code and
> > the BMC output entirely.
> Thanks, it looks good to me.
>
> Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx> +

Thanks for reviewing. If no further comments come in, I'll merge the 
patchset later this week.

Best regards
Thomas

> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > ---
> >   drivers/gpu/drm/ast/ast_drv.h  |   4 --
> >   drivers/gpu/drm/ast/ast_mode.c | 107 ---------------------------------
> >   2 files changed, 111 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> > b/drivers/gpu/drm/ast/ast_drv.h
> > index 3a4f80cb5c0f..a4cde495fde4 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.h
> > +++ b/drivers/gpu/drm/ast/ast_drv.h
> > @@ -206,10 +206,6 @@ struct ast_device {
> >               struct drm_encoder encoder;
> >               struct drm_connector connector;
> >           } astdp;
> > -        struct {
> > -            struct drm_encoder encoder;
> > -            struct ast_bmc_connector bmc_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 d823e9d85b04..ed496fb32bf3 100644
> > --- a/drivers/gpu/drm/ast/ast_mode.c
> > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > @@ -34,10 +34,8 @@
> >     #include <drm/drm_atomic.h>
> >   #include <drm/drm_atomic_helper.h>
> > -#include <drm/drm_atomic_state_helper.h>
> >   #include <drm/drm_crtc.h>
> >   #include <drm/drm_damage_helper.h>
> > -#include <drm/drm_edid.h>
> >   #include <drm/drm_format_helper.h>
> >   #include <drm/drm_fourcc.h>
> >   #include <drm/drm_gem_atomic_helper.h>
> > @@ -1309,103 +1307,6 @@ static int ast_crtc_init(struct drm_device *dev)
> >       return 0;
> >   }
> >   -/*
> > - * BMC virtual Connector
> > - */
> > -
> > -static const struct drm_encoder_funcs ast_bmc_encoder_funcs = {
> > -    .destroy = drm_encoder_cleanup,
> > -};
> > -
> > -static int ast_bmc_connector_helper_detect_ctx(struct drm_connector 
> > *connector,
> > -                           struct drm_modeset_acquire_ctx *ctx,
> > -                           bool force)
> > -{
> > -    struct ast_bmc_connector *bmc_connector = 
> > to_ast_bmc_connector(connector);
> > -    struct drm_connector *physical_connector = 
> > bmc_connector->physical_connector;
> > -
> > -    /*
> > -     * Most user-space compositors cannot handle more than one 
> > connected
> > -     * connector per CRTC. Hence, we only mark the BMC as connected 
> > if the
> > -     * physical connector is disconnected. If the physical 
> > connector's status
> > -     * is connected or unknown, the BMC remains disconnected. This 
> > has no
> > -     * effect on the output of the BMC.
> > -     *
> > -     * FIXME: Remove this logic once user-space compositors can 
> > handle more
> > -     *        than one connector per CRTC. The BMC should always be 
> > connected.
> > -     */
> > -
> > -    if (physical_connector && physical_connector->status == 
> > connector_status_disconnected)
> > -        return connector_status_connected;
> > -
> > -    return connector_status_disconnected;
> > -}
> > -
> > -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,
> > -    .detect_ctx = ast_bmc_connector_helper_detect_ctx,
> > -};
> > -
> > -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 int ast_bmc_connector_init(struct drm_device *dev,
> > -                  struct ast_bmc_connector *bmc_connector,
> > -                  struct drm_connector *physical_connector)
> > -{
> > -    struct drm_connector *connector = &bmc_connector->base;
> > -    int ret;
> > -
> > -    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);
> > -
> > -    bmc_connector->physical_connector = physical_connector;
> > -
> > -    return 0;
> > -}
> > -
> > -static int ast_bmc_output_init(struct ast_device *ast,
> > -                   struct drm_connector *physical_connector)
> > -{
> > -    struct drm_device *dev = &ast->base;
> > -    struct drm_crtc *crtc = &ast->crtc;
> > -    struct drm_encoder *encoder = &ast->output.bmc.encoder;
> > -    struct ast_bmc_connector *bmc_connector = 
> > &ast->output.bmc.bmc_connector;
> > -    struct drm_connector *connector = &bmc_connector->base;
> > -    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 = ast_bmc_connector_init(dev, bmc_connector, 
> > physical_connector);
> > -    if (ret)
> > -        return ret;
> > -
> > -    ret = drm_connector_attach_encoder(connector, encoder);
> > -    if (ret)
> > -        return ret;
> > -
> > -    return 0;
> > -}
> > -
> >   /*
> >    * Mode config
> >    */
> > @@ -1457,7 +1358,6 @@ static const struct drm_mode_config_funcs 
> > ast_mode_config_funcs = {
> >   int ast_mode_config_init(struct ast_device *ast)
> >   {
> >       struct drm_device *dev = &ast->base;
> > -    struct drm_connector *physical_connector = NULL;
> >       int ret;
> >         ret = drmm_mutex_init(dev, &ast->modeset_lock);
> > @@ -1502,29 +1402,22 @@ int ast_mode_config_init(struct ast_device *ast)
> >           ret = ast_vga_output_init(ast);
> >           if (ret)
> >               return ret;
> > -        physical_connector = &ast->output.vga.connector;
> >       }
> >       if (ast->tx_chip_types & AST_TX_SIL164_BIT) {
> >           ret = ast_sil164_output_init(ast);
> >           if (ret)
> >               return ret;
> > -        physical_connector = &ast->output.sil164.connector;
> >       }
> >       if (ast->tx_chip_types & AST_TX_DP501_BIT) {
> >           ret = ast_dp501_output_init(ast);
> >           if (ret)
> >               return ret;
> > -        physical_connector = &ast->output.dp501.connector;
> >       }
> >       if (ast->tx_chip_types & AST_TX_ASTDP_BIT) {
> >           ret = ast_astdp_output_init(ast);
> >           if (ret)
> >               return ret;
> > -        physical_connector = &ast->output.astdp.connector;
> >       }
> > -    ret = ast_bmc_output_init(ast, physical_connector);
> > -    if (ret)
> > -        return ret;
> >         drm_mode_config_reset(dev);

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