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

Daniel Vetter daniel at ffwll.ch
Tue Nov 5 10:31:56 UTC 2019


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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list