[PATCH 0/9] drm/ast: Convert to atomic modesetting

[PATCH 0/9] drm/ast: Convert to atomic modesetting

[Thomas Zimmermann]

This patch set adds universal planes to ast and converts the driver to
atomic modesetting.

The first patch is purely for clean-up.

Patches 2 to 5 prepare the ast modesetting code for universal planes and
atomic modesetting. The size calculation for each mode has to take double
buffering into account. Several functions have to be split to make them
work with the separate check and update on CRTCs and planes. There are no
functional changes.

Patches 6 to 8 add atomic modesetting code for planes and CRTC. Planes
immediately provide atomic functions. There's no intermediate step with
legacy functions for non-atomic drivers. The cursor plane HW only
supports ARGB4444, so the cursor plane converts the format internally;
just as the legacy implementation did.

Finally, patch 9 adds missing helpers and enables atomic modesetting. The
CRTC functions now provide page_flip, which enables Weston support on
ast hardware.

The patchset has been tested by running the fbdev console, X11 (Gnome)
and Weston on an AST2100 chipset.

Thomas Zimmermann (9):
  drm/ast: Remove last traces of struct ast_gem_object
  drm/ast: Check video-mode requirements against VRAM size
  drm/ast: Don't clear base address and offset with default values
  drm/ast: Split ast_set_ext_reg() into color and threshold function
  drm/ast: Split ast_set_vbios_mode_info()
  drm/ast: Add primary plane
  drm/ast: Add CRTC helpers for atomic modesetting
  drm/ast: Add cursor plane
  drm/ast: Enable atomic modesetting

 drivers/gpu/drm/ast/ast_drv.c  |  24 +-
 drivers/gpu/drm/ast/ast_drv.h  |   9 +-
 drivers/gpu/drm/ast/ast_main.c |  54 +--
 drivers/gpu/drm/ast/ast_mode.c | 698 ++++++++++++++++++++-------------
 4 files changed, 462 insertions(+), 323 deletions(-)

--
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/9] drm/ast: Remove last traces of struct ast_gem_object

[Thomas Zimmermann]

The ast driver has switched to struct drm_vram_gem_object a while ago.
This patch removes a function and forward declaration that were forgotten
before.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |  6 ------
 drivers/gpu/drm/ast/ast_main.c | 24 ------------------------
 2 files changed, 30 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index ff161bd622f3..f44150ff5483 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -137,8 +137,6 @@ struct ast_private {
 int ast_driver_load(struct drm_device *dev, unsigned long flags);
 void ast_driver_unload(struct drm_device *dev);
 
-struct ast_gem_object;
-
 #define AST_IO_AR_PORT_WRITE		(0x40)
 #define AST_IO_MISC_PORT_WRITE		(0x42)
 #define AST_IO_VGA_ENABLE_PORT		(0x43)
@@ -289,10 +287,6 @@ extern void ast_mode_fini(struct drm_device *dev);
 int ast_mm_init(struct ast_private *ast);
 void ast_mm_fini(struct ast_private *ast);
 
-int ast_gem_create(struct drm_device *dev,
-		   u32 size, bool iskernel,
-		   struct drm_gem_object **obj);
-
 /* ast post */
 void ast_enable_vga(struct drm_device *dev);
 void ast_enable_mmio(struct drm_device *dev);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 21715d6a9b56..3a9b4cb73f2f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -535,27 +535,3 @@ void ast_driver_unload(struct drm_device *dev)
 	pci_iounmap(dev->pdev, ast->regs);
 	kfree(ast);
 }
-
-int ast_gem_create(struct drm_device *dev,
-		   u32 size, bool iskernel,
-		   struct drm_gem_object **obj)
-{
-	struct drm_gem_vram_object *gbo;
-	int ret;
-
-	*obj = NULL;
-
-	size = roundup(size, PAGE_SIZE);
-	if (size == 0)
-		return -EINVAL;
-
-	gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0, false);
-	if (IS_ERR(gbo)) {
-		ret = PTR_ERR(gbo);
-		if (ret != -ERESTARTSYS)
-			DRM_ERROR("failed to allocate GEM object\n");
-		return ret;
-	}
-	*obj = &gbo->bo.base;
-	return 0;
-}
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/9] drm/ast: Remove last traces of struct ast_gem_object

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:20PM +0100, Thomas Zimmermann wrote:
> The ast driver has switched to struct drm_vram_gem_object a while ago.
> This patch removes a function and forward declaration that were forgotten
> before.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> ---
>  drivers/gpu/drm/ast/ast_drv.h  |  6 ------
>  drivers/gpu/drm/ast/ast_main.c | 24 ------------------------
>  2 files changed, 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index ff161bd622f3..f44150ff5483 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -137,8 +137,6 @@ struct ast_private {
>  int ast_driver_load(struct drm_device *dev, unsigned long flags);
>  void ast_driver_unload(struct drm_device *dev);
>  
> -struct ast_gem_object;
> -
>  #define AST_IO_AR_PORT_WRITE		(0x40)
>  #define AST_IO_MISC_PORT_WRITE		(0x42)
>  #define AST_IO_VGA_ENABLE_PORT		(0x43)
> @@ -289,10 +287,6 @@ extern void ast_mode_fini(struct drm_device *dev);
>  int ast_mm_init(struct ast_private *ast);
>  void ast_mm_fini(struct ast_private *ast);
>  
> -int ast_gem_create(struct drm_device *dev,
> -		   u32 size, bool iskernel,
> -		   struct drm_gem_object **obj);
> -
>  /* ast post */
>  void ast_enable_vga(struct drm_device *dev);
>  void ast_enable_mmio(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 21715d6a9b56..3a9b4cb73f2f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -535,27 +535,3 @@ void ast_driver_unload(struct drm_device *dev)
>  	pci_iounmap(dev->pdev, ast->regs);
>  	kfree(ast);
>  }
> -
> -int ast_gem_create(struct drm_device *dev,
> -		   u32 size, bool iskernel,
> -		   struct drm_gem_object **obj)
> -{
> -	struct drm_gem_vram_object *gbo;
> -	int ret;
> -
> -	*obj = NULL;
> -
> -	size = roundup(size, PAGE_SIZE);
> -	if (size == 0)
> -		return -EINVAL;
> -
> -	gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0, false);
> -	if (IS_ERR(gbo)) {
> -		ret = PTR_ERR(gbo);
> -		if (ret != -ERESTARTSYS)
> -			DRM_ERROR("failed to allocate GEM object\n");
> -		return ret;
> -	}
> -	*obj = &gbo->bo.base;
> -	return 0;
> -}
> -- 
> 2.23.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/9] drm/ast: Check video-mode requirements against VRAM size

[Thomas Zimmermann]

Each video mode's primary plane requires a minimum amount of video
memory. For double buffering, this is at most half the available
VRAM. Check this constraint.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_main.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 3a9b4cb73f2f..48d57ab42955 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -387,8 +387,31 @@ static int ast_get_dram_info(struct drm_device *dev)
 	return 0;
 }
 
+enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
+						const struct drm_display_mode *mode)
+{
+	static const unsigned long max_bpp = 4; /* DRM_FORMAT_XRGBA8888 */
+
+	struct ast_private *ast = dev->dev_private;
+	unsigned long fbsize, fbpages, max_fbpages;
+
+	/* To support double buffering, a framebuffer may not
+	 * consume more than half of the available VRAM.
+	 */
+	max_fbpages = (ast->vram_size / 2) >> PAGE_SHIFT;
+
+	fbsize = mode->hdisplay * mode->vdisplay * max_bpp;
+	fbpages = DIV_ROUND_UP(fbsize, PAGE_SIZE);
+
+	if (fbpages > max_fbpages)
+		return MODE_MEM;
+
+	return MODE_OK;
+}
+
 static const struct drm_mode_config_funcs ast_mode_funcs = {
-	.fb_create = drm_gem_fb_create
+	.fb_create = drm_gem_fb_create,
+	.mode_valid = ast_mode_config_mode_valid,
 };
 
 static u32 ast_get_vram_info(struct drm_device *dev)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/9] drm/ast: Check video-mode requirements against VRAM size

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:21PM +0100, Thomas Zimmermann wrote:
> Each video mode's primary plane requires a minimum amount of video
> memory. For double buffering, this is at most half the available
> VRAM. Check this constraint.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/ast/ast_main.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 3a9b4cb73f2f..48d57ab42955 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -387,8 +387,31 @@ static int ast_get_dram_info(struct drm_device *dev)
>  	return 0;
>  }
>  
> +enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
> +						const struct drm_display_mode *mode)
> +{
> +	static const unsigned long max_bpp = 4; /* DRM_FORMAT_XRGBA8888 */
> +
> +	struct ast_private *ast = dev->dev_private;
> +	unsigned long fbsize, fbpages, max_fbpages;
> +
> +	/* To support double buffering, a framebuffer may not
> +	 * consume more than half of the available VRAM.
> +	 */
> +	max_fbpages = (ast->vram_size / 2) >> PAGE_SHIFT;
> +
> +	fbsize = mode->hdisplay * mode->vdisplay * max_bpp;
> +	fbpages = DIV_ROUND_UP(fbsize, PAGE_SIZE);
> +
> +	if (fbpages > max_fbpages)
> +		return MODE_MEM;
> +
> +	return MODE_OK;
> +}
> +
>  static const struct drm_mode_config_funcs ast_mode_funcs = {
> -	.fb_create = drm_gem_fb_create
> +	.fb_create = drm_gem_fb_create,
> +	.mode_valid = ast_mode_config_mode_valid,
>  };
>  
>  static u32 ast_get_vram_info(struct drm_device *dev)
> -- 
> 2.23.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/9] drm/ast: Don't clear base address and offset with default values

[Thomas Zimmermann]

The content of the base-address and offset registers are state of
the primary plane. Clearing it to default values will interfere with
plane functions for atomic mode setting.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index b13eaa2619ab..b3f82c2d274d 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -253,9 +253,13 @@ static void ast_set_std_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
 		ast_set_index_reg(ast, AST_IO_SEQ_PORT, (i + 1) , jreg);
 	}
 
