diff mbox series

[9/9] drm/ast: Enable atomic modesetting

Message ID 20191028154928.4058-10-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/ast: Convert to atomic modesetting | expand

Commit Message

Thomas Zimmermann Oct. 28, 2019, 3:49 p.m. UTC
This commit sets the remaining atomic-modesetting helpers and the flag
DRIVER_ATOMIC. Legacy cursor functions are removed in favor of the cursor
plane. For power management, atomic helpers replace the indvidual
operations that the driver currently runs.

Atomic modesetting is enabled with this commit.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.c  |  24 ++-
 drivers/gpu/drm/ast/ast_main.c |   5 +
 drivers/gpu/drm/ast/ast_mode.c | 290 ++-------------------------------
 3 files changed, 33 insertions(+), 286 deletions(-)

Comments

Gerd Hoffmann Nov. 5, 2019, 9:57 a.m. UTC | #1
On Mon, Oct 28, 2019 at 04:49:28PM +0100, Thomas Zimmermann wrote:
> This commit sets the remaining atomic-modesetting helpers and the flag
> DRIVER_ATOMIC. Legacy cursor functions are removed in favor of the cursor
> plane. For power management, atomic helpers replace the indvidual
> operations that the driver currently runs.
> 
> Atomic modesetting is enabled with this commit.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/ast/ast_drv.c  |  24 ++-
>  drivers/gpu/drm/ast/ast_main.c |   5 +
>  drivers/gpu/drm/ast/ast_mode.c | 290 ++-------------------------------
>  3 files changed, 33 insertions(+), 286 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 1f17794b0890..d763da6f0834 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -99,14 +99,14 @@ ast_pci_remove(struct pci_dev *pdev)
>  	drm_put_dev(dev);
>  }
>  
> -
> -
>  static int ast_drm_freeze(struct drm_device *dev)
>  {
> -	drm_kms_helper_poll_disable(dev);
> -	pci_save_state(dev->pdev);
> -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
> +	int error;
>  
> +	error = drm_mode_config_helper_suspend(dev);
> +	if (error)
> +		return error;
> +	pci_save_state(dev->pdev);
>  	return 0;
>  }
>  
> @@ -114,11 +114,7 @@ static int ast_drm_thaw(struct drm_device *dev)
>  {
>  	ast_post_gpu(dev);
>  
> -	drm_mode_config_reset(dev);
> -	drm_helper_resume_force_mode(dev);
> -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
> -
> -	return 0;
> +	return drm_mode_config_helper_resume(dev);
>  }
>  
>  static int ast_drm_resume(struct drm_device *dev)
> @@ -131,8 +127,6 @@ static int ast_drm_resume(struct drm_device *dev)
>  	ret = ast_drm_thaw(dev);
>  	if (ret)
>  		return ret;
> -
> -	drm_kms_helper_poll_enable(dev);
>  	return 0;
>  }
>  
> @@ -150,6 +144,7 @@ static int ast_pm_suspend(struct device *dev)
>  	pci_set_power_state(pdev, PCI_D3hot);
>  	return 0;
>  }
> +
>  static int ast_pm_resume(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> @@ -165,7 +160,6 @@ static int ast_pm_freeze(struct device *dev)
>  	if (!ddev || !ddev->dev_private)
>  		return -ENODEV;
>  	return ast_drm_freeze(ddev);
> -
>  }
>  
>  static int ast_pm_thaw(struct device *dev)
> @@ -203,7 +197,9 @@ static struct pci_driver ast_pci_driver = {
>  DEFINE_DRM_GEM_FOPS(ast_fops);
>  
>  static struct drm_driver driver = {
> -	.driver_features = DRIVER_MODESET | DRIVER_GEM,
> +	.driver_features = DRIVER_ATOMIC |
> +			   DRIVER_GEM |
> +			   DRIVER_MODESET,
>  
>  	.load = ast_driver_load,
>  	.unload = ast_driver_unload,
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 48d57ab42955..b79f484e9bd2 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -28,6 +28,7 @@
>  
>  #include <linux/pci.h>
>  
> +#include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem.h>
> @@ -412,6 +413,8 @@ enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
>  static const struct drm_mode_config_funcs ast_mode_funcs = {
>  	.fb_create = drm_gem_fb_create,
>  	.mode_valid = ast_mode_config_mode_valid,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
>  };
>  
>  static u32 ast_get_vram_info(struct drm_device *dev)
> @@ -529,6 +532,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>  	if (ret)
>  		goto out_free;
>  
> +	drm_mode_config_reset(dev);
> +
>  	ret = drm_fbdev_generic_setup(dev, 32);
>  	if (ret)
>  		goto out_free;
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index f5f73200e8e4..5eccb6ae2ede 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -45,11 +45,6 @@
>  
>  static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
>  static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
> -static int ast_cursor_set(struct drm_crtc *crtc,
> -			  struct drm_file *file_priv,
> -			  uint32_t handle,
> -			  uint32_t width,
> -			  uint32_t height);
>  static int ast_cursor_move(struct drm_crtc *crtc,
>  			   int x, int y);
>  
> @@ -58,9 +53,6 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
>  static int ast_cursor_update(void *dst, void *src, unsigned int width,
>  			     unsigned int height);
>  static void ast_cursor_set_base(struct ast_private *ast, u64 address);
> -static int ast_show_cursor(struct drm_crtc *crtc, void *src,
> -			   unsigned int width, unsigned int height);
> -static void ast_hide_cursor(struct drm_crtc *crtc);
>  static int ast_cursor_move(struct drm_crtc *crtc,
>  			   int x, int y);
>  
> @@ -434,7 +426,7 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
>  static void ast_set_offset_reg(struct drm_crtc *crtc)
>  {
>  	struct ast_private *ast = crtc->dev->dev_private;
> -	const struct drm_framebuffer *fb = crtc->primary->fb;
> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
>  
>  	u16 offset;
>  
> @@ -528,7 +520,7 @@ static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
>  static bool ast_set_dac_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  		     struct ast_vbios_mode_info *vbios_mode)
>  {
> -	const struct drm_framebuffer *fb = crtc->primary->fb;
> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
>  
>  	switch (fb->format->cpp[0] * 8) {
>  	case 8:
> @@ -765,112 +757,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>  	}
>  }
>  
> -static int ast_crtc_do_set_base(struct drm_crtc *crtc,
> -				struct drm_framebuffer *fb,
> -				int x, int y, int atomic)
> -{
> -	struct drm_gem_vram_object *gbo;
> -	int ret;
> -	s64 gpu_addr;
> -
> -	if (!atomic && fb) {
> -		gbo = drm_gem_vram_of_gem(fb->obj[0]);
> -		drm_gem_vram_unpin(gbo);
> -	}
> -
> -	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
> -
> -	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> -	if (ret)
> -		return ret;
> -	gpu_addr = drm_gem_vram_offset(gbo);
> -	if (gpu_addr < 0) {
> -		ret = (int)gpu_addr;
> -		goto err_drm_gem_vram_unpin;
> -	}
> -
> -	ast_set_offset_reg(crtc);
> -	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
> -
> -	return 0;
> -
> -err_drm_gem_vram_unpin:
> -	drm_gem_vram_unpin(gbo);
> -	return ret;
> -}
> -
> -static int ast_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
> -			     struct drm_framebuffer *old_fb)
> -{
> -	return ast_crtc_do_set_base(crtc, old_fb, x, y, 0);
> -}
> -
> -static int ast_crtc_mode_set(struct drm_crtc *crtc,
> -			     struct drm_display_mode *mode,
> -			     struct drm_display_mode *adjusted_mode,
> -			     int x, int y,
> -			     struct drm_framebuffer *old_fb)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct ast_private *ast = crtc->dev->dev_private;
> -	const struct drm_framebuffer *fb = crtc->primary->fb;
> -	struct ast_vbios_mode_info vbios_mode;
> -	bool succ;
> -
> -	if (ast->chip == AST1180) {
> -		DRM_ERROR("AST 1180 modesetting not supported\n");
> -		return -EINVAL;
> -	}
> -
> -	succ = ast_get_vbios_mode_info(fb, mode, adjusted_mode, &vbios_mode);
> -	if (!succ)
> -		return -EINVAL;
> -
> -	ast_open_key(ast);
> -
> -	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
> -	ast_set_vbios_mode_reg(crtc, adjusted_mode, &vbios_mode);
> -	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
> -	ast_set_std_reg(crtc, adjusted_mode, &vbios_mode);
> -	ast_set_crtc_reg(crtc, adjusted_mode, &vbios_mode);
> -	ast_set_offset_reg(crtc);
> -	ast_set_dclk_reg(dev, adjusted_mode, &vbios_mode);
> -	ast_set_color_reg(crtc, fb);
> -	ast_set_crtthd_reg(crtc);
> -	ast_set_sync_reg(dev, adjusted_mode, &vbios_mode);
> -	ast_set_dac_reg(crtc, adjusted_mode, &vbios_mode);
> -
> -	ast_crtc_mode_set_base(crtc, x, y, old_fb);
> -
> -	return 0;
> -}
> -
> -static void ast_crtc_disable(struct drm_crtc *crtc)
> -{
> -	DRM_DEBUG_KMS("\n");
> -	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
> -	if (crtc->primary->fb) {
> -		struct drm_framebuffer *fb = crtc->primary->fb;
> -		struct drm_gem_vram_object *gbo =
> -			drm_gem_vram_of_gem(fb->obj[0]);
> -
> -		drm_gem_vram_unpin(gbo);
> -	}
> -	crtc->primary->fb = NULL;
> -}
> -
> -static void ast_crtc_prepare(struct drm_crtc *crtc)
> -{
> -
> -}
> -
> -static void ast_crtc_commit(struct drm_crtc *crtc)
> -{
> -	struct ast_private *ast = crtc->dev->dev_private;
> -	ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0);
> -	ast_crtc_load_lut(crtc);
> -}
> -
>  static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>  					struct drm_crtc_state *state)
>  {
> @@ -970,12 +856,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
> -	.dpms = ast_crtc_dpms,
> -	.mode_set = ast_crtc_mode_set,
> -	.mode_set_base = ast_crtc_mode_set_base,
> -	.disable = ast_crtc_disable,
> -	.prepare = ast_crtc_prepare,
> -	.commit = ast_crtc_commit,
>  	.atomic_check = ast_crtc_helper_atomic_check,
>  	.atomic_begin = ast_crtc_helper_atomic_begin,
>  	.atomic_flush = ast_crtc_helper_atomic_flush,
> @@ -983,21 +863,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>  	.atomic_disable = ast_crtc_helper_atomic_disable,
>  };
>  
> -static void ast_crtc_reset(struct drm_crtc *crtc)
> -{
> -
> -}
> -
> -static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, uint32_t size,
> -			      struct drm_modeset_acquire_ctx *ctx)
> -{
> -	ast_crtc_load_lut(crtc);
> -
> -	return 0;
> -}
> -
> -
>  static void ast_crtc_destroy(struct drm_crtc *crtc)
>  {
>  	drm_crtc_cleanup(crtc);
> @@ -1005,12 +870,12 @@ static void ast_crtc_destroy(struct drm_crtc *crtc)
>  }
>  
>  static const struct drm_crtc_funcs ast_crtc_funcs = {
> -	.cursor_set = ast_cursor_set,
> -	.cursor_move = ast_cursor_move,
> -	.reset = ast_crtc_reset,
> +	.reset = drm_atomic_helper_crtc_reset,
>  	.set_config = drm_crtc_helper_set_config,
> -	.gamma_set = ast_crtc_gamma_set,
> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.destroy = ast_crtc_destroy,
> +	.set_config = drm_atomic_helper_set_config,
> +	.page_flip = drm_atomic_helper_page_flip,
>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  };
> @@ -1040,6 +905,10 @@ static int ast_crtc_init(struct drm_device *dev)
>  	return ret;
>  }
>  
> +/*
> + * Encoder
> + */
> +
>  static void ast_encoder_destroy(struct drm_encoder *encoder)
>  {
>  	drm_encoder_cleanup(encoder);
> @@ -1050,34 +919,6 @@ static const struct drm_encoder_funcs ast_enc_funcs = {
>  	.destroy = ast_encoder_destroy,
>  };
>  
> -static void ast_encoder_dpms(struct drm_encoder *encoder, int mode)
> -{
> -
> -}
> -
> -static void ast_encoder_mode_set(struct drm_encoder *encoder,
> -			       struct drm_display_mode *mode,
> -			       struct drm_display_mode *adjusted_mode)
> -{
> -}
> -
> -static void ast_encoder_prepare(struct drm_encoder *encoder)
> -{
> -
> -}
> -
> -static void ast_encoder_commit(struct drm_encoder *encoder)
> -{
> -
> -}
> -
> -static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
> -	.dpms = ast_encoder_dpms,
> -	.prepare = ast_encoder_prepare,
> -	.commit = ast_encoder_commit,
> -	.mode_set = ast_encoder_mode_set,
> -};

