diff mbox series

[3/8] drm/ast: Move mode-setting code into mode_set_nofb CRTC helper

Message ID 20240627153638.8765-4-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/ast: Untangle the chaos in mode setting | expand

Commit Message

Thomas Zimmermann June 27, 2024, 3:27 p.m. UTC
Do all mode setting in ast_crtc_helper_mode_set_nofb(), which
always runs after disabling the CRTC and before programming the
planes. Removes implicit synchronization between the CRTC's
atomic disable, enable and the vertical retrace.

Display-mode updates require HW cursors to be disabled. The HW
cursor only picks up changes at vertical retrace periods. So the
CRTC's atomic_disable helper waited for the retrace to delay any
following mode-setting operations, which then happened in
atomic_enable. See [1] for a description of the problem.

With the CRTC helper callback mode_set_nofb, we can now synchronize
and reprogram in the same place. As it always runs before the plane
update, the plane code can be reordered with the CRTC's later
atomic_enable et al.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/series/79914/ # 1
---
 drivers/gpu/drm/ast/ast_mode.c | 52 ++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 24 deletions(-)

Comments

Jocelyn Falempe June 28, 2024, 9:48 a.m. UTC | #1
On 27/06/2024 17:27, Thomas Zimmermann wrote:
> Do all mode setting in ast_crtc_helper_mode_set_nofb(), which
> always runs after disabling the CRTC and before programming the
> planes. Removes implicit synchronization between the CRTC's
> atomic disable, enable and the vertical retrace.
> 
> Display-mode updates require HW cursors to be disabled. The HW
> cursor only picks up changes at vertical retrace periods. So the
> CRTC's atomic_disable helper waited for the retrace to delay any
> following mode-setting operations, which then happened in
> atomic_enable. See [1] for a description of the problem.
> 
> With the CRTC helper callback mode_set_nofb, we can now synchronize
> and reprogram in the same place. As it always runs before the plane
> update, the plane code can be reordered with the CRTC's later
> atomic_enable et al.

Thanks, it looks good te me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/series/79914/ # 1
> ---
>   drivers/gpu/drm/ast/ast_mode.c | 52 ++++++++++++++++++----------------
>   1 file changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 421fcdad40e4..e8312b3472ed 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1131,6 +1131,33 @@ ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode
>   	return status;
>   }
>   
> +static void ast_crtc_helper_mode_set_nofb(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct ast_device *ast = to_ast_device(dev);
> +	struct drm_crtc_state *crtc_state = crtc->state;
> +	struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
> +	struct ast_vbios_mode_info *vbios_mode_info =
> +		&ast_crtc_state->vbios_mode_info;
> +	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
> +
> +	/*
> +	 * Ensure that no scanout takes place before reprogramming mode
> +	 * and format registers.
> +	 *
> +	 * TODO: Get vblank interrupts working and remove this line.
> +	 */
> +	ast_wait_for_vretrace(ast);
> +
> +	ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info);
> +	ast_set_index_reg(ast, AST_IO_VGACRI, 0xa1, 0x06);
> +	ast_set_std_reg(ast, adjusted_mode, vbios_mode_info);
> +	ast_set_crtc_reg(ast, adjusted_mode, vbios_mode_info);
> +	ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info);
> +	ast_set_crtthd_reg(ast);
> +	ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info);
> +}
> +
>   static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>   					struct drm_atomic_state *state)
>   {
> @@ -1207,30 +1234,12 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
>   
>   static void ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state)
>   {
> -	struct drm_device *dev = crtc->dev;
> -	struct ast_device *ast = to_ast_device(dev);
> -	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> -	struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
> -	struct ast_vbios_mode_info *vbios_mode_info =
> -		&ast_crtc_state->vbios_mode_info;
> -	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
> -
> -	ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info);
> -	ast_set_index_reg(ast, AST_IO_VGACRI, 0xa1, 0x06);
> -	ast_set_std_reg(ast, adjusted_mode, vbios_mode_info);
> -	ast_set_crtc_reg(ast, adjusted_mode, vbios_mode_info);
> -	ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info);
> -	ast_set_crtthd_reg(ast);
> -	ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info);
> -
>   	ast_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>   }
>   
>   static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
>   {
>   	struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
> -	struct drm_device *dev = crtc->dev;
> -	struct ast_device *ast = to_ast_device(dev);
>   
>   	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
>   
> @@ -1245,16 +1254,11 @@ static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_ato
>   	 * simple pageflips on the planes.
>   	 */
>   	drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false);
> -
> -	/*
> -	 * Ensure that no scanout takes place before reprogramming mode
> -	 * and format registers.
> -	 */
> -	ast_wait_for_vretrace(ast);
>   }
>   
>   static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>   	.mode_valid = ast_crtc_helper_mode_valid,
> +	.mode_set_nofb = ast_crtc_helper_mode_set_nofb,
>   	.atomic_check = ast_crtc_helper_atomic_check,
>   	.atomic_flush = ast_crtc_helper_atomic_flush,
>   	.atomic_enable = ast_crtc_helper_atomic_enable,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 421fcdad40e4..e8312b3472ed 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1131,6 +1131,33 @@  ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode
 	return status;
 }
 