-	/* Set CRTC */
+	/* Set CRTC; except base address and offset */
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f, 0x00);
-	for (i = 0; i < 25; i++)
+	for (i = 0; i < 12; i++)
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, stdtable->crtc[i]);
+	for (i = 14; i < 19; i++)
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, stdtable->crtc[i]);
+	for (i = 20; i < 25; i++)
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, stdtable->crtc[i]);
 
 	/* set AR */
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/9] drm/ast: Don't clear base address and offset with default values

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:22PM +0100, Thomas Zimmermann wrote:
> The content of the base-address and offset registers are state of
> the primary plane. Clearing it to default values will interfere with
> plane functions for atomic mode setting.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> ---
>  drivers/gpu/drm/ast/ast_mode.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index b13eaa2619ab..b3f82c2d274d 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -253,9 +253,13 @@ static void ast_set_std_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
>  		ast_set_index_reg(ast, AST_IO_SEQ_PORT, (i + 1) , jreg);
>  	}
>  
> -	/* Set CRTC */
> +	/* Set CRTC; except base address and offset */
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f, 0x00);
> -	for (i = 0; i < 25; i++)
> +	for (i = 0; i < 12; i++)
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, stdtable->crtc[i]);
> +	for (i = 14; i < 19; i++)
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, stdtable->crtc[i]);
> +	for (i = 20; i < 25; i++)
>  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, stdtable->crtc[i]);
>  
>  	/* set AR */
> -- 
> 2.23.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/ast: Split ast_set_ext_reg() into color and threshold function

[Thomas Zimmermann]

In ast_set_ext_reg() sets several framebuffer options and CRT threshold
parameters. The former is mostly state of the primary plane; the latter
is constant. Hence, split the function in two and make it work with
atomic modesetting.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index b3f82c2d274d..5feb687191e0 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -419,11 +419,10 @@ static void ast_set_dclk_reg(struct drm_device *dev, struct drm_display_mode *mo
 			       ((clk_info->param3 & 0x3) << 4));
 }
 
-static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			     struct ast_vbios_mode_info *vbios_mode)
+static void ast_set_color_reg(struct drm_crtc *crtc,
+			      const struct drm_framebuffer *fb)
 {
 	struct ast_private *ast = crtc->dev->dev_private;
-	const struct drm_framebuffer *fb = crtc->primary->fb;
 	u8 jregA0 = 0, jregA3 = 0, jregA8 = 0;
 
 	switch (fb->format->cpp[0] * 8) {
@@ -448,6 +447,11 @@ static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa0, 0x8f, jregA0);
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa3, 0xf0, jregA3);
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd, jregA8);
+}
+
+static void ast_set_crtthd_reg(struct drm_crtc *crtc)
+{
+	struct ast_private *ast = crtc->dev->dev_private;
 
 	/* Set Threshold */
 	if (ast->chip == AST2300 || ast->chip == AST2400 ||
@@ -467,7 +471,7 @@ static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
 }
 
 static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mode,
-		      struct ast_vbios_mode_info *vbios_mode)
+			     struct ast_vbios_mode_info *vbios_mode)
 {
 	struct ast_private *ast = dev->dev_private;
 	u8 jreg;
@@ -595,7 +599,8 @@ static int ast_crtc_mode_set(struct drm_crtc *crtc,
 	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_ext_reg(crtc, 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);
 
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/ast: Split ast_set_ext_reg() into color and threshold function

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:23PM +0100, Thomas Zimmermann wrote:
> In ast_set_ext_reg() sets several framebuffer options and CRT threshold
> parameters. The former is mostly state of the primary plane; the latter
> is constant. Hence, split the function in two and make it work with
> atomic modesetting.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> ---
>  drivers/gpu/drm/ast/ast_mode.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index b3f82c2d274d..5feb687191e0 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -419,11 +419,10 @@ static void ast_set_dclk_reg(struct drm_device *dev, struct drm_display_mode *mo
>  			       ((clk_info->param3 & 0x3) << 4));
>  }
>  
> -static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
> -			     struct ast_vbios_mode_info *vbios_mode)
> +static void ast_set_color_reg(struct drm_crtc *crtc,
> +			      const struct drm_framebuffer *fb)
>  {
>  	struct ast_private *ast = crtc->dev->dev_private;
> -	const struct drm_framebuffer *fb = crtc->primary->fb;
>  	u8 jregA0 = 0, jregA3 = 0, jregA8 = 0;
>  
>  	switch (fb->format->cpp[0] * 8) {
> @@ -448,6 +447,11 @@ static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa0, 0x8f, jregA0);
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa3, 0xf0, jregA3);
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd, jregA8);
> +}
> +
> +static void ast_set_crtthd_reg(struct drm_crtc *crtc)
> +{
> +	struct ast_private *ast = crtc->dev->dev_private;
>  
>  	/* Set Threshold */
>  	if (ast->chip == AST2300 || ast->chip == AST2400 ||
> @@ -467,7 +471,7 @@ static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
>  }
>  
>  static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mode,
> -		      struct ast_vbios_mode_info *vbios_mode)
> +			     struct ast_vbios_mode_info *vbios_mode)
>  {
>  	struct ast_private *ast = dev->dev_private;
>  	u8 jreg;
> @@ -595,7 +599,8 @@ static int ast_crtc_mode_set(struct drm_crtc *crtc,
>  	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_ext_reg(crtc, 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);
>  
> -- 
> 2.23.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] drm/ast: Split ast_set_vbios_mode_info()

[Thomas Zimmermann]

The implementation of ast_set_vbios_mode() converts a DRM display mode
and framebuffer into an adjusted mode and stores information for the
video BIOS to several scratch regsiters.

Here we split the function into individual functions that do the
conversion, set the VBIOS mode information and format information.
This makes it compatible with support for primary planes and atomic
modesetting.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 111 +++++++++++++++++++++------------
 1 file changed, 72 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 5feb687191e0..c4c9fca0f00c 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -82,13 +82,12 @@ static void ast_crtc_load_lut(struct drm_crtc *crtc)
 		ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
 }
 
-static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mode *mode,
+static bool ast_get_vbios_mode_info(const struct drm_framebuffer *fb,
+				    const struct drm_display_mode *mode,
 				    struct drm_display_mode *adjusted_mode,
 				    struct ast_vbios_mode_info *vbios_mode)
 {
-	struct ast_private *ast = crtc->dev->dev_private;
-	const struct drm_framebuffer *fb = crtc->primary->fb;
-	u32 refresh_rate_index = 0, mode_id, color_index, refresh_rate;
+	u32 refresh_rate_index = 0, refresh_rate;
 	const struct ast_vbios_enhtable *best = NULL;
 	u32 hborder, vborder;
 	bool check_sync;
@@ -96,22 +95,19 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 	switch (fb->format->cpp[0] * 8) {
 	case 8:
 		vbios_mode->std_table = &vbios_stdtable[VGAModeIndex];
-		color_index = VGAModeIndex - 1;
 		break;
 	case 16:
 		vbios_mode->std_table = &vbios_stdtable[HiCModeIndex];
-		color_index = HiCModeIndex;
 		break;
 	case 24:
 	case 32:
 		vbios_mode->std_table = &vbios_stdtable[TrueCModeIndex];
-		color_index = TrueCModeIndex;
 		break;
 	default:
 		return false;
 	}
 
-	switch (crtc->mode.crtc_hdisplay) {
+	switch (mode->crtc_hdisplay) {
 	case 640:
 		vbios_mode->enh_table = &res_640x480[refresh_rate_index];
 		break;
@@ -122,7 +118,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 		vbios_mode->enh_table = &res_1024x768[refresh_rate_index];
 		break;
 	case 1280:
-		if (crtc->mode.crtc_vdisplay == 800)
+		if (mode->crtc_vdisplay == 800)
 			vbios_mode->enh_table = &res_1280x800[refresh_rate_index];
 		else
 			vbios_mode->enh_table = &res_1280x1024[refresh_rate_index];
@@ -134,7 +130,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 		vbios_mode->enh_table = &res_1440x900[refresh_rate_index];
 		break;
 	case 1600:
-		if (crtc->mode.crtc_vdisplay == 900)
+		if (mode->crtc_vdisplay == 900)
 			vbios_mode->enh_table = &res_1600x900[refresh_rate_index];
 		else
 			vbios_mode->enh_table = &res_1600x1200[refresh_rate_index];
@@ -143,7 +139,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 		vbios_mode->enh_table = &res_1680x1050[refresh_rate_index];
 		break;
 	case 1920:
-		if (crtc->mode.crtc_vdisplay == 1080)
+		if (mode->crtc_vdisplay == 1080)
 			vbios_mode->enh_table = &res_1920x1080[refresh_rate_index];
 		else
 			vbios_mode->enh_table = &res_1920x1200[refresh_rate_index];
@@ -154,7 +150,8 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 
 	refresh_rate = drm_mode_vrefresh(mode);
 	check_sync = vbios_mode->enh_table->flags & WideScreenMode;
-	do {
+
+	while (1) {
 		const struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
 
 		while (loop->refresh_rate != 0xff) {
@@ -178,7 +175,8 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 		if (best || !check_sync)
 			break;
 		check_sync = 0;
-	} while (1);
+	}
+
 	if (best)
 		vbios_mode->enh_table = best;
 
@@ -203,34 +201,65 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 					 vbios_mode->enh_table->vfp +
 					 vbios_mode->enh_table->vsync);
 
-	refresh_rate_index = vbios_mode->enh_table->refresh_rate_index;
-	mode_id = vbios_mode->enh_table->mode_id;
+	return true;
+}
 
-	if (ast->chip == AST1180) {
-		/* TODO 1180 */
-	} else {
-		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8c, (u8)((color_index & 0xf) << 4));
-		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8d, refresh_rate_index & 0xff);
-		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8e, mode_id & 0xff);
-
-		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0x00);
-		if (vbios_mode->enh_table->flags & NewModeInfo) {
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0xa8);
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x92,
-					  fb->format->cpp[0] * 8);
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x93, adjusted_mode->clock / 1000);
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x94, adjusted_mode->crtc_hdisplay);
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x95, adjusted_mode->crtc_hdisplay >> 8);
-
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x96, adjusted_mode->crtc_vdisplay);
-			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x97, adjusted_mode->crtc_vdisplay >> 8);
-		}
+static void ast_set_vbios_color_reg(struct drm_crtc *crtc,
+				    const struct drm_framebuffer *fb,
+				    const struct ast_vbios_mode_info *vbios_mode)
+{
+	struct ast_private *ast = crtc->dev->dev_private;
+	u32 color_index;
+
+	switch (fb->format->cpp[0]) {
+	case 1:
+		color_index = VGAModeIndex - 1;
+		break;
+	case 2:
+		color_index = HiCModeIndex;
+		break;
+	case 3:
+	case 4:
+		color_index = TrueCModeIndex;
+	default:
+		return;
 	}
 
-	return true;
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8c, (u8)((color_index & 0x0f) << 4));
+
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0x00);
+
+	if (vbios_mode->enh_table->flags & NewModeInfo) {
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0xa8);
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x92, fb->format->cpp[0] * 8);
+	}
+}
+
+static void ast_set_vbios_mode_reg(struct drm_crtc *crtc,
+				   const struct drm_display_mode *adjusted_mode,
+				   const struct ast_vbios_mode_info *vbios_mode)
+{
+	struct ast_private *ast = crtc->dev->dev_private;
+	u32 refresh_rate_index, mode_id;
+
+	refresh_rate_index = vbios_mode->enh_table->refresh_rate_index;
+	mode_id = vbios_mode->enh_table->mode_id;
+
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8d, refresh_rate_index & 0xff);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8e, mode_id & 0xff);
 
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0x00);
 