Hmm.  Pretty much a dummy encoder implementation.  Maybe ast is simple
enough that the simple pipe helpers can do the trick?

cheers,
  Gerd
Daniel Vetter Nov. 5, 2019, 10:31 a.m. UTC | #2
On Tue, Nov 05, 2019 at 10:57:11AM +0100, Gerd Hoffmann wrote:
> On Mon, Oct 28, 2019 at 04:49:28PM +0100, Thomas Zimmermann wrote:
> > This commit sets the remaining atomic-modesetting helpers and the flag
> > DRIVER_ATOMIC. Legacy cursor functions are removed in favor of the cursor
> > plane. For power management, atomic helpers replace the indvidual
> > operations that the driver currently runs.
> > 
> > Atomic modesetting is enabled with this commit.
> > 
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > ---
> >  drivers/gpu/drm/ast/ast_drv.c  |  24 ++-
> >  drivers/gpu/drm/ast/ast_main.c |   5 +
> >  drivers/gpu/drm/ast/ast_mode.c | 290 ++-------------------------------
> >  3 files changed, 33 insertions(+), 286 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> > index 1f17794b0890..d763da6f0834 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.c
> > +++ b/drivers/gpu/drm/ast/ast_drv.c
> > @@ -99,14 +99,14 @@ ast_pci_remove(struct pci_dev *pdev)
> >  	drm_put_dev(dev);
> >  }
> >  
> > -
> > -
> >  static int ast_drm_freeze(struct drm_device *dev)
> >  {
> > -	drm_kms_helper_poll_disable(dev);
> > -	pci_save_state(dev->pdev);
> > -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
> > +	int error;
> >  
> > +	error = drm_mode_config_helper_suspend(dev);
> > +	if (error)
> > +		return error;
> > +	pci_save_state(dev->pdev);
> >  	return 0;
> >  }
> >  
> > @@ -114,11 +114,7 @@ static int ast_drm_thaw(struct drm_device *dev)
> >  {
> >  	ast_post_gpu(dev);
> >  
> > -	drm_mode_config_reset(dev);
> > -	drm_helper_resume_force_mode(dev);
> > -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
> > -
> > -	return 0;
> > +	return drm_mode_config_helper_resume(dev);
> >  }
> >  
> >  static int ast_drm_resume(struct drm_device *dev)
> > @@ -131,8 +127,6 @@ static int ast_drm_resume(struct drm_device *dev)
> >  	ret = ast_drm_thaw(dev);
> >  	if (ret)
> >  		return ret;
> > -
> > -	drm_kms_helper_poll_enable(dev);
> >  	return 0;
> >  }
> >  
> > @@ -150,6 +144,7 @@ static int ast_pm_suspend(struct device *dev)
> >  	pci_set_power_state(pdev, PCI_D3hot);
> >  	return 0;
> >  }
> > +
> >  static int ast_pm_resume(struct device *dev)
> >  {
> >  	struct pci_dev *pdev = to_pci_dev(dev);
> > @@ -165,7 +160,6 @@ static int ast_pm_freeze(struct device *dev)
> >  	if (!ddev || !ddev->dev_private)
> >  		return -ENODEV;
> >  	return ast_drm_freeze(ddev);
> > -
> >  }
> >  
> >  static int ast_pm_thaw(struct device *dev)
> > @@ -203,7 +197,9 @@ static struct pci_driver ast_pci_driver = {
> >  DEFINE_DRM_GEM_FOPS(ast_fops);
> >  
> >  static struct drm_driver driver = {
> > -	.driver_features = DRIVER_MODESET | DRIVER_GEM,
> > +	.driver_features = DRIVER_ATOMIC |
> > +			   DRIVER_GEM |
> > +			   DRIVER_MODESET,
> >  
> >  	.load = ast_driver_load,
> >  	.unload = ast_driver_unload,
> > diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> > index 48d57ab42955..b79f484e9bd2 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -28,6 +28,7 @@
> >  
> >  #include <linux/pci.h>
> >  
> > +#include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_fb_helper.h>
> >  #include <drm/drm_gem.h>
> > @@ -412,6 +413,8 @@ enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
> >  static const struct drm_mode_config_funcs ast_mode_funcs = {
> >  	.fb_create = drm_gem_fb_create,
> >  	.mode_valid = ast_mode_config_mode_valid,
> > +	.atomic_check = drm_atomic_helper_check,
> > +	.atomic_commit = drm_atomic_helper_commit,
> >  };
> >  
> >  static u32 ast_get_vram_info(struct drm_device *dev)
> > @@ -529,6 +532,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> >  	if (ret)
> >  		goto out_free;
> >  
> > +	drm_mode_config_reset(dev);
> > +
> >  	ret = drm_fbdev_generic_setup(dev, 32);
> >  	if (ret)
> >  		goto out_free;
> > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> > index f5f73200e8e4..5eccb6ae2ede 100644
> > --- a/drivers/gpu/drm/ast/ast_mode.c
> > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > @@ -45,11 +45,6 @@
> >  
> >  static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
> >  static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
> > -static int ast_cursor_set(struct drm_crtc *crtc,
> > -			  struct drm_file *file_priv,
> > -			  uint32_t handle,
> > -			  uint32_t width,
> > -			  uint32_t height);
> >  static int ast_cursor_move(struct drm_crtc *crtc,
> >  			   int x, int y);
> >  
> > @@ -58,9 +53,6 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
> >  static int ast_cursor_update(void *dst, void *src, unsigned int width,
> >  			     unsigned int height);
> >  static void ast_cursor_set_base(struct ast_private *ast, u64 address);
> > -static int ast_show_cursor(struct drm_crtc *crtc, void *src,
> > -			   unsigned int width, unsigned int height);
> > -static void ast_hide_cursor(struct drm_crtc *crtc);
> >  static int ast_cursor_move(struct drm_crtc *crtc,
> >  			   int x, int y);
> >  
> > @@ -434,7 +426,7 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
> >  static void ast_set_offset_reg(struct drm_crtc *crtc)
> >  {
> >  	struct ast_private *ast = crtc->dev->dev_private;
> > -	const struct drm_framebuffer *fb = crtc->primary->fb;
> > +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
> >  
> >  	u16 offset;
> >  
> > @@ -528,7 +520,7 @@ static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
> >  static bool ast_set_dac_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
> >  		     struct ast_vbios_mode_info *vbios_mode)
> >  {
> > -	const struct drm_framebuffer *fb = crtc->primary->fb;
> > +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
> >  
> >  	switch (fb->format->cpp[0] * 8) {
> >  	case 8:
> > @@ -765,112 +757,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> >  	}
> >  }
> >  
> > -static int ast_crtc_do_set_base(struct drm_crtc *crtc,
> > -				struct drm_framebuffer *fb,
> > -				int x, int y, int atomic)
> > -{
> > -	struct drm_gem_vram_object *gbo;
> > -	int ret;
> > -	s64 gpu_addr;
> > -
> > -	if (!atomic && fb) {
> > -		gbo = drm_gem_vram_of_gem(fb->obj[0]);
> > -		drm_gem_vram_unpin(gbo);
> > -	}
> > -
> > -	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
> > -
> > -	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> > -	if (ret)
> > -		return ret;
> > -	gpu_addr = drm_gem_vram_offset(gbo);
> > -	if (gpu_addr < 0) {
> > -		ret = (int)gpu_addr;
> > -		goto err_drm_gem_vram_unpin;
> > -	}
> > -
> > -	ast_set_offset_reg(crtc);
> > -	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
> > -
> > -	return 0;
> > -
> > -err_drm_gem_vram_unpin:
> > -	drm_gem_vram_unpin(gbo);
> > -	return ret;
> > -}
> > -
> > -static int ast_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
> > -			     struct drm_framebuffer *old_fb)
> > -{
> > -	return ast_crtc_do_set_base(crtc, old_fb, x, y, 0);
> > -}
> > -
> > -static int ast_crtc_mode_set(struct drm_crtc *crtc,
> > -			     struct drm_display_mode *mode,
> > -			     struct drm_display_mode *adjusted_mode,
> > -			     int x, int y,
> > -			     struct drm_framebuffer *old_fb)
> > -{
> > -	struct drm_device *dev = crtc->dev;
> > -	struct ast_private *ast = crtc->dev->dev_private;
> > -	const struct drm_framebuffer *fb = crtc->primary->fb;
> > -	struct ast_vbios_mode_info vbios_mode;
> > -	bool succ;
> > -
> > -	if (ast->chip == AST1180) {
> > -		DRM_ERROR("AST 1180 modesetting not supported\n");
> > -		return -EINVAL;
> > -	}
> > -
> > -	succ = ast_get_vbios_mode_info(fb, mode, adjusted_mode, &vbios_mode);
> > -	if (!succ)
> > -		return -EINVAL;
> > -
> > -	ast_open_key(ast);
> > -
> > -	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
> > -	ast_set_vbios_mode_reg(crtc, adjusted_mode, &vbios_mode);
> > -	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
> > -	ast_set_std_reg(crtc, adjusted_mode, &vbios_mode);
> > -	ast_set_crtc_reg(crtc, adjusted_mode, &vbios_mode);
> > -	ast_set_offset_reg(crtc);
> > -	ast_set_dclk_reg(dev, adjusted_mode, &vbios_mode);
> > -	ast_set_color_reg(crtc, fb);
> > -	ast_set_crtthd_reg(crtc);
> > -	ast_set_sync_reg(dev, adjusted_mode, &vbios_mode);
> > -	ast_set_dac_reg(crtc, adjusted_mode, &vbios_mode);
> > -
> > -	ast_crtc_mode_set_base(crtc, x, y, old_fb);
> > -
> > -	return 0;
> > -}
> > -
> > -static void ast_crtc_disable(struct drm_crtc *crtc)
> > -{
> > -	DRM_DEBUG_KMS("\n");
> > -	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
> > -	if (crtc->primary->fb) {
> > -		struct drm_framebuffer *fb = crtc->primary->fb;
> > -		struct drm_gem_vram_object *gbo =
> > -			drm_gem_vram_of_gem(fb->obj[0]);
> > -
> > -		drm_gem_vram_unpin(gbo);
> > -	}
> > -	crtc->primary->fb = NULL;
> > -}
> > -
> > -static void ast_crtc_prepare(struct drm_crtc *crtc)
> > -{
> > -
> > -}
> > -
> > -static void ast_crtc_commit(struct drm_crtc *crtc)
> > -{
> > -	struct ast_private *ast = crtc->dev->dev_private;
> > -	ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0);
> > -	ast_crtc_load_lut(crtc);
> > -}
> > -
> >  static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> >  					struct drm_crtc_state *state)
> >  {
> > @@ -970,12 +856,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> >  }
> >  
> >  static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
> > -	.dpms = ast_crtc_dpms,
> > -	.mode_set = ast_crtc_mode_set,
> > -	.mode_set_base = ast_crtc_mode_set_base,
> > -	.disable = ast_crtc_disable,
> > -	.prepare = ast_crtc_prepare,
> > -	.commit = ast_crtc_commit,
> >  	.atomic_check = ast_crtc_helper_atomic_check,
> >  	.atomic_begin = ast_crtc_helper_atomic_begin,
> >  	.atomic_flush = ast_crtc_helper_atomic_flush,
> > @@ -983,21 +863,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
> >  	.atomic_disable = ast_crtc_helper_atomic_disable,
> >  };
> >  
> > -static void ast_crtc_reset(struct drm_crtc *crtc)
> > -{
> > -
> > -}
> > -
> > -static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> > -			      u16 *blue, uint32_t size,
> > -			      struct drm_modeset_acquire_ctx *ctx)
> > -{
> > -	ast_crtc_load_lut(crtc);
> > -
> > -	return 0;
> > -}
> > -
> > -
> >  static void ast_crtc_destroy(struct drm_crtc *crtc)
> >  {
> >  	drm_crtc_cleanup(crtc);
> > @@ -1005,12 +870,12 @@ static void ast_crtc_destroy(struct drm_crtc *crtc)
> >  }
> >  
> >  static const struct drm_crtc_funcs ast_crtc_funcs = {
> > -	.cursor_set = ast_cursor_set,
> > -	.cursor_move = ast_cursor_move,
> > -	.reset = ast_crtc_reset,
> > +	.reset = drm_atomic_helper_crtc_reset,
> >  	.set_config = drm_crtc_helper_set_config,
> > -	.gamma_set = ast_crtc_gamma_set,
> > +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> >  	.destroy = ast_crtc_destroy,
> > +	.set_config = drm_atomic_helper_set_config,
> > +	.page_flip = drm_atomic_helper_page_flip,
> >  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> >  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> >  };
> > @@ -1040,6 +905,10 @@ static int ast_crtc_init(struct drm_device *dev)
> >  	return ret;
> >  }
> >  
> > +/*
> > + * Encoder
> > + */
> > +
> >  static void ast_encoder_destroy(struct drm_encoder *encoder)
> >  {
> >  	drm_encoder_cleanup(encoder);
> > @@ -1050,34 +919,6 @@ static const struct drm_encoder_funcs ast_enc_funcs = {
> >  	.destroy = ast_encoder_destroy,
> >  };
> >  
> > -static void ast_encoder_dpms(struct drm_encoder *encoder, int mode)
> > -{
> > -
> > -}
> > -
> > -static void ast_encoder_mode_set(struct drm_encoder *encoder,
> > -			       struct drm_display_mode *mode,
> > -			       struct drm_display_mode *adjusted_mode)
> > -{
> > -}
> > -
> > -static void ast_encoder_prepare(struct drm_encoder *encoder)
> > -{
> > -
> > -}
> > -
> > -static void ast_encoder_commit(struct drm_encoder *encoder)
> > -{
> > -
> > -}
> > -
> > -static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
> > -	.dpms = ast_encoder_dpms,
> > -	.prepare = ast_encoder_prepare,
> > -	.commit = ast_encoder_commit,
> > -	.mode_set = ast_encoder_mode_set,
> > -};
> 
> Hmm.  Pretty much a dummy encoder implementation.  Maybe ast is simple
> enough that the simple pipe helpers can do the trick?

