| Message ID | 20240805130622.63458-3-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mgag200: Handle BMC in dedicated VGA output | expand |
On 05/08/2024 15:05, Thomas Zimmermann 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. > Thanks for this work. Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com> > Signed-off-by: Thomas Zimmermann <tzimmermann@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) > + 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 = {
On Mon, 05 Aug 2024, Thomas Zimmermann <tzimmermann@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@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. 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 = {
Hi Am 27.09.24 um 16:08 schrieb Jani Nikula: > On Mon, 05 Aug 2024, Thomas Zimmermann <tzimmermann@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@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 = {
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) + 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 = {
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@suse.de> --- drivers/gpu/drm/mgag200/mgag200_vga_bmc.c | 50 ++++++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-)