+	if (vbios_mode->enh_table->flags & NewModeInfo) {
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0xa8);
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x93, adjusted_mode->clock / 1000);
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x94, adjusted_mode->crtc_hdisplay);
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x95, adjusted_mode->crtc_hdisplay >> 8);
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x96, adjusted_mode->crtc_vdisplay);
+		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x97, adjusted_mode->crtc_vdisplay >> 8);
+	}
 }
+
 static void ast_set_std_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			    struct ast_vbios_mode_info *vbios_mode)
 {
@@ -581,20 +610,24 @@ static int ast_crtc_mode_set(struct drm_crtc *crtc,
 {
 	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 ret;
+	bool succ;
+
 	if (ast->chip == AST1180) {
 		DRM_ERROR("AST 1180 modesetting not supported\n");
 		return -EINVAL;
 	}
 
-	ret = ast_get_vbios_mode_info(crtc, mode, adjusted_mode, &vbios_mode);
-	if (ret == false)
+	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);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/ast: Split ast_set_vbios_mode_info()

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:24PM +0100, Thomas Zimmermann wrote:
> The implementation of ast_set_vbios_mode() converts a DRM display mode
> and framebuffer into an adjusted mode and stores information for the
> video BIOS to several scratch regsiters.
> 
> Here we split the function into individual functions that do the
> conversion, set the VBIOS mode information and format information.
> This makes it compatible with support for primary planes and atomic
> modesetting.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/ast/ast_mode.c | 111 +++++++++++++++++++++------------
>  1 file changed, 72 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 5feb687191e0..c4c9fca0f00c 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -82,13 +82,12 @@ static void ast_crtc_load_lut(struct drm_crtc *crtc)
>  		ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
>  }
>  
> -static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mode *mode,
> +static bool ast_get_vbios_mode_info(const struct drm_framebuffer *fb,
> +				    const struct drm_display_mode *mode,
>  				    struct drm_display_mode *adjusted_mode,
>  				    struct ast_vbios_mode_info *vbios_mode)
>  {
> -	struct ast_private *ast = crtc->dev->dev_private;
> -	const struct drm_framebuffer *fb = crtc->primary->fb;
> -	u32 refresh_rate_index = 0, mode_id, color_index, refresh_rate;
> +	u32 refresh_rate_index = 0, refresh_rate;
>  	const struct ast_vbios_enhtable *best = NULL;
>  	u32 hborder, vborder;
>  	bool check_sync;
> @@ -96,22 +95,19 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  	switch (fb->format->cpp[0] * 8) {
>  	case 8:
>  		vbios_mode->std_table = &vbios_stdtable[VGAModeIndex];
> -		color_index = VGAModeIndex - 1;
>  		break;
>  	case 16:
>  		vbios_mode->std_table = &vbios_stdtable[HiCModeIndex];
> -		color_index = HiCModeIndex;
>  		break;
>  	case 24:
>  	case 32:
>  		vbios_mode->std_table = &vbios_stdtable[TrueCModeIndex];
> -		color_index = TrueCModeIndex;
>  		break;
>  	default:
>  		return false;
>  	}
>  
> -	switch (crtc->mode.crtc_hdisplay) {
> +	switch (mode->crtc_hdisplay) {
>  	case 640:
>  		vbios_mode->enh_table = &res_640x480[refresh_rate_index];
>  		break;
> @@ -122,7 +118,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  		vbios_mode->enh_table = &res_1024x768[refresh_rate_index];
>  		break;
>  	case 1280:
> -		if (crtc->mode.crtc_vdisplay == 800)
> +		if (mode->crtc_vdisplay == 800)
>  			vbios_mode->enh_table = &res_1280x800[refresh_rate_index];
>  		else
>  			vbios_mode->enh_table = &res_1280x1024[refresh_rate_index];
> @@ -134,7 +130,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  		vbios_mode->enh_table = &res_1440x900[refresh_rate_index];
>  		break;
>  	case 1600:
> -		if (crtc->mode.crtc_vdisplay == 900)
> +		if (mode->crtc_vdisplay == 900)
>  			vbios_mode->enh_table = &res_1600x900[refresh_rate_index];
>  		else
>  			vbios_mode->enh_table = &res_1600x1200[refresh_rate_index];
> @@ -143,7 +139,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  		vbios_mode->enh_table = &res_1680x1050[refresh_rate_index];
>  		break;
>  	case 1920:
> -		if (crtc->mode.crtc_vdisplay == 1080)
> +		if (mode->crtc_vdisplay == 1080)
>  			vbios_mode->enh_table = &res_1920x1080[refresh_rate_index];
>  		else
>  			vbios_mode->enh_table = &res_1920x1200[refresh_rate_index];
> @@ -154,7 +150,8 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  
>  	refresh_rate = drm_mode_vrefresh(mode);
>  	check_sync = vbios_mode->enh_table->flags & WideScreenMode;
> -	do {
> +
> +	while (1) {
>  		const struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
>  
>  		while (loop->refresh_rate != 0xff) {
> @@ -178,7 +175,8 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  		if (best || !check_sync)
>  			break;
>  		check_sync = 0;
> -	} while (1);
> +	}
> +
>  	if (best)
>  		vbios_mode->enh_table = best;
>  
> @@ -203,34 +201,65 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
>  					 vbios_mode->enh_table->vfp +
>  					 vbios_mode->enh_table->vsync);
>  
> -	refresh_rate_index = vbios_mode->enh_table->refresh_rate_index;
> -	mode_id = vbios_mode->enh_table->mode_id;
> +	return true;
> +}
>  
> -	if (ast->chip == AST1180) {
> -		/* TODO 1180 */
> -	} else {
> -		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8c, (u8)((color_index & 0xf) << 4));
> -		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8d, refresh_rate_index & 0xff);
> -		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8e, mode_id & 0xff);
> -
> -		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0x00);
> -		if (vbios_mode->enh_table->flags & NewModeInfo) {
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0xa8);
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x92,
> -					  fb->format->cpp[0] * 8);
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x93, adjusted_mode->clock / 1000);
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x94, adjusted_mode->crtc_hdisplay);
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x95, adjusted_mode->crtc_hdisplay >> 8);
> -
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x96, adjusted_mode->crtc_vdisplay);
> -			ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x97, adjusted_mode->crtc_vdisplay >> 8);
> -		}
> +static void ast_set_vbios_color_reg(struct drm_crtc *crtc,
> +				    const struct drm_framebuffer *fb,
> +				    const struct ast_vbios_mode_info *vbios_mode)
> +{
> +	struct ast_private *ast = crtc->dev->dev_private;
> +	u32 color_index;
> +
> +	switch (fb->format->cpp[0]) {
> +	case 1:
> +		color_index = VGAModeIndex - 1;
> +		break;
> +	case 2:
> +		color_index = HiCModeIndex;
> +		break;
> +	case 3:
> +	case 4:
> +		color_index = TrueCModeIndex;
> +	default:
> +		return;
>  	}
>  
> -	return true;
> +	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8c, (u8)((color_index & 0x0f) << 4));
> +
> +	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0x00);
> +
> +	if (vbios_mode->enh_table->flags & NewModeInfo) {
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0xa8);
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x92, fb->format->cpp[0] * 8);
> +	}
> +}
> +
> +static void ast_set_vbios_mode_reg(struct drm_crtc *crtc,
> +				   const struct drm_display_mode *adjusted_mode,
> +				   const struct ast_vbios_mode_info *vbios_mode)
> +{
> +	struct ast_private *ast = crtc->dev->dev_private;
> +	u32 refresh_rate_index, mode_id;
> +
> +	refresh_rate_index = vbios_mode->enh_table->refresh_rate_index;
> +	mode_id = vbios_mode->enh_table->mode_id;
> +
> +	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8d, refresh_rate_index & 0xff);
> +	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x8e, mode_id & 0xff);
>  
> +	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0x00);
>  
> +	if (vbios_mode->enh_table->flags & NewModeInfo) {
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x91, 0xa8);
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x93, adjusted_mode->clock / 1000);
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x94, adjusted_mode->crtc_hdisplay);
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x95, adjusted_mode->crtc_hdisplay >> 8);
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x96, adjusted_mode->crtc_vdisplay);
> +		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x97, adjusted_mode->crtc_vdisplay >> 8);
> +	}
>  }
> +
>  static void ast_set_std_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  			    struct ast_vbios_mode_info *vbios_mode)
>  {
> @@ -581,20 +610,24 @@ static int ast_crtc_mode_set(struct drm_crtc *crtc,
>  {
>  	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 ret;
> +	bool succ;
> +
>  	if (ast->chip == AST1180) {
>  		DRM_ERROR("AST 1180 modesetting not supported\n");
>  		return -EINVAL;
>  	}
>  
> -	ret = ast_get_vbios_mode_info(crtc, mode, adjusted_mode, &vbios_mode);
> -	if (ret == false)
> +	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);
> -- 
> 2.23.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/9] drm/ast: Add primary plane

[Thomas Zimmermann]

Like the original mode-setting code, the primary plane supports XRGB888,
RGB565 and C8. The plane itself only pins BOs and sets the base address
and scanline offset. The mode-setting code will be located in the CRTC's
atomic helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |  2 +
 drivers/gpu/drm/ast/ast_mode.c | 96 +++++++++++++++++++++++++++++++++-
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index f44150ff5483..13560622f22a 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -121,6 +121,8 @@ struct ast_private {
 		unsigned int next_index;
 	} cursor;
 
+	struct drm_plane primary_plane;
+
 	bool support_wide_screen;
 	enum {
 		ast_use_p2a,
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c4c9fca0f00c..dbbfc4d44cb5 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -31,6 +31,8 @@
 #include <linux/export.h>
 #include <linux/pci.h>
 
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fourcc.h>
@@ -51,6 +53,7 @@ static int ast_cursor_set(struct drm_crtc *crtc,
 static int ast_cursor_move(struct drm_crtc *crtc,
 			   int x, int y);
 
+
 static inline void ast_load_palette_index(struct ast_private *ast,
 				     u8 index, u8 red, u8 green,
 				     u8 blue)
@@ -538,6 +541,63 @@ static void ast_set_start_address_crt1(struct drm_crtc *crtc, unsigned offset)
 
 }
 
+/*
+ * Primary plane
+ */
+
+int ast_primary_plane_helper_atomic_check(struct drm_plane *plane,
+					  struct drm_plane_state *state)
+{
+	return 0;
+}
+
+void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
+					    struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = plane->state;
+	struct drm_crtc *crtc = state->crtc;
+	struct drm_gem_vram_object *gbo;
+	s64 gpu_addr;
+
+	if (!crtc || !state->fb)
+		return;
+
+	gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
+	gpu_addr = drm_gem_vram_offset(gbo);
+	if (WARN_ON_ONCE(gpu_addr < 0))
+		return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
+
+	ast_set_offset_reg(crtc);
+	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
+}
+
+static const struct drm_plane_helper_funcs ast_primary_plane_helper_funcs = {
+	.prepare_fb = drm_gem_vram_plane_helper_prepare_fb,
+	.cleanup_fb = drm_gem_vram_plane_helper_cleanup_fb,
+	.atomic_check = ast_primary_plane_helper_atomic_check,
+	.atomic_update = ast_primary_plane_helper_atomic_update,
+};
+
+static const struct drm_plane_funcs ast_primary_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	.reset = drm_atomic_helper_plane_reset,
+	.set_property = NULL,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.atomic_set_property = NULL,
+	.atomic_get_property = NULL,
+	.late_register = NULL,
+	.early_unregister = NULL,
+	.atomic_print_state = NULL,
+	.format_mod_supported = NULL,
+};
+
+/*
+ * CRTC
+ */
+
 static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct ast_private *ast = crtc->dev->dev_private;