simple pipe doesn't do multi-plane, so not applicable. Maybe there's a few
more things we can for dummy encoders (like default cleanup perhaps?).
-Daniel

> 
> cheers,
>   Gerd
>
Thomas Zimmermann Nov. 6, 2019, 8:28 a.m. UTC | #3
Hi

Am 05.11.19 um 11:31 schrieb Daniel Vetter:
> On Tue, Nov 05, 2019 at 10:57:11AM +0100, Gerd Hoffmann wrote:
>> On Mon, Oct 28, 2019 at 04:49:28PM +0100, Thomas Zimmermann wrote:
>>> This commit sets the remaining atomic-modesetting helpers and the flag
>>> DRIVER_ATOMIC. Legacy cursor functions are removed in favor of the cursor
>>> plane. For power management, atomic helpers replace the indvidual
>>> operations that the driver currently runs.
>>>
>>> Atomic modesetting is enabled with this commit.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>  drivers/gpu/drm/ast/ast_drv.c  |  24 ++-
>>>  drivers/gpu/drm/ast/ast_main.c |   5 +
>>>  drivers/gpu/drm/ast/ast_mode.c | 290 ++-------------------------------
>>>  3 files changed, 33 insertions(+), 286 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
>>> index 1f17794b0890..d763da6f0834 100644
>>> --- a/drivers/gpu/drm/ast/ast_drv.c
>>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>>> @@ -99,14 +99,14 @@ ast_pci_remove(struct pci_dev *pdev)
>>>  	drm_put_dev(dev);
>>>  }
>>>  
>>> -
>>> -
>>>  static int ast_drm_freeze(struct drm_device *dev)
>>>  {
>>> -	drm_kms_helper_poll_disable(dev);
>>> -	pci_save_state(dev->pdev);
>>> -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
>>> +	int error;
>>>  
>>> +	error = drm_mode_config_helper_suspend(dev);
>>> +	if (error)
>>> +		return error;
>>> +	pci_save_state(dev->pdev);
>>>  	return 0;
>>>  }
>>>  
>>> @@ -114,11 +114,7 @@ static int ast_drm_thaw(struct drm_device *dev)
>>>  {
>>>  	ast_post_gpu(dev);
>>>  
>>> -	drm_mode_config_reset(dev);
>>> -	drm_helper_resume_force_mode(dev);
>>> -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
>>> -
>>> -	return 0;
>>> +	return drm_mode_config_helper_resume(dev);
>>>  }
>>>  
>>>  static int ast_drm_resume(struct drm_device *dev)
>>> @@ -131,8 +127,6 @@ static int ast_drm_resume(struct drm_device *dev)
>>>  	ret = ast_drm_thaw(dev);
>>>  	if (ret)
>>>  		return ret;
>>> -
>>> -	drm_kms_helper_poll_enable(dev);
>>>  	return 0;
>>>  }
>>>  
>>> @@ -150,6 +144,7 @@ static int ast_pm_suspend(struct device *dev)
>>>  	pci_set_power_state(pdev, PCI_D3hot);
>>>  	return 0;
>>>  }
>>> +
>>>  static int ast_pm_resume(struct device *dev)
>>>  {
>>>  	struct pci_dev *pdev = to_pci_dev(dev);
>>> @@ -165,7 +160,6 @@ static int ast_pm_freeze(struct device *dev)
>>>  	if (!ddev || !ddev->dev_private)
>>>  		return -ENODEV;
>>>  	return ast_drm_freeze(ddev);
>>> -
>>>  }
>>>  
>>>  static int ast_pm_thaw(struct device *dev)
>>> @@ -203,7 +197,9 @@ static struct pci_driver ast_pci_driver = {
>>>  DEFINE_DRM_GEM_FOPS(ast_fops);
>>>  
>>>  static struct drm_driver driver = {
>>> -	.driver_features = DRIVER_MODESET | DRIVER_GEM,
>>> +	.driver_features = DRIVER_ATOMIC |
>>> +			   DRIVER_GEM |
>>> +			   DRIVER_MODESET,
>>>  
>>>  	.load = ast_driver_load,
>>>  	.unload = ast_driver_unload,
>>> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
>>> index 48d57ab42955..b79f484e9bd2 100644
>>> --- a/drivers/gpu/drm/ast/ast_main.c
>>> +++ b/drivers/gpu/drm/ast/ast_main.c
>>> @@ -28,6 +28,7 @@
>>>  
>>>  #include <linux/pci.h>
>>>  
>>> +#include <drm/drm_atomic_helper.h>
>>>  #include <drm/drm_crtc_helper.h>
>>>  #include <drm/drm_fb_helper.h>
>>>  #include <drm/drm_gem.h>
>>> @@ -412,6 +413,8 @@ enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
>>>  static const struct drm_mode_config_funcs ast_mode_funcs = {
>>>  	.fb_create = drm_gem_fb_create,
>>>  	.mode_valid = ast_mode_config_mode_valid,
>>> +	.atomic_check = drm_atomic_helper_check,
>>> +	.atomic_commit = drm_atomic_helper_commit,
>>>  };
>>>  
>>>  static u32 ast_get_vram_info(struct drm_device *dev)
>>> @@ -529,6 +532,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>>>  	if (ret)
>>>  		goto out_free;
>>>  
>>> +	drm_mode_config_reset(dev);
>>> +
>>>  	ret = drm_fbdev_generic_setup(dev, 32);
>>>  	if (ret)
>>>  		goto out_free;
>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>>> index f5f73200e8e4..5eccb6ae2ede 100644
>>> --- a/drivers/gpu/drm/ast/ast_mode.c
>>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>>> @@ -45,11 +45,6 @@
>>>  
>>>  static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
>>>  static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
>>> -static int ast_cursor_set(struct drm_crtc *crtc,
>>> -			  struct drm_file *file_priv,
>>> -			  uint32_t handle,
>>> -			  uint32_t width,
>>> -			  uint32_t height);
>>>  static int ast_cursor_move(struct drm_crtc *crtc,
>>>  			   int x, int y);
>>>  
>>> @@ -58,9 +53,6 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
>>>  static int ast_cursor_update(void *dst, void *src, unsigned int width,
>>>  			     unsigned int height);
>>>  static void ast_cursor_set_base(struct ast_private *ast, u64 address);
>>> -static int ast_show_cursor(struct drm_crtc *crtc, void *src,
>>> -			   unsigned int width, unsigned int height);
>>> -static void ast_hide_cursor(struct drm_crtc *crtc);
>>>  static int ast_cursor_move(struct drm_crtc *crtc,
>>>  			   int x, int y);
>>>  
>>> @@ -434,7 +426,7 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
>>>  static void ast_set_offset_reg(struct drm_crtc *crtc)
>>>  {
>>>  	struct ast_private *ast = crtc->dev->dev_private;
>>> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>>> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
>>>  
>>>  	u16 offset;
>>>  
>>> @@ -528,7 +520,7 @@ static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
>>>  static bool ast_set_dac_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
>>>  		     struct ast_vbios_mode_info *vbios_mode)
>>>  {
>>> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>>> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
>>>  
>>>  	switch (fb->format->cpp[0] * 8) {
>>>  	case 8:
>>> @@ -765,112 +757,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>>>  	}
>>>  }
>>>  
>>> -static int ast_crtc_do_set_base(struct drm_crtc *crtc,
>>> -				struct drm_framebuffer *fb,
>>> -				int x, int y, int atomic)
>>> -{
>>> -	struct drm_gem_vram_object *gbo;
>>> -	int ret;
>>> -	s64 gpu_addr;
>>> -
>>> -	if (!atomic && fb) {
>>> -		gbo = drm_gem_vram_of_gem(fb->obj[0]);
>>> -		drm_gem_vram_unpin(gbo);
>>> -	}
>>> -
>>> -	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
>>> -
>>> -	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>>> -	if (ret)
>>> -		return ret;
>>> -	gpu_addr = drm_gem_vram_offset(gbo);
>>> -	if (gpu_addr < 0) {
>>> -		ret = (int)gpu_addr;
>>> -		goto err_drm_gem_vram_unpin;
>>> -	}
>>> -
>>> -	ast_set_offset_reg(crtc);
>>> -	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
>>> -
>>> -	return 0;
>>> -
>>> -err_drm_gem_vram_unpin:
>>> -	drm_gem_vram_unpin(gbo);
>>> -	return ret;
>>> -}
>>> -
>>> -static int ast_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>>> -			     struct drm_framebuffer *old_fb)
>>> -{
>>> -	return ast_crtc_do_set_base(crtc, old_fb, x, y, 0);
>>> -}
>>> -
>>> -static int ast_crtc_mode_set(struct drm_crtc *crtc,
>>> -			     struct drm_display_mode *mode,
>>> -			     struct drm_display_mode *adjusted_mode,
>>> -			     int x, int y,
>>> -			     struct drm_framebuffer *old_fb)
>>> -{
>>> -	struct drm_device *dev = crtc->dev;
>>> -	struct ast_private *ast = crtc->dev->dev_private;
>>> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>>> -	struct ast_vbios_mode_info vbios_mode;
>>> -	bool succ;
>>> -
>>> -	if (ast->chip == AST1180) {
>>> -		DRM_ERROR("AST 1180 modesetting not supported\n");
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	succ = ast_get_vbios_mode_info(fb, mode, adjusted_mode, &vbios_mode);
>>> -	if (!succ)
>>> -		return -EINVAL;
>>> -
>>> -	ast_open_key(ast);
>>> -
>>> -	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
>>> -	ast_set_vbios_mode_reg(crtc, adjusted_mode, &vbios_mode);
>>> -	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
>>> -	ast_set_std_reg(crtc, adjusted_mode, &vbios_mode);
>>> -	ast_set_crtc_reg(crtc, adjusted_mode, &vbios_mode);
>>> -	ast_set_offset_reg(crtc);
>>> -	ast_set_dclk_reg(dev, adjusted_mode, &vbios_mode);
>>> -	ast_set_color_reg(crtc, fb);
>>> -	ast_set_crtthd_reg(crtc);
>>> -	ast_set_sync_reg(dev, adjusted_mode, &vbios_mode);
>>> -	ast_set_dac_reg(crtc, adjusted_mode, &vbios_mode);
>>> -
>>> -	ast_crtc_mode_set_base(crtc, x, y, old_fb);
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -static void ast_crtc_disable(struct drm_crtc *crtc)
>>> -{
>>> -	DRM_DEBUG_KMS("\n");
>>> -	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
>>> -	if (crtc->primary->fb) {
>>> -		struct drm_framebuffer *fb = crtc->primary->fb;
>>> -		struct drm_gem_vram_object *gbo =
>>> -			drm_gem_vram_of_gem(fb->obj[0]);
>>> -
>>> -		drm_gem_vram_unpin(gbo);
>>> -	}
>>> -	crtc->primary->fb = NULL;
>>> -}
>>> -
>>> -static void ast_crtc_prepare(struct drm_crtc *crtc)
>>> -{
>>> -
>>> -}
>>> -
>>> -static void ast_crtc_commit(struct drm_crtc *crtc)
>>> -{
>>> -	struct ast_private *ast = crtc->dev->dev_private;
>>> -	ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0);
>>> -	ast_crtc_load_lut(crtc);
>>> -}
>>> -
>>>  static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>>>  					struct drm_crtc_state *state)
>>>  {
>>> @@ -970,12 +856,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>>>  }
>>>  
>>>  static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>>> -	.dpms = ast_crtc_dpms,
>>> -	.mode_set = ast_crtc_mode_set,
>>> -	.mode_set_base = ast_crtc_mode_set_base,
>>> -	.disable = ast_crtc_disable,
>>> -	.prepare = ast_crtc_prepare,
>>> -	.commit = ast_crtc_commit,
>>>  	.atomic_check = ast_crtc_helper_atomic_check,
>>>  	.atomic_begin = ast_crtc_helper_atomic_begin,
>>>  	.atomic_flush = ast_crtc_helper_atomic_flush,
>>> @@ -983,21 +863,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>>>  	.atomic_disable = ast_crtc_helper_atomic_disable,
>>>  };
>>>  
>>> -static void ast_crtc_reset(struct drm_crtc *crtc)
>>> -{
>>> -
>>> -}
>>> -
>>> -static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>>> -			      u16 *blue, uint32_t size,
>>> -			      struct drm_modeset_acquire_ctx *ctx)
>>> -{
>>> -	ast_crtc_load_lut(crtc);
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -
>>>  static void ast_crtc_destroy(struct drm_crtc *crtc)
>>>  {
>>>  	drm_crtc_cleanup(crtc);
>>> @@ -1005,12 +870,12 @@ static void ast_crtc_destroy(struct drm_crtc *crtc)
>>>  }
>>>  
>>>  static const struct drm_crtc_funcs ast_crtc_funcs = {
>>> -	.cursor_set = ast_cursor_set,
>>> -	.cursor_move = ast_cursor_move,
>>> -	.reset = ast_crtc_reset,
>>> +	.reset = drm_atomic_helper_crtc_reset,
>>>  	.set_config = drm_crtc_helper_set_config,
>>> -	.gamma_set = ast_crtc_gamma_set,
>>> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>>>  	.destroy = ast_crtc_destroy,
>>> +	.set_config = drm_atomic_helper_set_config,
>>> +	.page_flip = drm_atomic_helper_page_flip,
>>>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>>>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>>  };
>>> @@ -1040,6 +905,10 @@ static int ast_crtc_init(struct drm_device *dev)
>>>  	return ret;
>>>  }
>>>  
>>> +/*
>>> + * Encoder
>>> + */
>>> +
>>>  static void ast_encoder_destroy(struct drm_encoder *encoder)
>>>  {
>>>  	drm_encoder_cleanup(encoder);
>>> @@ -1050,34 +919,6 @@ static const struct drm_encoder_funcs ast_enc_funcs = {
>>>  	.destroy = ast_encoder_destroy,
>>>  };
>>>  
>>> -static void ast_encoder_dpms(struct drm_encoder *encoder, int mode)
>>> -{
>>> -
>>> -}
>>> -
>>> -static void ast_encoder_mode_set(struct drm_encoder *encoder,
>>> -			       struct drm_display_mode *mode,
>>> -			       struct drm_display_mode *adjusted_mode)
>>> -{
>>> -}
>>> -
>>> -static void ast_encoder_prepare(struct drm_encoder *encoder)
>>> -{
>>> -
>>> -}
>>> -
>>> -static void ast_encoder_commit(struct drm_encoder *encoder)
>>> -{
>>> -
>>> -}
>>> -
>>> -static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
>>> -	.dpms = ast_encoder_dpms,
>>> -	.prepare = ast_encoder_prepare,
>>> -	.commit = ast_encoder_commit,
>>> -	.mode_set = ast_encoder_mode_set,
>>> -};
>>
>> Hmm.  Pretty much a dummy encoder implementation.  Maybe ast is simple
>> enough that the simple pipe helpers can do the trick?
> 
> simple pipe doesn't do multi-plane, so not applicable. Maybe there's a few
> more things we can for dummy encoders (like default cleanup perhaps?).