+static void ast_crtc_helper_mode_set_nofb(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct ast_device *ast = to_ast_device(dev);
+	struct drm_crtc_state *crtc_state = crtc->state;
+	struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
+	struct ast_vbios_mode_info *vbios_mode_info =
+		&ast_crtc_state->vbios_mode_info;
+	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
+
+	/*
+	 * Ensure that no scanout takes place before reprogramming mode
+	 * and format registers.
+	 *
+	 * TODO: Get vblank interrupts working and remove this line.
+	 */
+	ast_wait_for_vretrace(ast);
+
+	ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info);
+	ast_set_index_reg(ast, AST_IO_VGACRI, 0xa1, 0x06);
+	ast_set_std_reg(ast, adjusted_mode, vbios_mode_info);
+	ast_set_crtc_reg(ast, adjusted_mode, vbios_mode_info);
+	ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info);
+	ast_set_crtthd_reg(ast);
+	ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info);
+}
+
 static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
 					struct drm_atomic_state *state)
 {
@@ -1207,30 +1234,12 @@  ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 
 static void ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
-	struct drm_device *dev = crtc->dev;
-	struct ast_device *ast = to_ast_device(dev);
-	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
-	struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
-	struct ast_vbios_mode_info *vbios_mode_info =
-		&ast_crtc_state->vbios_mode_info;
-	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
-
-	ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info);
-	ast_set_index_reg(ast, AST_IO_VGACRI, 0xa1, 0x06);
-	ast_set_std_reg(ast, adjusted_mode, vbios_mode_info);
-	ast_set_crtc_reg(ast, adjusted_mode, vbios_mode_info);
-	ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info);
-	ast_set_crtthd_reg(ast);
-	ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info);
-
 	ast_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
 }
 
 static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
 	struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
-	struct drm_device *dev = crtc->dev;
-	struct ast_device *ast = to_ast_device(dev);
 
 	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
 
@@ -1245,16 +1254,11 @@  static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_ato
 	 * simple pageflips on the planes.
 	 */
 	drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false);
-
-	/*
-	 * Ensure that no scanout takes place before reprogramming mode
-	 * and format registers.
-	 */
-	ast_wait_for_vretrace(ast);
 }
 
 static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
 	.mode_valid = ast_crtc_helper_mode_valid,
+	.mode_set_nofb = ast_crtc_helper_mode_set_nofb,
 	.atomic_check = ast_crtc_helper_atomic_check,
 	.atomic_flush = ast_crtc_helper_atomic_flush,
 	.atomic_enable = ast_crtc_helper_atomic_enable,