@@ -711,16 +771,26 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
 
 static int ast_crtc_init(struct drm_device *dev)
 {
+	struct ast_private *ast = dev->dev_private;
 	struct ast_crtc *crtc;
+	int ret;
 
 	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
 	if (!crtc)
 		return -ENOMEM;
 
-	drm_crtc_init(dev, &crtc->base, &ast_crtc_funcs);
+	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
+					NULL, &ast_crtc_funcs, NULL);
+	if (ret)
+		goto err_kfree;
+
 	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
 	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
 	return 0;
+
+err_kfree:
+	kfree(crtc);
+	return ret;
 }
 
 static void ast_encoder_destroy(struct drm_encoder *encoder)
@@ -754,7 +824,6 @@ 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,
@@ -976,10 +1045,33 @@ static void ast_cursor_fini(struct drm_device *dev)
 
 int ast_mode_init(struct drm_device *dev)
 {
+	static const uint32_t primary_plane_formats[] = {
+		DRM_FORMAT_XRGB8888,
+		DRM_FORMAT_RGB565,
+		DRM_FORMAT_C8,
+	};
+
+	struct ast_private *ast = dev->dev_private;
+	int ret;
+
+	memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
+	ret = drm_universal_plane_init(dev, &ast->primary_plane, 0x01,
+				       &ast_primary_plane_funcs,
+				       primary_plane_formats,
+				       ARRAY_SIZE(primary_plane_formats),
+				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		DRM_ERROR("ast: drm_universal_plane_init() failed: %d\n", ret);
+		return ret;
+	}
+	drm_plane_helper_add(&ast->primary_plane,
+			     &ast_primary_plane_helper_funcs);
+
 	ast_cursor_init(dev);
 	ast_crtc_init(dev);
 	ast_encoder_init(dev);
 	ast_connector_init(dev);
+
 	return 0;
 }
 
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/9] drm/ast: Add primary plane

[Gerd Hoffmann]

> +static const struct drm_plane_funcs ast_primary_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = drm_plane_cleanup,
> +	.reset = drm_atomic_helper_plane_reset,
> +	.set_property = NULL,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +	.atomic_set_property = NULL,
> +	.atomic_get_property = NULL,

It's not needed to explicitly set optional function pointers to NULL.

>  static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
>  	.dpms = ast_encoder_dpms,
>  	.prepare = ast_encoder_prepare,
> @@ -976,10 +1045,33 @@ static void ast_cursor_fini(struct drm_device *dev)
>  
>  int ast_mode_init(struct drm_device *dev)
>  {
> +	static const uint32_t primary_plane_formats[] = {
> +		DRM_FORMAT_XRGB8888,
> +		DRM_FORMAT_RGB565,
> +		DRM_FORMAT_C8,
> +	};

I'd suggest to move this out of the function.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/9] drm/ast: Add primary plane

[Daniel Vetter]

On Tue, Nov 05, 2019 at 10:51:02AM +0100, Gerd Hoffmann wrote:
> > +static const struct drm_plane_funcs ast_primary_plane_funcs = {
> > +	.update_plane = drm_atomic_helper_update_plane,
> > +	.disable_plane = drm_atomic_helper_disable_plane,
> > +	.destroy = drm_plane_cleanup,
> > +	.reset = drm_atomic_helper_plane_reset,
> > +	.set_property = NULL,
> > +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> > +	.atomic_set_property = NULL,
> > +	.atomic_get_property = NULL,
> 
> It's not needed to explicitly set optional function pointers to NULL.

Yeah leaving them out completely helps a lot with grepping over all the
drivers to find users of stuff.
-Daniel

> 
> >  static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
> >  	.dpms = ast_encoder_dpms,
> >  	.prepare = ast_encoder_prepare,
> > @@ -976,10 +1045,33 @@ static void ast_cursor_fini(struct drm_device *dev)
> >  
> >  int ast_mode_init(struct drm_device *dev)
> >  {
> > +	static const uint32_t primary_plane_formats[] = {
> > +		DRM_FORMAT_XRGB8888,
> > +		DRM_FORMAT_RGB565,
> > +		DRM_FORMAT_C8,
> > +	};
> 
> I'd suggest to move this out of the function.
> 
> cheers,
>   Gerd
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/9] drm/ast: Add primary plane

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1511 bytes --]

Hi

Am 05.11.19 um 10:51 schrieb Gerd Hoffmann:
>> +static const struct drm_plane_funcs ast_primary_plane_funcs = {
>> +	.update_plane = drm_atomic_helper_update_plane,
>> +	.disable_plane = drm_atomic_helper_disable_plane,
>> +	.destroy = drm_plane_cleanup,
>> +	.reset = drm_atomic_helper_plane_reset,
>> +	.set_property = NULL,
>> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>> +	.atomic_set_property = NULL,
>> +	.atomic_get_property = NULL,
> 
> It's not needed to explicitly set optional function pointers to NULL.

Sure. These NULL assignments helped me with keeping track of the work
during the conversion. I forgot to remove them here and in the other patch.

Best regards
Thomas

> 
>>  static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
>>  	.dpms = ast_encoder_dpms,
>>  	.prepare = ast_encoder_prepare,
>> @@ -976,10 +1045,33 @@ static void ast_cursor_fini(struct drm_device *dev)
>>  
>>  int ast_mode_init(struct drm_device *dev)
>>  {
>> +	static const uint32_t primary_plane_formats[] = {
>> +		DRM_FORMAT_XRGB8888,
>> +		DRM_FORMAT_RGB565,
>> +		DRM_FORMAT_C8,
>> +	};
> 
> I'd suggest to move this out of the function.
> 
> cheers,
>   Gerd
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/9] drm/ast: Add CRTC helpers for atomic modesetting

[Thomas Zimmermann]

As the CRTC code has already been prepared for a split between mode
setting and plane handling, most of the CRTC's atomic modesetting is
build upon primitives of the non-atomic implementation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 105 ++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index dbbfc4d44cb5..7667f4502eb9 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -728,6 +728,103 @@ static void ast_crtc_commit(struct drm_crtc *crtc)
 	ast_crtc_load_lut(crtc);
 }
 
+static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
+					struct drm_crtc_state *state)
+{
+	struct ast_private *ast = crtc->dev->dev_private;
+	struct drm_plane_state *plane_state;
+	bool succ;
+	struct drm_display_mode adjusted_mode;
+	struct ast_vbios_mode_info vbios_mode;
+
+	if (ast->chip == AST1180) {
+		DRM_ERROR("AST 1180 modesetting not supported\n");
+		return -EINVAL;
+	}
+
+	plane_state = crtc->primary->state;
+
+	if (plane_state && plane_state->fb) {
+		succ = ast_get_vbios_mode_info(plane_state->fb, &state->mode,
+					       &adjusted_mode, &vbios_mode);
+		if (!succ)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
+					 struct drm_crtc_state *old_crtc_state)
+{
+	struct ast_private *ast = crtc->dev->dev_private;
+
+	ast_open_key(ast);
+}
+
+static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
+					 struct drm_crtc_state *old_crtc_state)
+{
+	const struct drm_framebuffer *fb = crtc->primary->state->fb;
+	struct drm_display_mode adjusted_mode;
+	struct ast_vbios_mode_info vbios_mode;
+	bool succ;
+
+	crtc->state->no_vblank = true;
+
+	if (!fb)
+		return;
+
+	ast_set_color_reg(crtc, fb);
+
+	memset(&adjusted_mode, 0, sizeof(adjusted_mode));
+	drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode);
+
+	succ = ast_get_vbios_mode_info(fb, &crtc->state->adjusted_mode,
+				       &adjusted_mode, &vbios_mode);
+	if (WARN_ON_ONCE(!succ))
+		return;
+
+	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
+}
+
+static void
+ast_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+			      struct drm_crtc_state *old_crtc_state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct ast_private *ast = crtc->dev->dev_private;
+	const struct drm_framebuffer *fb = crtc->primary->state->fb;
+	struct drm_display_mode adjusted_mode;
+	struct ast_vbios_mode_info vbios_mode;
+	bool succ;
+
+	memset(&adjusted_mode, 0, sizeof(adjusted_mode));
+	drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode);
+
+	succ = ast_get_vbios_mode_info(fb, &crtc->state->adjusted_mode,
+				       &adjusted_mode, &vbios_mode);
+	if (WARN_ON_ONCE(!succ))
+		return;
+
+	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_dclk_reg(dev, &adjusted_mode, &vbios_mode);
+	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_dpms(crtc, DRM_MODE_DPMS_ON);
+}
+
+static void
+ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+			       struct drm_crtc_state *old_crtc_state)
+{
+	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+}
 
 static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
 	.dpms = ast_crtc_dpms,
@@ -736,7 +833,11 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
 	.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,
+	.atomic_enable = ast_crtc_helper_atomic_enable,
+	.atomic_disable = ast_crtc_helper_atomic_disable,
 };
 
 static void ast_crtc_reset(struct drm_crtc *crtc)