I'll see what I can do for the patchset's next iteration.

Best regards
Thomas

> -Daniel
> 
>>
>> cheers,
>>   Gerd
>>
>
Thomas Zimmermann Nov. 6, 2019, 1:36 p.m. UTC | #4
Hi

Am 05.11.19 um 10:57 schrieb Gerd Hoffmann:
> On Mon, Oct 28, 2019 at 04:49:28PM +0100, Thomas Zimmermann wrote:
>> This commit sets the remaining atomic-modesetting helpers and the flag
>> DRIVER_ATOMIC. Legacy cursor functions are removed in favor of the cursor
>> plane. For power management, atomic helpers replace the indvidual
>> operations that the driver currently runs.
>>
>> Atomic modesetting is enabled with this commit.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/ast/ast_drv.c  |  24 ++-
>>  drivers/gpu/drm/ast/ast_main.c |   5 +
>>  drivers/gpu/drm/ast/ast_mode.c | 290 ++-------------------------------
>>  3 files changed, 33 insertions(+), 286 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
>> index 1f17794b0890..d763da6f0834 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.c
>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>> @@ -99,14 +99,14 @@ ast_pci_remove(struct pci_dev *pdev)
>>  	drm_put_dev(dev);
>>  }
>>  
>> -
>> -
>>  static int ast_drm_freeze(struct drm_device *dev)
>>  {
>> -	drm_kms_helper_poll_disable(dev);
>> -	pci_save_state(dev->pdev);
>> -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
>> +	int error;
>>  
>> +	error = drm_mode_config_helper_suspend(dev);
>> +	if (error)
>> +		return error;
>> +	pci_save_state(dev->pdev);
>>  	return 0;
>>  }
>>  
>> @@ -114,11 +114,7 @@ static int ast_drm_thaw(struct drm_device *dev)
>>  {
>>  	ast_post_gpu(dev);
>>  
>> -	drm_mode_config_reset(dev);
>> -	drm_helper_resume_force_mode(dev);
>> -	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
>> -
>> -	return 0;
>> +	return drm_mode_config_helper_resume(dev);
>>  }
>>  
>>  static int ast_drm_resume(struct drm_device *dev)
>> @@ -131,8 +127,6 @@ static int ast_drm_resume(struct drm_device *dev)
>>  	ret = ast_drm_thaw(dev);
>>  	if (ret)
>>  		return ret;
>> -
>> -	drm_kms_helper_poll_enable(dev);
>>  	return 0;
>>  }
>>  
>> @@ -150,6 +144,7 @@ static int ast_pm_suspend(struct device *dev)
>>  	pci_set_power_state(pdev, PCI_D3hot);
>>  	return 0;
>>  }
>> +
>>  static int ast_pm_resume(struct device *dev)
>>  {
>>  	struct pci_dev *pdev = to_pci_dev(dev);
>> @@ -165,7 +160,6 @@ static int ast_pm_freeze(struct device *dev)
>>  	if (!ddev || !ddev->dev_private)
>>  		return -ENODEV;
>>  	return ast_drm_freeze(ddev);
>> -
>>  }
>>  
>>  static int ast_pm_thaw(struct device *dev)
>> @@ -203,7 +197,9 @@ static struct pci_driver ast_pci_driver = {
>>  DEFINE_DRM_GEM_FOPS(ast_fops);
>>  
>>  static struct drm_driver driver = {
>> -	.driver_features = DRIVER_MODESET | DRIVER_GEM,
>> +	.driver_features = DRIVER_ATOMIC |
>> +			   DRIVER_GEM |
>> +			   DRIVER_MODESET,
>>  
>>  	.load = ast_driver_load,
>>  	.unload = ast_driver_unload,
>> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
>> index 48d57ab42955..b79f484e9bd2 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -28,6 +28,7 @@
>>  
>>  #include <linux/pci.h>
>>  
>> +#include <drm/drm_atomic_helper.h>
>>  #include <drm/drm_crtc_helper.h>
>>  #include <drm/drm_fb_helper.h>
>>  #include <drm/drm_gem.h>
>> @@ -412,6 +413,8 @@ enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
>>  static const struct drm_mode_config_funcs ast_mode_funcs = {
>>  	.fb_create = drm_gem_fb_create,
>>  	.mode_valid = ast_mode_config_mode_valid,
>> +	.atomic_check = drm_atomic_helper_check,
>> +	.atomic_commit = drm_atomic_helper_commit,
>>  };
>>  
>>  static u32 ast_get_vram_info(struct drm_device *dev)
>> @@ -529,6 +532,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>>  	if (ret)
>>  		goto out_free;
>>  
>> +	drm_mode_config_reset(dev);
>> +
>>  	ret = drm_fbdev_generic_setup(dev, 32);
>>  	if (ret)
>>  		goto out_free;
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index f5f73200e8e4..5eccb6ae2ede 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -45,11 +45,6 @@
>>  
>>  static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
>>  static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
>> -static int ast_cursor_set(struct drm_crtc *crtc,
>> -			  struct drm_file *file_priv,
>> -			  uint32_t handle,
>> -			  uint32_t width,
>> -			  uint32_t height);
>>  static int ast_cursor_move(struct drm_crtc *crtc,
>>  			   int x, int y);
>>  
>> @@ -58,9 +53,6 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
>>  static int ast_cursor_update(void *dst, void *src, unsigned int width,
>>  			     unsigned int height);
>>  static void ast_cursor_set_base(struct ast_private *ast, u64 address);
>> -static int ast_show_cursor(struct drm_crtc *crtc, void *src,
>> -			   unsigned int width, unsigned int height);
>> -static void ast_hide_cursor(struct drm_crtc *crtc);
>>  static int ast_cursor_move(struct drm_crtc *crtc,
>>  			   int x, int y);
>>  
>> @@ -434,7 +426,7 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
>>  static void ast_set_offset_reg(struct drm_crtc *crtc)
>>  {
>>  	struct ast_private *ast = crtc->dev->dev_private;
>> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
>>  
>>  	u16 offset;
>>  
>> @@ -528,7 +520,7 @@ static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
>>  static bool ast_set_dac_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
>>  		     struct ast_vbios_mode_info *vbios_mode)
>>  {
>> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
>>  
>>  	switch (fb->format->cpp[0] * 8) {
>>  	case 8:
>> @@ -765,112 +757,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>>  	}
>>  }
>>  
>> -static int ast_crtc_do_set_base(struct drm_crtc *crtc,
>> -				struct drm_framebuffer *fb,
>> -				int x, int y, int atomic)
>> -{
>> -	struct drm_gem_vram_object *gbo;
>> -	int ret;
>> -	s64 gpu_addr;
>> -
>> -	if (!atomic && fb) {
>> -		gbo = drm_gem_vram_of_gem(fb->obj[0]);
>> -		drm_gem_vram_unpin(gbo);
>> -	}
>> -
>> -	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
>> -
>> -	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>> -	if (ret)
>> -		return ret;
>> -	gpu_addr = drm_gem_vram_offset(gbo);
>> -	if (gpu_addr < 0) {
>> -		ret = (int)gpu_addr;
>> -		goto err_drm_gem_vram_unpin;
>> -	}
>> -
>> -	ast_set_offset_reg(crtc);
>> -	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
>> -
>> -	return 0;
>> -
>> -err_drm_gem_vram_unpin:
>> -	drm_gem_vram_unpin(gbo);
>> -	return ret;
>> -}
>> -
>> -static int ast_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>> -			     struct drm_framebuffer *old_fb)
>> -{
>> -	return ast_crtc_do_set_base(crtc, old_fb, x, y, 0);
>> -}
>> -
>> -static int ast_crtc_mode_set(struct drm_crtc *crtc,
>> -			     struct drm_display_mode *mode,
>> -			     struct drm_display_mode *adjusted_mode,
>> -			     int x, int y,
>> -			     struct drm_framebuffer *old_fb)
>> -{
>> -	struct drm_device *dev = crtc->dev;
>> -	struct ast_private *ast = crtc->dev->dev_private;
>> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>> -	struct ast_vbios_mode_info vbios_mode;
>> -	bool succ;
>> -
>> -	if (ast->chip == AST1180) {
>> -		DRM_ERROR("AST 1180 modesetting not supported\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	succ = ast_get_vbios_mode_info(fb, mode, adjusted_mode, &vbios_mode);
>> -	if (!succ)
>> -		return -EINVAL;
>> -
>> -	ast_open_key(ast);
>> -
>> -	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
>> -	ast_set_vbios_mode_reg(crtc, adjusted_mode, &vbios_mode);
>> -	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
>> -	ast_set_std_reg(crtc, adjusted_mode, &vbios_mode);
>> -	ast_set_crtc_reg(crtc, adjusted_mode, &vbios_mode);
>> -	ast_set_offset_reg(crtc);
>> -	ast_set_dclk_reg(dev, adjusted_mode, &vbios_mode);
>> -	ast_set_color_reg(crtc, fb);
>> -	ast_set_crtthd_reg(crtc);
>> -	ast_set_sync_reg(dev, adjusted_mode, &vbios_mode);
>> -	ast_set_dac_reg(crtc, adjusted_mode, &vbios_mode);
>> -
>> -	ast_crtc_mode_set_base(crtc, x, y, old_fb);
>> -
>> -	return 0;
>> -}
>> -
>> -static void ast_crtc_disable(struct drm_crtc *crtc)
>> -{
>> -	DRM_DEBUG_KMS("\n");
>> -	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
>> -	if (crtc->primary->fb) {
>> -		struct drm_framebuffer *fb = crtc->primary->fb;
>> -		struct drm_gem_vram_object *gbo =
>> -			drm_gem_vram_of_gem(fb->obj[0]);
>> -
>> -		drm_gem_vram_unpin(gbo);
>> -	}
>> -	crtc->primary->fb = NULL;
>> -}
>> -
>> -static void ast_crtc_prepare(struct drm_crtc *crtc)
>> -{
>> -
>> -}
>> -
>> -static void ast_crtc_commit(struct drm_crtc *crtc)
>> -{
>> -	struct ast_private *ast = crtc->dev->dev_private;
>> -	ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0);
>> -	ast_crtc_load_lut(crtc);
>> -}
>> -
>>  static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>>  					struct drm_crtc_state *state)
>>  {
>> @@ -970,12 +856,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>>  }
>>  
>>  static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>> -	.dpms = ast_crtc_dpms,
>> -	.mode_set = ast_crtc_mode_set,
>> -	.mode_set_base = ast_crtc_mode_set_base,
>> -	.disable = ast_crtc_disable,
>> -	.prepare = ast_crtc_prepare,
>> -	.commit = ast_crtc_commit,
>>  	.atomic_check = ast_crtc_helper_atomic_check,
>>  	.atomic_begin = ast_crtc_helper_atomic_begin,
>>  	.atomic_flush = ast_crtc_helper_atomic_flush,
>> @@ -983,21 +863,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>>  	.atomic_disable = ast_crtc_helper_atomic_disable,
>>  };
>>  
>> -static void ast_crtc_reset(struct drm_crtc *crtc)
>> -{
>> -
>> -}
>> -
>> -static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>> -			      u16 *blue, uint32_t size,
>> -			      struct drm_modeset_acquire_ctx *ctx)
>> -{
>> -	ast_crtc_load_lut(crtc);
>> -
>> -	return 0;
>> -}
>> -
>> -
>>  static void ast_crtc_destroy(struct drm_crtc *crtc)
>>  {
>>  	drm_crtc_cleanup(crtc);
>> @@ -1005,12 +870,12 @@ static void ast_crtc_destroy(struct drm_crtc *crtc)
>>  }
>>  
>>  static const struct drm_crtc_funcs ast_crtc_funcs = {
>> -	.cursor_set = ast_cursor_set,
>> -	.cursor_move = ast_cursor_move,
>> -	.reset = ast_crtc_reset,
>> +	.reset = drm_atomic_helper_crtc_reset,
>>  	.set_config = drm_crtc_helper_set_config,
>> -	.gamma_set = ast_crtc_gamma_set,
>> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>>  	.destroy = ast_crtc_destroy,
>> +	.set_config = drm_atomic_helper_set_config,
>> +	.page_flip = drm_atomic_helper_page_flip,
>>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>  };
>> @@ -1040,6 +905,10 @@ static int ast_crtc_init(struct drm_device *dev)
>>  	return ret;
>>  }
>>  
>> +/*
>> + * Encoder
>> + */
>> +
>>  static void ast_encoder_destroy(struct drm_encoder *encoder)
>>  {
>>  	drm_encoder_cleanup(encoder);
>> @@ -1050,34 +919,6 @@ static const struct drm_encoder_funcs ast_enc_funcs = {
>>  	.destroy = ast_encoder_destroy,
>>  };
>>  
>> -static void ast_encoder_dpms(struct drm_encoder *encoder, int mode)
>> -{
>> -
>> -}
>> -
>> -static void ast_encoder_mode_set(struct drm_encoder *encoder,
>> -			       struct drm_display_mode *mode,
>> -			       struct drm_display_mode *adjusted_mode)
>> -{
>> -}
>> -
>> -static void ast_encoder_prepare(struct drm_encoder *encoder)
>> -{
>> -
>> -}
>> -
>> -static void ast_encoder_commit(struct drm_encoder *encoder)
>> -{
>> -
>> -}
>> -
>> -static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
>> -	.dpms = ast_encoder_dpms,
>> -	.prepare = ast_encoder_prepare,
>> -	.commit = ast_encoder_commit,
>> -	.mode_set = ast_encoder_mode_set,
>> -};
> 
> Hmm.  Pretty much a dummy encoder implementation.  Maybe ast is simple
> enough that the simple pipe helpers can do the trick?

