Re: [PATCH 4/5] drm/mgag200: vga-bmc: Control BMC scanout from encoder

[Jocelyn Falempe <jfalempe@xxxxxxxxxx>]

On 05/08/2024 15:06, Thomas Zimmermann wrote:
> Move calls to stop and start BMC scanout from CRTC helpers to the
> VGA-BMC encoder's atomic_disable and atomic_enable. Makes the BMC
> scanout transparent to the CRTC.
>
> DRM's atomic helpers call an encoder's atomic_disable and atomic_enable
> helpers for all enabled encoders. The BMC stops scanning out the VGA
> signal if modeset disables the VGA encoder, and starts scanning out
> if the modeset enables the VGA encoder.

Thanks for this work.

Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>

> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
>   drivers/gpu/drm/mgag200/mgag200_g200er.c  |  3 ---
>   drivers/gpu/drm/mgag200/mgag200_g200ev.c  |  3 ---
>   drivers/gpu/drm/mgag200/mgag200_g200se.c  |  3 ---
>   drivers/gpu/drm/mgag200/mgag200_mode.c    |  6 ------
>   drivers/gpu/drm/mgag200/mgag200_vga_bmc.c | 20 ++++++++++++++++++++
>   5 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> index e0f7816ad87a..b99b308a7e54 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> @@ -207,9 +207,6 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   
>   	mgag200_enable_display(mdev);
>   
> -	if (mdev->info->sync_bmc)
> -		mgag200_bmc_start_scanout(mdev);
> -
>   	drm_crtc_vblank_on(crtc);
>   }
>   
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> index a2c683f82127..717e4357adcc 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> @@ -208,9 +208,6 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   
>   	mgag200_enable_display(mdev);
>   
> -	if (mdev->info->sync_bmc)
> -		mgag200_bmc_start_scanout(mdev);
> -
>   	drm_crtc_vblank_on(crtc);
>   }
>   
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> index b83f3f31ed0e..0db7e14e3328 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> @@ -339,9 +339,6 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   
>   	mgag200_enable_display(mdev);
>   
> -	if (mdev->info->sync_bmc)
> -		mgag200_bmc_start_scanout(mdev);
> -
>   	drm_crtc_vblank_on(crtc);
>   }
>   
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 8dc16821e1cd..7159909aca1e 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -693,9 +693,6 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
>   
>   	mgag200_enable_display(mdev);
>   
> -	if (mdev->info->sync_bmc)
> -		mgag200_bmc_start_scanout(mdev);
> -
>   	drm_crtc_vblank_on(crtc);
>   }
>   
> @@ -705,9 +702,6 @@ void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic
>   
>   	drm_crtc_vblank_off(crtc);
>   
> -	if (mdev->info->sync_bmc)
> -		mgag200_bmc_stop_scanout(mdev);
> -
>   	mgag200_disable_display(mdev);
>   }
>   
> diff --git a/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c b/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
> index 77340f2dee17..a5a3ac108bd5 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
> @@ -8,6 +8,24 @@
>   #include "mgag200_ddc.h"
>   #include "mgag200_drv.h"
>   
> +static void mgag200_vga_bmc_encoder_atomic_disable(struct drm_encoder *encoder,
> +						   struct drm_atomic_state *state)
> +{
> +	struct mga_device *mdev = to_mga_device(encoder->dev);
> +
> +	if (mdev->info->sync_bmc)
> +		mgag200_bmc_stop_scanout(mdev);
> +}
> +
> +static void mgag200_vga_bmc_encoder_atomic_enable(struct drm_encoder *encoder,
> +						  struct drm_atomic_state *state)
> +{
> +	struct mga_device *mdev = to_mga_device(encoder->dev);
> +
> +	if (mdev->info->sync_bmc)
> +		mgag200_bmc_start_scanout(mdev);
> +}
> +
>   static int mgag200_vga_bmc_encoder_atomic_check(struct drm_encoder *encoder,
>   						struct drm_crtc_state *new_crtc_state,
>   						struct drm_connector_state *new_connector_state)
> @@ -21,6 +39,8 @@ static int mgag200_vga_bmc_encoder_atomic_check(struct drm_encoder *encoder,
>   }
>   
>   static const struct drm_encoder_helper_funcs mgag200_dac_encoder_helper_funcs = {
> +	.atomic_disable = mgag200_vga_bmc_encoder_atomic_disable,
> +	.atomic_enable = mgag200_vga_bmc_encoder_atomic_enable,
>   	.atomic_check = mgag200_vga_bmc_encoder_atomic_check,
>   };
>