@@ -767,6 +868,8 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
 	.set_config = drm_crtc_helper_set_config,
 	.gamma_set = ast_crtc_gamma_set,
 	.destroy = ast_crtc_destroy,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
 static int ast_crtc_init(struct drm_device *dev)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/9] drm/ast: Add CRTC helpers for atomic modesetting

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:26PM +0100, Thomas Zimmermann wrote:
> As the CRTC code has already been prepared for a split between mode
> setting and plane handling, most of the CRTC's atomic modesetting is
> build upon primitives of the non-atomic implementation.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> ---
>  drivers/gpu/drm/ast/ast_mode.c | 105 ++++++++++++++++++++++++++++++++-
>  1 file changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index dbbfc4d44cb5..7667f4502eb9 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -728,6 +728,103 @@ static void ast_crtc_commit(struct drm_crtc *crtc)
>  	ast_crtc_load_lut(crtc);
>  }
>  
> +static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> +					struct drm_crtc_state *state)
> +{
> +	struct ast_private *ast = crtc->dev->dev_private;
> +	struct drm_plane_state *plane_state;
> +	bool succ;
> +	struct drm_display_mode adjusted_mode;
> +	struct ast_vbios_mode_info vbios_mode;
> +
> +	if (ast->chip == AST1180) {
> +		DRM_ERROR("AST 1180 modesetting not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	plane_state = crtc->primary->state;
> +
> +	if (plane_state && plane_state->fb) {
> +		succ = ast_get_vbios_mode_info(plane_state->fb, &state->mode,
> +					       &adjusted_mode, &vbios_mode);
> +		if (!succ)
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
> +					 struct drm_crtc_state *old_crtc_state)
> +{
> +	struct ast_private *ast = crtc->dev->dev_private;
> +
> +	ast_open_key(ast);
> +}
> +
> +static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
> +					 struct drm_crtc_state *old_crtc_state)
> +{
> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
> +	struct drm_display_mode adjusted_mode;
> +	struct ast_vbios_mode_info vbios_mode;
> +	bool succ;
> +
> +	crtc->state->no_vblank = true;
> +
> +	if (!fb)
> +		return;
> +
> +	ast_set_color_reg(crtc, fb);
> +
> +	memset(&adjusted_mode, 0, sizeof(adjusted_mode));
> +	drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode);
> +
> +	succ = ast_get_vbios_mode_info(fb, &crtc->state->adjusted_mode,
> +				       &adjusted_mode, &vbios_mode);
> +	if (WARN_ON_ONCE(!succ))
> +		return;
> +
> +	ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
> +}
> +
> +static void
> +ast_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> +			      struct drm_crtc_state *old_crtc_state)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct ast_private *ast = crtc->dev->dev_private;
> +	const struct drm_framebuffer *fb = crtc->primary->state->fb;
> +	struct drm_display_mode adjusted_mode;
> +	struct ast_vbios_mode_info vbios_mode;
> +	bool succ;
> +
> +	memset(&adjusted_mode, 0, sizeof(adjusted_mode));
> +	drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode);
> +
> +	succ = ast_get_vbios_mode_info(fb, &crtc->state->adjusted_mode,
> +				       &adjusted_mode, &vbios_mode);
> +	if (WARN_ON_ONCE(!succ))
> +		return;
> +
> +	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_dclk_reg(dev, &adjusted_mode, &vbios_mode);
> +	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_dpms(crtc, DRM_MODE_DPMS_ON);
> +}
> +
> +static void
> +ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> +			       struct drm_crtc_state *old_crtc_state)
> +{
> +	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
> +}
>  
>  static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>  	.dpms = ast_crtc_dpms,
> @@ -736,7 +833,11 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>  	.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,
> +	.atomic_enable = ast_crtc_helper_atomic_enable,
> +	.atomic_disable = ast_crtc_helper_atomic_disable,
>  };
>  
>  static void ast_crtc_reset(struct drm_crtc *crtc)
> @@ -767,6 +868,8 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>  	.set_config = drm_crtc_helper_set_config,
>  	.gamma_set = ast_crtc_gamma_set,
>  	.destroy = ast_crtc_destroy,
> +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  };
>  
>  static int ast_crtc_init(struct drm_device *dev)
> -- 
> 2.23.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 8/9] drm/ast: Add cursor plane

[Thomas Zimmermann]

The cursor plane uses an internal format of ARGB4444. To userspace, we
announce ARGB8888 and do the transformation internally.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |   1 +
 drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
 2 files changed, 161 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 13560622f22a..49557a73390f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -122,6 +122,7 @@ struct ast_private {
 	} cursor;
 
 	struct drm_plane primary_plane;
+	struct drm_plane cursor_plane;
 
 	bool support_wide_screen;
 	enum {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 7667f4502eb9..f5f73200e8e4 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
 			   int x, int y);
 
 
+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);
+
 static inline void ast_load_palette_index(struct ast_private *ast,
 				     u8 index, u8 red, u8 green,
 				     u8 blue)
@@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
 	.format_mod_supported = NULL,
 };
 
+/*
+ * Cursor plane
+ */
+
+static int
+ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
+				   struct drm_plane_state *new_state)
+{
+	struct drm_framebuffer *fb = new_state->fb;
+	struct drm_crtc *crtc = new_state->crtc;
+	struct drm_gem_vram_object *gbo;
+	struct ast_private *ast;
+	int ret;
+	void *src, *dst;
+
+	if (!crtc || !fb)
+		return 0;
+
+	if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
+		return -EINVAL;
+
+	ast = crtc->dev->dev_private;
+
+	gbo = drm_gem_vram_of_gem(fb->obj[0]);
+	src = drm_gem_vram_vmap(gbo);
+	if (IS_ERR(src)) {
+		ret = PTR_ERR(src);
+		goto err_drm_gem_vram_unpin;
+	}
+
+	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
+	if (IS_ERR(dst)) {
+		ret = PTR_ERR(dst);
+		goto err_drm_gem_vram_vunmap_src;
+	}
+
+	ret = ast_cursor_update(dst, src, fb->width, fb->height);
+	if (ret)
+		goto err_drm_gem_vram_vunmap_dst;
+
+	/* Always unmap buffers here. Destination buffers are
+	 * perma-pinned while the driver is active. We're only
+	 * changing ref-counters here.
+	 */
+	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
+	drm_gem_vram_vunmap(gbo, src);
+
+	return 0;
+
+err_drm_gem_vram_vunmap_dst:
+	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
+err_drm_gem_vram_vunmap_src:
+	drm_gem_vram_vunmap(gbo, src);
+err_drm_gem_vram_unpin:
+	drm_gem_vram_unpin(gbo);
+	return ret;
+}
+
+static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
+						struct drm_plane_state *state)
+{
+	return 0;
+}
+
+static void
+ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
+				      struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = plane->state;
+	struct drm_crtc *crtc = state->crtc;
+	struct drm_framebuffer *fb = state->fb;
+	struct ast_private *ast = plane->dev->dev_private;
+	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+	struct drm_gem_vram_object *gbo;
+	s64 off;
+	u8 jreg;
+
+	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
+	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
+
+	if (state->fb != old_state->fb) {
+		/* A new cursor image was installed. */
+		gbo = ast->cursor.gbo[ast->cursor.next_index];
+		off = drm_gem_vram_offset(gbo);
+		if (WARN_ON_ONCE(off < 0))
+			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
+		ast_cursor_set_base(ast, off);
+
+		++ast->cursor.next_index;
+		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
+	}
+
+	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
+
+	jreg = 0x2;
+	/* enable ARGB cursor */
+	jreg |= 1;
+	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
+}
+
+static void
+ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
+				       struct drm_plane_state *old_state)
+{
+	struct ast_private *ast = plane->dev->dev_private;
+
+	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
+}
+
+static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
+	.prepare_fb = ast_cursor_plane_helper_prepare_fb,
+	.cleanup_fb = NULL, /* not required for cursor plane */
+	.atomic_check = ast_cursor_plane_helper_atomic_check,
+	.atomic_update = ast_cursor_plane_helper_atomic_update,
+	.atomic_disable = ast_cursor_plane_helper_atomic_disable,
+};
+
+static const struct drm_plane_funcs ast_cursor_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	.reset = drm_atomic_helper_plane_reset,
+	.set_property = NULL,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.atomic_set_property = NULL,
+	.atomic_get_property = NULL,
+	.late_register = NULL,
+	.early_unregister = NULL,
+	.atomic_print_state = NULL,
+	.format_mod_supported = NULL,
+};
+
 /*
  * CRTC
  */
@@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
 		return -ENOMEM;
 
 	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
-					NULL, &ast_crtc_funcs, NULL);
+					&ast->cursor_plane, &ast_crtc_funcs,
+					NULL);
 	if (ret)
 		goto err_kfree;
 
@@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
 		DRM_FORMAT_RGB565,
 		DRM_FORMAT_C8,
 	};
+	static const uint32_t cursor_plane_formats[] = {
+		DRM_FORMAT_ARGB8888,
+	};
 
 	struct ast_private *ast = dev->dev_private;
 	int ret;
@@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
 	drm_plane_helper_add(&ast->primary_plane,
 			     &ast_primary_plane_helper_funcs);
 
+	ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
+				       &ast_cursor_plane_funcs,
+				       cursor_plane_formats,
+				       ARRAY_SIZE(cursor_plane_formats),
+				       NULL, DRM_PLANE_TYPE_CURSOR, NULL);
+	if (ret) {
+		DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
+		return ret;
+	}
+	drm_plane_helper_add(&ast->cursor_plane,
+			     &ast_cursor_plane_helper_funcs);
+
 	ast_cursor_init(dev);
 	ast_crtc_init(dev);
 	ast_encoder_init(dev);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] drm/ast: Add cursor plane

[Gerd Hoffmann]

On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
> The cursor plane uses an internal format of ARGB4444. To userspace, we
> announce ARGB8888 and do the transformation internally.

Same comments as on patch 6/9 (primary plane ...)

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] drm/ast: Add cursor plane

[Daniel Vetter]

On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
> The cursor plane uses an internal format of ARGB4444. To userspace, we
> announce ARGB8888 and do the transformation internally.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
will actually use it :-/
-Daniel

> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
>  2 files changed, 161 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 13560622f22a..49557a73390f 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -122,6 +122,7 @@ struct ast_private {
>  	} cursor;
>  
>  	struct drm_plane primary_plane;
> +	struct drm_plane cursor_plane;
>  
>  	bool support_wide_screen;
>  	enum {
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 7667f4502eb9..f5f73200e8e4 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>  			   int x, int y);
>  
>  
> +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);
> +
>  static inline void ast_load_palette_index(struct ast_private *ast,
>  				     u8 index, u8 red, u8 green,
>  				     u8 blue)
> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
>  	.format_mod_supported = NULL,
>  };
>  
> +/*
> + * Cursor plane
> + */
> +
> +static int
> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> +				   struct drm_plane_state *new_state)
> +{
> +	struct drm_framebuffer *fb = new_state->fb;
> +	struct drm_crtc *crtc = new_state->crtc;
> +	struct drm_gem_vram_object *gbo;
> +	struct ast_private *ast;
> +	int ret;
> +	void *src, *dst;
> +
> +	if (!crtc || !fb)
> +		return 0;
> +
> +	if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
> +		return -EINVAL;
> +
> +	ast = crtc->dev->dev_private;
> +
> +	gbo = drm_gem_vram_of_gem(fb->obj[0]);
> +	src = drm_gem_vram_vmap(gbo);
> +	if (IS_ERR(src)) {
> +		ret = PTR_ERR(src);
> +		goto err_drm_gem_vram_unpin;
> +	}
> +
> +	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
> +	if (IS_ERR(dst)) {
> +		ret = PTR_ERR(dst);
> +		goto err_drm_gem_vram_vunmap_src;
> +	}
> +
> +	ret = ast_cursor_update(dst, src, fb->width, fb->height);
> +	if (ret)
> +		goto err_drm_gem_vram_vunmap_dst;
> +
> +	/* Always unmap buffers here. Destination buffers are
> +	 * perma-pinned while the driver is active. We're only
> +	 * changing ref-counters here.
> +	 */
> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> +	drm_gem_vram_vunmap(gbo, src);
> +
> +	return 0;
> +
> +err_drm_gem_vram_vunmap_dst:
> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> +err_drm_gem_vram_vunmap_src:
> +	drm_gem_vram_vunmap(gbo, src);
> +err_drm_gem_vram_unpin:
> +	drm_gem_vram_unpin(gbo);
> +	return ret;
> +}
> +
> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
> +						struct drm_plane_state *state)
> +{
> +	return 0;
> +}
> +
> +static void
> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> +				      struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = plane->state;
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_framebuffer *fb = state->fb;
> +	struct ast_private *ast = plane->dev->dev_private;
> +	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> +	struct drm_gem_vram_object *gbo;
> +	s64 off;
> +	u8 jreg;
> +
> +	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
> +	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
> +
> +	if (state->fb != old_state->fb) {
> +		/* A new cursor image was installed. */
> +		gbo = ast->cursor.gbo[ast->cursor.next_index];
> +		off = drm_gem_vram_offset(gbo);
> +		if (WARN_ON_ONCE(off < 0))
> +			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
> +		ast_cursor_set_base(ast, off);
> +
> +		++ast->cursor.next_index;
> +		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
> +	}
> +
> +	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
> +
> +	jreg = 0x2;
> +	/* enable ARGB cursor */
> +	jreg |= 1;
> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
> +}
> +
> +static void
> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> +				       struct drm_plane_state *old_state)
> +{
> +	struct ast_private *ast = plane->dev->dev_private;
> +
> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> +}
> +
> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
> +	.prepare_fb = ast_cursor_plane_helper_prepare_fb,
> +	.cleanup_fb = NULL, /* not required for cursor plane */
> +	.atomic_check = ast_cursor_plane_helper_atomic_check,
> +	.atomic_update = ast_cursor_plane_helper_atomic_update,
> +	.atomic_disable = ast_cursor_plane_helper_atomic_disable,
> +};
> +
> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = drm_plane_cleanup,
> +	.reset = drm_atomic_helper_plane_reset,
> +	.set_property = NULL,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +	.atomic_set_property = NULL,
> +	.atomic_get_property = NULL,
> +	.late_register = NULL,
> +	.early_unregister = NULL,
> +	.atomic_print_state = NULL,
> +	.format_mod_supported = NULL,
> +};
> +
>  /*
>   * CRTC
>   */
> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
>  		return -ENOMEM;
>  
>  	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
> -					NULL, &ast_crtc_funcs, NULL);
> +					&ast->cursor_plane, &ast_crtc_funcs,
> +					NULL);
>  	if (ret)
>  		goto err_kfree;
>  
> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
>  		DRM_FORMAT_RGB565,
>  		DRM_FORMAT_C8,
>  	};
> +	static const uint32_t cursor_plane_formats[] = {
> +		DRM_FORMAT_ARGB8888,
> +	};
>  
>  	struct ast_private *ast = dev->dev_private;
>  	int ret;
> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
>  	drm_plane_helper_add(&ast->primary_plane,
>  			     &ast_primary_plane_helper_funcs);
>  
> +	ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
> +				       &ast_cursor_plane_funcs,
> +				       cursor_plane_formats,
> +				       ARRAY_SIZE(cursor_plane_formats),
> +				       NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> +	if (ret) {
> +		DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
> +		return ret;
> +	}
> +	drm_plane_helper_add(&ast->cursor_plane,
> +			     &ast_cursor_plane_helper_funcs);
> +
>  	ast_cursor_init(dev);
>  	ast_crtc_init(dev);
>  	ast_encoder_init(dev);
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] drm/ast: Add cursor plane

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 8250 bytes --]

Hi

Am 05.11.19 um 10:55 schrieb Daniel Vetter:
> On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
>> The cursor plane uses an internal format of ARGB4444. To userspace, we
>> announce ARGB8888 and do the transformation internally.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
> will actually use it :-/

Is that a serious proposal? I thought about ARGB4444 and quickly
dismissed it because no one will ever support it anyway.

Best regards
Thomas

> -Daniel
> 
>> ---
>>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index 13560622f22a..49557a73390f 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -122,6 +122,7 @@ struct ast_private {
>>  	} cursor;
>>  
>>  	struct drm_plane primary_plane;
>> +	struct drm_plane cursor_plane;
>>  
>>  	bool support_wide_screen;
>>  	enum {
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 7667f4502eb9..f5f73200e8e4 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>>  			   int x, int y);
>>  
>>  
>> +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);
>> +
>>  static inline void ast_load_palette_index(struct ast_private *ast,
>>  				     u8 index, u8 red, u8 green,
>>  				     u8 blue)
>> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
>>  	.format_mod_supported = NULL,
>>  };
>>  
>> +/*
>> + * Cursor plane
>> + */
>> +
>> +static int
>> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
>> +				   struct drm_plane_state *new_state)
>> +{
>> +	struct drm_framebuffer *fb = new_state->fb;
>> +	struct drm_crtc *crtc = new_state->crtc;
>> +	struct drm_gem_vram_object *gbo;
>> +	struct ast_private *ast;
>> +	int ret;
>> +	void *src, *dst;
>> +
>> +	if (!crtc || !fb)
>> +		return 0;
>> +
>> +	if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
>> +		return -EINVAL;
>> +
>> +	ast = crtc->dev->dev_private;
>> +
>> +	gbo = drm_gem_vram_of_gem(fb->obj[0]);
>> +	src = drm_gem_vram_vmap(gbo);
>> +	if (IS_ERR(src)) {
>> +		ret = PTR_ERR(src);
>> +		goto err_drm_gem_vram_unpin;
>> +	}
>> +
>> +	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
>> +	if (IS_ERR(dst)) {
>> +		ret = PTR_ERR(dst);
>> +		goto err_drm_gem_vram_vunmap_src;
>> +	}
>> +
>> +	ret = ast_cursor_update(dst, src, fb->width, fb->height);
>> +	if (ret)
>> +		goto err_drm_gem_vram_vunmap_dst;
>> +
>> +	/* Always unmap buffers here. Destination buffers are
>> +	 * perma-pinned while the driver is active. We're only
>> +	 * changing ref-counters here.
>> +	 */
>> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>> +	drm_gem_vram_vunmap(gbo, src);
>> +
>> +	return 0;
>> +
>> +err_drm_gem_vram_vunmap_dst:
>> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>> +err_drm_gem_vram_vunmap_src:
>> +	drm_gem_vram_vunmap(gbo, src);
>> +err_drm_gem_vram_unpin:
>> +	drm_gem_vram_unpin(gbo);
>> +	return ret;
>> +}
>> +
>> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
>> +						struct drm_plane_state *state)
>> +{
>> +	return 0;
>> +}
>> +
>> +static void
>> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>> +				      struct drm_plane_state *old_state)
>> +{
>> +	struct drm_plane_state *state = plane->state;
>> +	struct drm_crtc *crtc = state->crtc;
>> +	struct drm_framebuffer *fb = state->fb;
>> +	struct ast_private *ast = plane->dev->dev_private;
>> +	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>> +	struct drm_gem_vram_object *gbo;
>> +	s64 off;
>> +	u8 jreg;
>> +
>> +	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
>> +	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
>> +
>> +	if (state->fb != old_state->fb) {
>> +		/* A new cursor image was installed. */
>> +		gbo = ast->cursor.gbo[ast->cursor.next_index];
>> +		off = drm_gem_vram_offset(gbo);
>> +		if (WARN_ON_ONCE(off < 0))
>> +			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
>> +		ast_cursor_set_base(ast, off);
>> +
>> +		++ast->cursor.next_index;
>> +		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
>> +	}
>> +
>> +	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
>> +
>> +	jreg = 0x2;
>> +	/* enable ARGB cursor */
>> +	jreg |= 1;
>> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
>> +}
>> +
>> +static void
>> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
>> +				       struct drm_plane_state *old_state)
>> +{
>> +	struct ast_private *ast = plane->dev->dev_private;
>> +
>> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
>> +}
>> +
>> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
>> +	.prepare_fb = ast_cursor_plane_helper_prepare_fb,
>> +	.cleanup_fb = NULL, /* not required for cursor plane */
>> +	.atomic_check = ast_cursor_plane_helper_atomic_check,
>> +	.atomic_update = ast_cursor_plane_helper_atomic_update,
>> +	.atomic_disable = ast_cursor_plane_helper_atomic_disable,
>> +};
>> +
>> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>> +	.update_plane = drm_atomic_helper_update_plane,
>> +	.disable_plane = drm_atomic_helper_disable_plane,
>> +	.destroy = drm_plane_cleanup,
>> +	.reset = drm_atomic_helper_plane_reset,
>> +	.set_property = NULL,
>> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>> +	.atomic_set_property = NULL,
>> +	.atomic_get_property = NULL,
>> +	.late_register = NULL,
>> +	.early_unregister = NULL,
>> +	.atomic_print_state = NULL,
>> +	.format_mod_supported = NULL,
>> +};
>> +
>>  /*
>>   * CRTC
>>   */
>> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
>>  		return -ENOMEM;
>>  
>>  	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
>> -					NULL, &ast_crtc_funcs, NULL);
>> +					&ast->cursor_plane, &ast_crtc_funcs,
>> +					NULL);
>>  	if (ret)
>>  		goto err_kfree;
>>  
>> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
>>  		DRM_FORMAT_RGB565,
>>  		DRM_FORMAT_C8,
>>  	};
>> +	static const uint32_t cursor_plane_formats[] = {
>> +		DRM_FORMAT_ARGB8888,
>> +	};
>>  
>>  	struct ast_private *ast = dev->dev_private;
>>  	int ret;
>> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
>>  	drm_plane_helper_add(&ast->primary_plane,
>>  			     &ast_primary_plane_helper_funcs);
>>  
>> +	ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
>> +				       &ast_cursor_plane_funcs,
>> +				       cursor_plane_formats,
>> +				       ARRAY_SIZE(cursor_plane_formats),
>> +				       NULL, DRM_PLANE_TYPE_CURSOR, NULL);
>> +	if (ret) {
>> +		DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
>> +		return ret;
>> +	}
>> +	drm_plane_helper_add(&ast->cursor_plane,
>> +			     &ast_cursor_plane_helper_funcs);
>> +
>>  	ast_cursor_init(dev);
>>  	ast_crtc_init(dev);
>>  	ast_encoder_init(dev);
>> -- 
>> 2.23.0
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] drm/ast: Add cursor plane