As Daniel said, simple pipe helpers don't support cursors. I
investigated his comment on a encoder helpers and found that many
drivers (including ast) only create an encoder structure without
additional functionality.

It's probably worth introducing a default implementation for the
encoder, but I'd like to do that in a separate patch set. Ok?

Best regards
Thomas

> 
> cheers,
>   Gerd
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Gerd Hoffmann Nov. 7, 2019, 6:55 a.m. UTC | #5
> > Hmm.  Pretty much a dummy encoder implementation.  Maybe ast is simple
> > enough that the simple pipe helpers can do the trick?
> 
> As Daniel said, simple pipe helpers don't support cursors. I
> investigated his comment on a encoder helpers and found that many
> drivers (including ast) only create an encoder structure without
> additional functionality.
> 
> It's probably worth introducing a default implementation for the
> encoder,

Either that, or make all the callbacks optional so a encoder without
additional functionality needs only a few lines of code.

> but I'd like to do that in a separate patch set. Ok?

Yep, that totally makes sense, given that it'll probably become a patch
series of its own (with driver cleanups included).

So, for this patch:
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd
Thomas Zimmermann Nov. 7, 2019, 7:32 a.m. UTC | #6
Hi

Am 07.11.19 um 07:55 schrieb Gerd Hoffmann:
>>> Hmm.  Pretty much a dummy encoder implementation.  Maybe ast is simple
>>> enough that the simple pipe helpers can do the trick?
>>
>> As Daniel said, simple pipe helpers don't support cursors. I
>> investigated his comment on a encoder helpers and found that many
>> drivers (including ast) only create an encoder structure without
>> additional functionality.
>>
>> It's probably worth introducing a default implementation for the
>> encoder,
> 
> Either that, or make all the callbacks optional so a encoder without
> additional functionality needs only a few lines of code.
> 
>> but I'd like to do that in a separate patch set. Ok?
> 
> Yep, that totally makes sense, given that it'll probably become a patch
> series of its own (with driver cleanups included).

Absolutely. I took a look at other driver's encoders and most of them
are empty implementations; just like ast. Having a simple-encoder helper
should make most of this go away.

> 
> So, for this patch:
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>

Thanks.

Best regards
Thomas

> 
> cheers,
>   Gerd
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 1f17794b0890..d763da6f0834 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -99,14 +99,14 @@  ast_pci_remove(struct pci_dev *pdev)
 	drm_put_dev(dev);
 }
 
-
-
 static int ast_drm_freeze(struct drm_device *dev)
 {
-	drm_kms_helper_poll_disable(dev);
-	pci_save_state(dev->pdev);
-	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
+	int error;
 
+	error = drm_mode_config_helper_suspend(dev);
+	if (error)
+		return error;
+	pci_save_state(dev->pdev);
 	return 0;
 }
 
@@ -114,11 +114,7 @@  static int ast_drm_thaw(struct drm_device *dev)
 {
 	ast_post_gpu(dev);
 
-	drm_mode_config_reset(dev);
-	drm_helper_resume_force_mode(dev);
-	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
-
-	return 0;
+	return drm_mode_config_helper_resume(dev);
 }
 
 static int ast_drm_resume(struct drm_device *dev)
@@ -131,8 +127,6 @@  static int ast_drm_resume(struct drm_device *dev)
 	ret = ast_drm_thaw(dev);
 	if (ret)
 		return ret;