[Daniel Vetter]

On Wed, Nov 6, 2019 at 9:31 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 05.11.19 um 10:55 schrieb Daniel Vetter:
> > On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
> >> The cursor plane uses an internal format of ARGB4444. To userspace, we
> >> announce ARGB8888 and do the transformation internally.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >
> > Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
> > will actually use it :-/
>
> Is that a serious proposal? I thought about ARGB4444 and quickly
> dismissed it because no one will ever support it anyway.

For cursor maybe not, but in other cases where we added the
RGB8888->native format hack because userspace, we did make sure that
the driver exposes the native formats too. Up to you really.
-Daniel

>
> Best regards
> Thomas
>
> > -Daniel
> >
> >> ---
> >>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
> >>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
> >>  2 files changed, 161 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> >> index 13560622f22a..49557a73390f 100644
> >> --- a/drivers/gpu/drm/ast/ast_drv.h
> >> +++ b/drivers/gpu/drm/ast/ast_drv.h
> >> @@ -122,6 +122,7 @@ struct ast_private {
> >>      } cursor;
> >>
> >>      struct drm_plane primary_plane;
> >> +    struct drm_plane cursor_plane;
> >>
> >>      bool support_wide_screen;
> >>      enum {
> >> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> >> index 7667f4502eb9..f5f73200e8e4 100644
> >> --- a/drivers/gpu/drm/ast/ast_mode.c
> >> +++ b/drivers/gpu/drm/ast/ast_mode.c
> >> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
> >>                         int x, int y);
> >>
> >>
> >> +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);
> >> +
> >>  static inline void ast_load_palette_index(struct ast_private *ast,
> >>                                   u8 index, u8 red, u8 green,
> >>                                   u8 blue)
> >> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
> >>      .format_mod_supported = NULL,
> >>  };
> >>
> >> +/*
> >> + * Cursor plane
> >> + */
> >> +
> >> +static int
> >> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> >> +                               struct drm_plane_state *new_state)
> >> +{
> >> +    struct drm_framebuffer *fb = new_state->fb;
> >> +    struct drm_crtc *crtc = new_state->crtc;
> >> +    struct drm_gem_vram_object *gbo;
> >> +    struct ast_private *ast;
> >> +    int ret;
> >> +    void *src, *dst;
> >> +
> >> +    if (!crtc || !fb)
> >> +            return 0;
> >> +
> >> +    if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
> >> +            return -EINVAL;
> >> +
> >> +    ast = crtc->dev->dev_private;
> >> +
> >> +    gbo = drm_gem_vram_of_gem(fb->obj[0]);
> >> +    src = drm_gem_vram_vmap(gbo);
> >> +    if (IS_ERR(src)) {
> >> +            ret = PTR_ERR(src);
> >> +            goto err_drm_gem_vram_unpin;
> >> +    }
> >> +
> >> +    dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
> >> +    if (IS_ERR(dst)) {
> >> +            ret = PTR_ERR(dst);
> >> +            goto err_drm_gem_vram_vunmap_src;
> >> +    }
> >> +
> >> +    ret = ast_cursor_update(dst, src, fb->width, fb->height);
> >> +    if (ret)
> >> +            goto err_drm_gem_vram_vunmap_dst;
> >> +
> >> +    /* Always unmap buffers here. Destination buffers are
> >> +     * perma-pinned while the driver is active. We're only
> >> +     * changing ref-counters here.
> >> +     */
> >> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> >> +    drm_gem_vram_vunmap(gbo, src);
> >> +
> >> +    return 0;
> >> +
> >> +err_drm_gem_vram_vunmap_dst:
> >> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> >> +err_drm_gem_vram_vunmap_src:
> >> +    drm_gem_vram_vunmap(gbo, src);
> >> +err_drm_gem_vram_unpin:
> >> +    drm_gem_vram_unpin(gbo);
> >> +    return ret;
> >> +}
> >> +
> >> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
> >> +                                            struct drm_plane_state *state)
> >> +{
> >> +    return 0;
> >> +}
> >> +
> >> +static void
> >> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> >> +                                  struct drm_plane_state *old_state)
> >> +{
> >> +    struct drm_plane_state *state = plane->state;
> >> +    struct drm_crtc *crtc = state->crtc;
> >> +    struct drm_framebuffer *fb = state->fb;
> >> +    struct ast_private *ast = plane->dev->dev_private;
> >> +    struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> >> +    struct drm_gem_vram_object *gbo;
> >> +    s64 off;
> >> +    u8 jreg;
> >> +
> >> +    ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
> >> +    ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
> >> +
> >> +    if (state->fb != old_state->fb) {
> >> +            /* A new cursor image was installed. */
> >> +            gbo = ast->cursor.gbo[ast->cursor.next_index];
> >> +            off = drm_gem_vram_offset(gbo);
> >> +            if (WARN_ON_ONCE(off < 0))
> >> +                    return; /* Bug: we didn't pin cursor HW BO to VRAM. */
> >> +            ast_cursor_set_base(ast, off);
> >> +
> >> +            ++ast->cursor.next_index;
> >> +            ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
> >> +    }
> >> +
> >> +    ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
> >> +
> >> +    jreg = 0x2;
> >> +    /* enable ARGB cursor */
> >> +    jreg |= 1;
> >> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
> >> +}
> >> +
> >> +static void
> >> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> >> +                                   struct drm_plane_state *old_state)
> >> +{
> >> +    struct ast_private *ast = plane->dev->dev_private;
> >> +
> >> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> >> +}
> >> +
> >> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
> >> +    .prepare_fb = ast_cursor_plane_helper_prepare_fb,
> >> +    .cleanup_fb = NULL, /* not required for cursor plane */
> >> +    .atomic_check = ast_cursor_plane_helper_atomic_check,
> >> +    .atomic_update = ast_cursor_plane_helper_atomic_update,
> >> +    .atomic_disable = ast_cursor_plane_helper_atomic_disable,
> >> +};
> >> +
> >> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
> >> +    .update_plane = drm_atomic_helper_update_plane,
> >> +    .disable_plane = drm_atomic_helper_disable_plane,
> >> +    .destroy = drm_plane_cleanup,
> >> +    .reset = drm_atomic_helper_plane_reset,
> >> +    .set_property = NULL,
> >> +    .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> >> +    .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> >> +    .atomic_set_property = NULL,
> >> +    .atomic_get_property = NULL,
> >> +    .late_register = NULL,
> >> +    .early_unregister = NULL,
> >> +    .atomic_print_state = NULL,
> >> +    .format_mod_supported = NULL,
> >> +};
> >> +
> >>  /*
> >>   * CRTC
> >>   */
> >> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
> >>              return -ENOMEM;
> >>
> >>      ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
> >> -                                    NULL, &ast_crtc_funcs, NULL);
> >> +                                    &ast->cursor_plane, &ast_crtc_funcs,
> >> +                                    NULL);
> >>      if (ret)
> >>              goto err_kfree;
> >>
> >> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
> >>              DRM_FORMAT_RGB565,
> >>              DRM_FORMAT_C8,
> >>      };
> >> +    static const uint32_t cursor_plane_formats[] = {
> >> +            DRM_FORMAT_ARGB8888,
> >> +    };
> >>
> >>      struct ast_private *ast = dev->dev_private;
> >>      int ret;
> >> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
> >>      drm_plane_helper_add(&ast->primary_plane,
> >>                           &ast_primary_plane_helper_funcs);
> >>
> >> +    ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
> >> +                                   &ast_cursor_plane_funcs,
> >> +                                   cursor_plane_formats,
> >> +                                   ARRAY_SIZE(cursor_plane_formats),
> >> +                                   NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> >> +    if (ret) {
> >> +            DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
> >> +            return ret;
> >> +    }
> >> +    drm_plane_helper_add(&ast->cursor_plane,
> >> +                         &ast_cursor_plane_helper_funcs);
> >> +
> >>      ast_cursor_init(dev);
> >>      ast_crtc_init(dev);
> >>      ast_encoder_init(dev);
> >> --
> >> 2.23.0
> >>
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] drm/ast: Add cursor plane

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 10378 bytes --]

Hi

Am 06.11.19 um 10:05 schrieb Daniel Vetter:
> On Wed, Nov 6, 2019 at 9:31 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 05.11.19 um 10:55 schrieb Daniel Vetter:
>>> On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
>>>> The cursor plane uses an internal format of ARGB4444. To userspace, we
>>>> announce ARGB8888 and do the transformation internally.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>> Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
>>> will actually use it :-/
>>
>> Is that a serious proposal? I thought about ARGB4444 and quickly
>> dismissed it because no one will ever support it anyway.
> 
> For cursor maybe not, but in other cases where we added the
> RGB8888->native format hack because userspace, we did make sure that
> the driver exposes the native formats too. Up to you really.

I see. In that case, I'd like to continue with the patchset as-is, but
provide ARGB4444 in a later patch.

Best regards
Thomas