-
-	drm_kms_helper_poll_enable(dev);
 	return 0;
 }
 
@@ -150,6 +144,7 @@  static int ast_pm_suspend(struct device *dev)
 	pci_set_power_state(pdev, PCI_D3hot);
 	return 0;
 }
+
 static int ast_pm_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -165,7 +160,6 @@  static int ast_pm_freeze(struct device *dev)
 	if (!ddev || !ddev->dev_private)
 		return -ENODEV;
 	return ast_drm_freeze(ddev);
-
 }
 
 static int ast_pm_thaw(struct device *dev)
@@ -203,7 +197,9 @@  static struct pci_driver ast_pci_driver = {
 DEFINE_DRM_GEM_FOPS(ast_fops);
 
 static struct drm_driver driver = {
-	.driver_features = DRIVER_MODESET | DRIVER_GEM,
+	.driver_features = DRIVER_ATOMIC |
+			   DRIVER_GEM |
+			   DRIVER_MODESET,
 
 	.load = ast_driver_load,
 	.unload = ast_driver_unload,
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 48d57ab42955..b79f484e9bd2 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -28,6 +28,7 @@ 
 
 #include <linux/pci.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
@@ -412,6 +413,8 @@  enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
 static const struct drm_mode_config_funcs ast_mode_funcs = {
 	.fb_create = drm_gem_fb_create,
 	.mode_valid = ast_mode_config_mode_valid,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
 };
 
 static u32 ast_get_vram_info(struct drm_device *dev)
@@ -529,6 +532,8 @@  int ast_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret)
 		goto out_free;
 
+	drm_mode_config_reset(dev);
+
 	ret = drm_fbdev_generic_setup(dev, 32);
 	if (ret)
 		goto out_free;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index f5f73200e8e4..5eccb6ae2ede 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -45,11 +45,6 @@ 
 
 static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
 static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
-static int ast_cursor_set(struct drm_crtc *crtc,
-			  struct drm_file *file_priv,
-			  uint32_t handle,
-			  uint32_t width,
-			  uint32_t height);
 static int ast_cursor_move(struct drm_crtc *crtc,
 			   int x, int y);
 
@@ -58,9 +53,6 @@  static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
 static int ast_cursor_update(void *dst, void *src, unsigned int width,
 			     unsigned int height);
 static void ast_cursor_set_base(struct ast_private *ast, u64 address);
-static int ast_show_cursor(struct drm_crtc *crtc, void *src,
-			   unsigned int width, unsigned int height);
-static void ast_hide_cursor(struct drm_crtc *crtc);
 static int ast_cursor_move(struct drm_crtc *crtc,
 			   int x, int y);
 
@@ -434,7 +426,7 @@  static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
 static void ast_set_offset_reg(struct drm_crtc *crtc)
 {
 	struct ast_private *ast = crtc->dev->dev_private;
-	const struct drm_framebuffer *fb = crtc->primary->fb;
+	const struct drm_framebuffer *fb = crtc->primary->state->fb;
 
 	u16 offset;
 
@@ -528,7 +520,7 @@  static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
 static bool ast_set_dac_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
 		     struct ast_vbios_mode_info *vbios_mode)
 {
-	const struct drm_framebuffer *fb = crtc->primary->fb;
+	const struct drm_framebuffer *fb = crtc->primary->state->fb;
 
 	switch (fb->format->cpp[0] * 8) {
 	case 8:
@@ -765,112 +757,6 @@  static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
 	}
 }
 
-static int ast_crtc_do_set_base(struct drm_crtc *crtc,
-				struct drm_framebuffer *fb,
-				int x, int y, int atomic)
-{
-	struct drm_gem_vram_object *gbo;
-	int ret;
-	s64 gpu_addr;
-
-	if (!atomic && fb) {
-		gbo = drm_gem_vram_of_gem(fb->obj[0]);
-		drm_gem_vram_unpin(gbo);
-	}
-
-	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
-
-	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret)
-		return ret;
-	gpu_addr = drm_gem_vram_offset(gbo);
-	if (gpu_addr < 0) {
-		ret = (int)gpu_addr;
-		goto err_drm_gem_vram_unpin;
-	}
-
-	ast_set_offset_reg(crtc);
-	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
-
-	return 0;
-
-err_drm_gem_vram_unpin:
-	drm_gem_vram_unpin(gbo);
-	return ret;
-}
-
-static int ast_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-			     struct drm_framebuffer *old_fb)
-{
-	return ast_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-static int ast_crtc_mode_set(struct drm_crtc *crtc,
-			     struct drm_display_mode *mode,
-			     struct drm_display_mode *adjusted_mode,
-			     int x, int y,
-			     struct drm_framebuffer *old_fb)
-{
-	struct drm_device *dev = crtc->dev;
-	struct ast_private *ast = crtc->dev->dev_private;
-	const struct drm_framebuffer *fb = crtc->primary->fb;
-	struct ast_vbios_mode_info vbios_mode;
-	bool succ;
-
-	if (ast->chip == AST1180) {
-		DRM_ERROR("AST 1180 modesetting not supported\n");
-		return -EINVAL;
-	}
-
-	succ = ast_get_vbios_mode_info(fb, mode, adjusted_mode, &vbios_mode);
-	if (!succ)
-		return -EINVAL;
-
-	ast_open_key(ast);
-
-	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
-	ast_set_vbios_mode_reg(crtc, adjusted_mode, &vbios_mode);
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
-	ast_set_std_reg(crtc, adjusted_mode, &vbios_mode);
-	ast_set_crtc_reg(crtc, adjusted_mode, &vbios_mode);
-	ast_set_offset_reg(crtc);
-	ast_set_dclk_reg(dev, adjusted_mode, &vbios_mode);
-	ast_set_color_reg(crtc, fb);
-	ast_set_crtthd_reg(crtc);
-	ast_set_sync_reg(dev, adjusted_mode, &vbios_mode);
-	ast_set_dac_reg(crtc, adjusted_mode, &vbios_mode);
-
-	ast_crtc_mode_set_base(crtc, x, y, old_fb);
-
-	return 0;
-}
-
-static void ast_crtc_disable(struct drm_crtc *crtc)
-{
-	DRM_DEBUG_KMS("\n");
-	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
-	if (crtc->primary->fb) {
-		struct drm_framebuffer *fb = crtc->primary->fb;
-		struct drm_gem_vram_object *gbo =
-			drm_gem_vram_of_gem(fb->obj[0]);
-
-		drm_gem_vram_unpin(gbo);
-	}
-	crtc->primary->fb = NULL;
-}
-
-static void ast_crtc_prepare(struct drm_crtc *crtc)
-{
-
-}
-
-static void ast_crtc_commit(struct drm_crtc *crtc)
-{
-	struct ast_private *ast = crtc->dev->dev_private;
-	ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0);
-	ast_crtc_load_lut(crtc);
-}
-
 static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
 					struct drm_crtc_state *state)
 {
@@ -970,12 +856,6 @@  ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
-	.dpms = ast_crtc_dpms,
-	.mode_set = ast_crtc_mode_set,
-	.mode_set_base = ast_crtc_mode_set_base,
-	.disable = ast_crtc_disable,
-	.prepare = ast_crtc_prepare,
-	.commit = ast_crtc_commit,
 	.atomic_check = ast_crtc_helper_atomic_check,
 	.atomic_begin = ast_crtc_helper_atomic_begin,
 	.atomic_flush = ast_crtc_helper_atomic_flush,
@@ -983,21 +863,6 @@  static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
 	.atomic_disable = ast_crtc_helper_atomic_disable,
 };
 
-static void ast_crtc_reset(struct drm_crtc *crtc)
-{
-
-}
-
-static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, uint32_t size,
-			      struct drm_modeset_acquire_ctx *ctx)
-{
-	ast_crtc_load_lut(crtc);
-
-	return 0;
-}
-
-
 static void ast_crtc_destroy(struct drm_crtc *crtc)
 {
 	drm_crtc_cleanup(crtc);
@@ -1005,12 +870,12 @@  static void ast_crtc_destroy(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs ast_crtc_funcs = {
-	.cursor_set = ast_cursor_set,
-	.cursor_move = ast_cursor_move,
-	.reset = ast_crtc_reset,
+	.reset = drm_atomic_helper_crtc_reset,
 	.set_config = drm_crtc_helper_set_config,
-	.gamma_set = ast_crtc_gamma_set,
+	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.destroy = ast_crtc_destroy,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
@@ -1040,6 +905,10 @@  static int ast_crtc_init(struct drm_device *dev)
 	return ret;
 }
 
+/*
+ * Encoder
+ */
+
 static void ast_encoder_destroy(struct drm_encoder *encoder)
 {
 	drm_encoder_cleanup(encoder);
@@ -1050,34 +919,6 @@  static const struct drm_encoder_funcs ast_enc_funcs = {
 	.destroy = ast_encoder_destroy,
 };
 
-static void ast_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-
-}
-
-static void ast_encoder_mode_set(struct drm_encoder *encoder,
-			       struct drm_display_mode *mode,
-			       struct drm_display_mode *adjusted_mode)
-{
-}
-
-static void ast_encoder_prepare(struct drm_encoder *encoder)
-{
-
-}
-
-static void ast_encoder_commit(struct drm_encoder *encoder)
-{
-
-}
-
-static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
-	.dpms = ast_encoder_dpms,
-	.prepare = ast_encoder_prepare,
-	.commit = ast_encoder_commit,
-	.mode_set = ast_encoder_mode_set,
-};
-
 static int ast_encoder_init(struct drm_device *dev)
 {
 	struct ast_encoder *ast_encoder;
@@ -1088,12 +929,15 @@  static int ast_encoder_init(struct drm_device *dev)
 
 	drm_encoder_init(dev, &ast_encoder->base, &ast_enc_funcs,
 			 DRM_MODE_ENCODER_DAC, NULL);
-	drm_encoder_helper_add(&ast_encoder->base, &ast_enc_helper_funcs);
 
 	ast_encoder->base.possible_crtcs = 1;
 	return 0;
 }
 
+/*
+ * Connector
+ */
+
 static int ast_get_modes(struct drm_connector *connector)
 {
 	struct ast_connector *ast_connector = to_ast_connector(connector);
@@ -1192,14 +1036,16 @@  static void ast_connector_destroy(struct drm_connector *connector)
 }
 
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
-	.mode_valid = ast_mode_valid,
 	.get_modes = ast_get_modes,
+	.mode_valid = ast_mode_valid,
 };
 
 static const struct drm_connector_funcs ast_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = ast_connector_destroy,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static int ast_connector_init(struct drm_device *dev)
@@ -1549,106 +1395,6 @@  static void ast_cursor_set_base(struct ast_private *ast, u64 address)
 	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xca, addr2);
 }
 
-static int ast_show_cursor(struct drm_crtc *crtc, void *src,
-			   unsigned int width, unsigned int height)
-{
-	struct ast_private *ast = crtc->dev->dev_private;
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	struct drm_gem_vram_object *gbo;
-	void *dst;
-	s64 off;
-	int ret;
-	u8 jreg;
-
-	gbo = ast->cursor.gbo[ast->cursor.next_index];
-	dst = drm_gem_vram_vmap(gbo);
-	if (IS_ERR(dst))
-		return PTR_ERR(dst);
-	off = drm_gem_vram_offset(gbo);
-	if (off < 0) {
-		ret = (int)off;
-		goto err_drm_gem_vram_vunmap;
-	}
-
-	ret = ast_cursor_update(dst, src, width, height);
-	if (ret)
-		goto err_drm_gem_vram_vunmap;
-	ast_cursor_set_base(ast, off);
-
-	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - width;
-	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - height;
-
-	jreg = 0x2;
-	/* enable ARGB cursor */
-	jreg |= 1;
-	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
-
-	++ast->cursor.next_index;
-	ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
-
-	drm_gem_vram_vunmap(gbo, dst);
-
-	return 0;
-
-err_drm_gem_vram_vunmap:
-	drm_gem_vram_vunmap(gbo, dst);
-	return ret;
-}
-
-static void ast_hide_cursor(struct drm_crtc *crtc)
-{
-	struct ast_private *ast = crtc->dev->dev_private;
-
-	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
-}
-
-static int ast_cursor_set(struct drm_crtc *crtc,
-			  struct drm_file *file_priv,
-			  uint32_t handle,
-			  uint32_t width,
-			  uint32_t height)
-{
-	struct drm_gem_object *obj;
-	struct drm_gem_vram_object *gbo;
-	u8 *src;
-	int ret;
-
-	if (!handle) {
-		ast_hide_cursor(crtc);
-		return 0;
-	}
-
-	if (width > AST_MAX_HWC_WIDTH || height > AST_MAX_HWC_HEIGHT)
-		return -EINVAL;
-
-	obj = drm_gem_object_lookup(file_priv, handle);
-	if (!obj) {
-		DRM_ERROR("Cannot find cursor object %x for crtc\n", handle);
-		return -ENOENT;
-	}
-	gbo = drm_gem_vram_of_gem(obj);
-	src = drm_gem_vram_vmap(gbo);
-	if (IS_ERR(src)) {
-		ret = PTR_ERR(src);
-		goto err_drm_gem_object_put_unlocked;
-	}
-
-	ret = ast_show_cursor(crtc, src, width, height);
-	if (ret)
-		goto err_drm_gem_vram_vunmap;
-
-	drm_gem_vram_vunmap(gbo, src);
-	drm_gem_object_put_unlocked(obj);
-
-	return 0;
-
-err_drm_gem_vram_vunmap:
-	drm_gem_vram_vunmap(gbo, src);
-err_drm_gem_object_put_unlocked:
-	drm_gem_object_put_unlocked(obj);
-	return ret;
-}
-
 static int ast_cursor_move(struct drm_crtc *crtc,
 			   int x, int y)
 {