> -Daniel
> 
>>
>> Best regards
>> Thomas
>>
>>> -Daniel
>>>
>>>> ---
>>>>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>>>>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
>>>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>>>> index 13560622f22a..49557a73390f 100644
>>>> --- a/drivers/gpu/drm/ast/ast_drv.h
>>>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>>>> @@ -122,6 +122,7 @@ struct ast_private {
>>>>      } cursor;
>>>>
>>>>      struct drm_plane primary_plane;
>>>> +    struct drm_plane cursor_plane;
>>>>
>>>>      bool support_wide_screen;
>>>>      enum {
>>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>>>> index 7667f4502eb9..f5f73200e8e4 100644
>>>> --- a/drivers/gpu/drm/ast/ast_mode.c
>>>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>>>> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>>>>                         int x, int y);
>>>>
>>>>
>>>> +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);
>>>> +
>>>>  static inline void ast_load_palette_index(struct ast_private *ast,
>>>>                                   u8 index, u8 red, u8 green,
>>>>                                   u8 blue)
>>>> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
>>>>      .format_mod_supported = NULL,
>>>>  };
>>>>
>>>> +/*
>>>> + * Cursor plane
>>>> + */
>>>> +
>>>> +static int
>>>> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
>>>> +                               struct drm_plane_state *new_state)
>>>> +{
>>>> +    struct drm_framebuffer *fb = new_state->fb;
>>>> +    struct drm_crtc *crtc = new_state->crtc;
>>>> +    struct drm_gem_vram_object *gbo;
>>>> +    struct ast_private *ast;
>>>> +    int ret;
>>>> +    void *src, *dst;
>>>> +
>>>> +    if (!crtc || !fb)
>>>> +            return 0;
>>>> +
>>>> +    if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
>>>> +            return -EINVAL;
>>>> +
>>>> +    ast = crtc->dev->dev_private;
>>>> +
>>>> +    gbo = drm_gem_vram_of_gem(fb->obj[0]);
>>>> +    src = drm_gem_vram_vmap(gbo);
>>>> +    if (IS_ERR(src)) {
>>>> +            ret = PTR_ERR(src);
>>>> +            goto err_drm_gem_vram_unpin;
>>>> +    }
>>>> +
>>>> +    dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
>>>> +    if (IS_ERR(dst)) {
>>>> +            ret = PTR_ERR(dst);
>>>> +            goto err_drm_gem_vram_vunmap_src;
>>>> +    }
>>>> +
>>>> +    ret = ast_cursor_update(dst, src, fb->width, fb->height);
>>>> +    if (ret)
>>>> +            goto err_drm_gem_vram_vunmap_dst;
>>>> +
>>>> +    /* Always unmap buffers here. Destination buffers are
>>>> +     * perma-pinned while the driver is active. We're only
>>>> +     * changing ref-counters here.
>>>> +     */
>>>> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>>>> +    drm_gem_vram_vunmap(gbo, src);
>>>> +
>>>> +    return 0;
>>>> +
>>>> +err_drm_gem_vram_vunmap_dst:
>>>> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>>>> +err_drm_gem_vram_vunmap_src:
>>>> +    drm_gem_vram_vunmap(gbo, src);
>>>> +err_drm_gem_vram_unpin:
>>>> +    drm_gem_vram_unpin(gbo);
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
>>>> +                                            struct drm_plane_state *state)
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void
>>>> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>>>> +                                  struct drm_plane_state *old_state)
>>>> +{
>>>> +    struct drm_plane_state *state = plane->state;
>>>> +    struct drm_crtc *crtc = state->crtc;
>>>> +    struct drm_framebuffer *fb = state->fb;
>>>> +    struct ast_private *ast = plane->dev->dev_private;
>>>> +    struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>>>> +    struct drm_gem_vram_object *gbo;
>>>> +    s64 off;
>>>> +    u8 jreg;
>>>> +
>>>> +    ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
>>>> +    ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
>>>> +
>>>> +    if (state->fb != old_state->fb) {
>>>> +            /* A new cursor image was installed. */
>>>> +            gbo = ast->cursor.gbo[ast->cursor.next_index];
>>>> +            off = drm_gem_vram_offset(gbo);
>>>> +            if (WARN_ON_ONCE(off < 0))
>>>> +                    return; /* Bug: we didn't pin cursor HW BO to VRAM. */
>>>> +            ast_cursor_set_base(ast, off);
>>>> +
>>>> +            ++ast->cursor.next_index;
>>>> +            ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
>>>> +    }
>>>> +
>>>> +    ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
>>>> +
>>>> +    jreg = 0x2;
>>>> +    /* enable ARGB cursor */
>>>> +    jreg |= 1;
>>>> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
>>>> +}
>>>> +
>>>> +static void
>>>> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
>>>> +                                   struct drm_plane_state *old_state)
>>>> +{
>>>> +    struct ast_private *ast = plane->dev->dev_private;
>>>> +
>>>> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
>>>> +}
>>>> +
>>>> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
>>>> +    .prepare_fb = ast_cursor_plane_helper_prepare_fb,
>>>> +    .cleanup_fb = NULL, /* not required for cursor plane */
>>>> +    .atomic_check = ast_cursor_plane_helper_atomic_check,
>>>> +    .atomic_update = ast_cursor_plane_helper_atomic_update,
>>>> +    .atomic_disable = ast_cursor_plane_helper_atomic_disable,
>>>> +};
>>>> +
>>>> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>>>> +    .update_plane = drm_atomic_helper_update_plane,
>>>> +    .disable_plane = drm_atomic_helper_disable_plane,
>>>> +    .destroy = drm_plane_cleanup,
>>>> +    .reset = drm_atomic_helper_plane_reset,
>>>> +    .set_property = NULL,
>>>> +    .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>>>> +    .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>>>> +    .atomic_set_property = NULL,
>>>> +    .atomic_get_property = NULL,
>>>> +    .late_register = NULL,
>>>> +    .early_unregister = NULL,
>>>> +    .atomic_print_state = NULL,
>>>> +    .format_mod_supported = NULL,
>>>> +};
>>>> +
>>>>  /*
>>>>   * CRTC
>>>>   */
>>>> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
>>>>              return -ENOMEM;
>>>>
>>>>      ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
>>>> -                                    NULL, &ast_crtc_funcs, NULL);
>>>> +                                    &ast->cursor_plane, &ast_crtc_funcs,
>>>> +                                    NULL);
>>>>      if (ret)
>>>>              goto err_kfree;
>>>>
>>>> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
>>>>              DRM_FORMAT_RGB565,
>>>>              DRM_FORMAT_C8,
>>>>      };
>>>> +    static const uint32_t cursor_plane_formats[] = {
>>>> +            DRM_FORMAT_ARGB8888,
>>>> +    };
>>>>
>>>>      struct ast_private *ast = dev->dev_private;
>>>>      int ret;
>>>> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
>>>>      drm_plane_helper_add(&ast->primary_plane,
>>>>                           &ast_primary_plane_helper_funcs);
>>>>
>>>> +    ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
>>>> +                                   &ast_cursor_plane_funcs,
>>>> +                                   cursor_plane_formats,
>>>> +                                   ARRAY_SIZE(cursor_plane_formats),
>>>> +                                   NULL, DRM_PLANE_TYPE_CURSOR, NULL);
>>>> +    if (ret) {
>>>> +            DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
>>>> +            return ret;
>>>> +    }
>>>> +    drm_plane_helper_add(&ast->cursor_plane,
>>>> +                         &ast_cursor_plane_helper_funcs);
>>>> +
>>>>      ast_cursor_init(dev);
>>>>      ast_crtc_init(dev);
>>>>      ast_encoder_init(dev);
>>>> --
>>>> 2.23.0
>>>>
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thomas Zimmermann]

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,
-};
-
 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)
 {
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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?

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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?).
-Daniel

> 
> cheers,
>   Gerd
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 13337 bytes --]

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
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 13193 bytes --]

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
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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).

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

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1359 bytes --]

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
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/9] drm/ast: Convert to atomic modesetting

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2169 bytes --]

(cc: yc_chen@aspeedtech.com)

Am 28.10.19 um 16:49 schrieb Thomas Zimmermann:
> This patch set adds universal planes to ast and converts the driver to
> atomic modesetting.
> 
> The first patch is purely for clean-up.
> 
> Patches 2 to 5 prepare the ast modesetting code for universal planes and
> atomic modesetting. The size calculation for each mode has to take double
> buffering into account. Several functions have to be split to make them
> work with the separate check and update on CRTCs and planes. There are no
> functional changes.
> 
> Patches 6 to 8 add atomic modesetting code for planes and CRTC. Planes
> immediately provide atomic functions. There's no intermediate step with
> legacy functions for non-atomic drivers. The cursor plane HW only
> supports ARGB4444, so the cursor plane converts the format internally;
> just as the legacy implementation did.
> 
> Finally, patch 9 adds missing helpers and enables atomic modesetting. The
> CRTC functions now provide page_flip, which enables Weston support on
> ast hardware.
> 
> The patchset has been tested by running the fbdev console, X11 (Gnome)
> and Weston on an AST2100 chipset.
> 
> Thomas Zimmermann (9):
>   drm/ast: Remove last traces of struct ast_gem_object
>   drm/ast: Check video-mode requirements against VRAM size
>   drm/ast: Don't clear base address and offset with default values
>   drm/ast: Split ast_set_ext_reg() into color and threshold function
>   drm/ast: Split ast_set_vbios_mode_info()
>   drm/ast: Add primary plane
>   drm/ast: Add CRTC helpers for atomic modesetting
>   drm/ast: Add cursor plane
>   drm/ast: Enable atomic modesetting
> 
>  drivers/gpu/drm/ast/ast_drv.c  |  24 +-
>  drivers/gpu/drm/ast/ast_drv.h  |   9 +-
>  drivers/gpu/drm/ast/ast_main.c |  54 +--
>  drivers/gpu/drm/ast/ast_mode.c | 698 ++++++++++++++++++++-------------
>  4 files changed, 462 insertions(+), 323 deletions(-)
> 
> --
> 2.23.0
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/9] drm/ast: Convert to atomic modesetting

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2169 bytes --]

(cc: yc_chen@aspeedtech.com)

Am 28.10.19 um 16:49 schrieb Thomas Zimmermann:
> This patch set adds universal planes to ast and converts the driver to
> atomic modesetting.
> 
> The first patch is purely for clean-up.
> 
> Patches 2 to 5 prepare the ast modesetting code for universal planes and
> atomic modesetting. The size calculation for each mode has to take double
> buffering into account. Several functions have to be split to make them
> work with the separate check and update on CRTCs and planes. There are no
> functional changes.
> 
> Patches 6 to 8 add atomic modesetting code for planes and CRTC. Planes
> immediately provide atomic functions. There's no intermediate step with
> legacy functions for non-atomic drivers. The cursor plane HW only
> supports ARGB4444, so the cursor plane converts the format internally;
> just as the legacy implementation did.
> 
> Finally, patch 9 adds missing helpers and enables atomic modesetting. The
> CRTC functions now provide page_flip, which enables Weston support on
> ast hardware.
> 
> The patchset has been tested by running the fbdev console, X11 (Gnome)
> and Weston on an AST2100 chipset.
> 
> Thomas Zimmermann (9):
>   drm/ast: Remove last traces of struct ast_gem_object
>   drm/ast: Check video-mode requirements against VRAM size
>   drm/ast: Don't clear base address and offset with default values
>   drm/ast: Split ast_set_ext_reg() into color and threshold function
>   drm/ast: Split ast_set_vbios_mode_info()
>   drm/ast: Add primary plane
>   drm/ast: Add CRTC helpers for atomic modesetting
>   drm/ast: Add cursor plane
>   drm/ast: Enable atomic modesetting
> 
>  drivers/gpu/drm/ast/ast_drv.c  |  24 +-
>  drivers/gpu/drm/ast/ast_drv.h  |   9 +-
>  drivers/gpu/drm/ast/ast_main.c |  54 +--
>  drivers/gpu/drm/ast/ast_mode.c | 698 ++++++++++++++++++++-------------
>  4 files changed, 462 insertions(+), 323 deletions(-)
> 
> --
> 2.23.0
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel