[PATCHv7 0/6] Add AFBC support for Rockchip

[PATCHv7 0/6] Add AFBC support for Rockchip

[Andrzej Pietrasiewicz]

This series adds AFBC support for Rockchip. It is inspired by:

https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/factory-gru-9017.B-chromeos-4.4/drivers/gpu/drm/rockchip/rockchip_drm_vop.c

This is the seventh iteration of the afbc series, which addresses
comments received for v6.

Compared to v5 it simplifies the way afbc-related helpers are
exposed to their users.

A new struct drm_afbc_framebuffer is added, which stores afbc-related
driver-specific data. Interested drivers need to explicitly allocate
an instance of struct drm_afbc_framebuffer, use drm_gem_fb_init_with_funcs()
call drm_gem_fb_afbc_init() and do their specific afbc-related checks.

There are 3 interested drivers: malidp, komeda and rockchip and I only have
rockchip hardware. As Liviu reports, due to the coronavirus outbreak,
it is difficult to test komeda now, so, according to his suggestion,
I purposedly omit changes to komeda. Malidp is changed accordingly, though,
which is a proof that it can be done. Then adding afbc support for rockchip
follows the malidp example.

I kindly ask for reviewing the series. I need to mention that my ultimate
goal is merging afbc for rockchip and I don't have other hardware, so some
help from malidp developers/maintainers would be appreciated.

Rebased onto drm-misc-next.

v6..v7:
- used IS_ERR() instead of IS_ERR_OR_NULL() (Emil)
- made drm_gem_fb_afbc_init() symmetric in terms of not putting the objects
in case of error, it is now caller's responsibility (Emil)
- added an entry in Documentation/gpu/todo.rst about drivers not encoding
cpp in their format info (Emil)
- sticked to the 80 columns per line rule wherever possible, excluding
error messages so that they can be grepped (Emil)
- removed redundant WARN_ON() in rockchip_mod_supported() (Emil)
- made drm_gem_fb_init() and drm_gem_fb_init_with_funcs() return an int (James)
- factored in drm_afbc_get_superblock_wh() (James)
- eliminated unknown types error for u32/u64 (kbuild)

v5..v6:
- reworked the way afbc-specific helpers are exposed to drivers (Daniel)
- not checking block size mask in drm_is_afbc (James)
- fixed the test for afbc format (Boris)
- documented unused afbc format modifier values in drm_afbc_get_superblock_wh()
(Boris)
- changed drm_is_afbc to a macro
- renamed drm_gem_fb_lookup() to drm_gem_fb_objs_lookup() (James)
- eliminated storing block/tile alignment constraint in
 struct drm_afbc_framebuffer (James)
- eliminated storing afbc header alignment constraint
 in struct drm_afbc_framebuffer (James)
- eliminated storing afbc payload's offset in struct
drm_afbc_framebuffer (James)
- moved to taking bpp value from drm_format_info except malidp which doesn't
set it properly (James)
- removed unrelated coding style fixes in rockchip (Boris)
- consolidated 2-line error messages into one-liners in rockchip (Boris)
- added checking that there is at most one AFBC plane in
vop_crtc_atomic_check() (Boris)
- added checking that AFBC format is indeed supported in
rockchip_mod_supported() (Boris)
- removed requirement of exactly one color plane in rockchip_mod_supported()
- removed afbc_win hack in rockchip in favor of adding a field to crtc state
and ensuring only one AFBC plane in crtc's atomic check (Boris)

v4..v5:
- used proper way of subclassing drm_framebuffer (Daniel Vetter)
- added documentation to exported functions (Liviu Dudau)
- reordered new functions in drm_gem_framebuffer_helper.c to make a saner
diff (Liviu Dudau)
- used "2" suffix instead of "_special" for the special version of size
checks (Liviu Dudau)
- dropped unnecessarily added condition in drm_get_format_info() (Liviu
Dudau)
- dropped "block_size = 0;" trick in framebuffer_check() (Daniel Vetter)
- relaxed sticking to 80 characters per line rule in some cases

v3..v4:

- addressed (some) comments from Daniel Stone, Ezequiel Garcia, Daniel
Vetter and James Qian Wang - thank you for input
- refactored helpers to ease accommodating drivers with afbc needs
- moved afbc checks to helpers
- converted komeda, malidp and (the newly added) rockchip to use the afbc
helpers
- eliminated a separate, dedicated source code file

v2..v3:

- addressed (some) comments from Daniel Stone, Liviu Dudau, Daniel Vetter
and Brian Starkey - thank you all

In this iteration some rework has been done. The checking logic is now moved
to framebuffer_check() so it is common to all drivers. But the common part
is not good for komeda, so this series is not good for merging yet.
I kindly ask for feedback whether the changes are in the right direction.
I also kindly ask for input on how to accommodate komeda.

The CONFIG_DRM_AFBC option has been eliminated in favour of adding
drm_afbc.c to drm_kms_helper.

v1..v2:

- addressed comments from Daniel Stone, Ayan Halder, Mihail Atanassov
- coding style fixes

Andrzej Pietrasiewicz (6):
  drm/core: Allow drivers allocate a subclass of struct drm_framebuffer
  drm/core: Add drm_afbc_framebuffer and a corresponding helper
  drm/arm/malidp: Factor-in framebuffer creation
  drm/arm/malidp: Allocate an afbc-specific drm_framebuffer
  drm/arm/malidp: Switch to afbc helpers
  drm/rockchip: Add support for afbc

 Documentation/gpu/todo.rst                   |  15 ++
 drivers/gpu/drm/arm/malidp_drv.c             | 151 ++++++--------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 195 ++++++++++++++++---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h  |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |  43 +++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c  | 137 ++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h  |  17 ++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c  |  83 +++++++-
 include/drm/drm_framebuffer.h                |  45 +++++
 include/drm/drm_gem_framebuffer_helper.h     |  15 ++
 10 files changed, 580 insertions(+), 122 deletions(-)


base-commit: 41252c6d1c3bc640c3283a797400719fbe7dcec1
-- 
2.17.1

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

[PATCHv7 1/6] drm/core: Allow drivers allocate a subclass of struct drm_framebuffer

[Andrzej Pietrasiewicz]

Allow allocating a specialized version of struct drm_framebuffer
by moving the actual fb allocation out of drm_gem_fb_create_with_funcs();
the respective functions names are adjusted to reflect that fact.
Please note, though, that standard size checks are performed on buffers,
so the drm_gem_fb_init_with_funcs() is useful for cases where those
standard size checks are appropriate or at least don't conflict the
checks to be performed in the specialized case.

Thanks to this change the drivers can call drm_gem_fb_init_with_funcs()
having allocated their special version of struct drm_framebuffer, exactly
the way the new version of drm_gem_fb_create_with_funcs() does.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 87 ++++++++++++++------
 include/drm/drm_gem_framebuffer_helper.h     |  5 ++
 2 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 3a7ace19a902..86c1907c579a 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -54,19 +54,15 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
 
-static struct drm_framebuffer *
-drm_gem_fb_alloc(struct drm_device *dev,
+static int
+drm_gem_fb_init(struct drm_device *dev,
+		 struct drm_framebuffer *fb,
 		 const struct drm_mode_fb_cmd2 *mode_cmd,
 		 struct drm_gem_object **obj, unsigned int num_planes,
 		 const struct drm_framebuffer_funcs *funcs)
 {
-	struct drm_framebuffer *fb;
 	int ret, i;
 
-	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
-	if (!fb)
-		return ERR_PTR(-ENOMEM);
-
 	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
 
 	for (i = 0; i < num_planes; i++)
@@ -76,10 +72,9 @@ drm_gem_fb_alloc(struct drm_device *dev,
 	if (ret) {
 		drm_err(dev, "Failed to init framebuffer: %d\n", ret);
 		kfree(fb);
-		return ERR_PTR(ret);
 	}
 
-	return fb;
+	return ret;
 }
 
 /**
@@ -123,10 +118,13 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
 EXPORT_SYMBOL(drm_gem_fb_create_handle);
 
 /**
- * drm_gem_fb_create_with_funcs() - Helper function for the
- *                                  &drm_mode_config_funcs.fb_create
- *                                  callback
+ * drm_gem_fb_init_with_funcs() - Helper function for implementing
+ *				  &drm_mode_config_funcs.fb_create
+ *				  callback in cases when the driver
+ *				  allocates a subclass of
+ *				  struct drm_framebuffer
  * @dev: DRM device
+ * @fb: framebuffer object
  * @file: DRM file that holds the GEM handle(s) backing the framebuffer
  * @mode_cmd: Metadata from the userspace framebuffer creation request
  * @funcs: vtable to be used for the new framebuffer object
@@ -134,23 +132,26 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle);
  * This function can be used to set &drm_framebuffer_funcs for drivers that need
  * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
  * change &drm_framebuffer_funcs. The function does buffer size validation.
+ * The buffer size validation is for a general case, though, so users should
+ * pay attention to the checks being appropriate for them or, at least,
+ * non-conflicting.
  *
  * Returns:
- * Pointer to a &drm_framebuffer on success or an error pointer on failure.
+ * Zero or a negative error code.
  */
-struct drm_framebuffer *
-drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
-			     const struct drm_mode_fb_cmd2 *mode_cmd,
-			     const struct drm_framebuffer_funcs *funcs)
+int drm_gem_fb_init_with_funcs(struct drm_device *dev,
+			       struct drm_framebuffer *fb,
+			       struct drm_file *file,
+			       const struct drm_mode_fb_cmd2 *mode_cmd,
+			       const struct drm_framebuffer_funcs *funcs)
 {
 	const struct drm_format_info *info;
 	struct drm_gem_object *objs[4];
-	struct drm_framebuffer *fb;
 	int ret, i;
 
 	info = drm_get_format_info(dev, mode_cmd);
 	if (!info)
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
 	for (i = 0; i < info->num_planes; i++) {
 		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
@@ -175,19 +176,55 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
 		}
 	}
 
-	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
-	if (IS_ERR(fb)) {
-		ret = PTR_ERR(fb);
+	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
+	if (ret)
 		goto err_gem_object_put;
-	}
 
-	return fb;
+	return 0;
 
 err_gem_object_put:
 	for (i--; i >= 0; i--)
 		drm_gem_object_put_unlocked(objs[i]);
 
-	return ERR_PTR(ret);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
+
+/**
+ * drm_gem_fb_create_with_funcs() - Helper function for the
+ *                                  &drm_mode_config_funcs.fb_create
+ *                                  callback
+ * @dev: DRM device
+ * @file: DRM file that holds the GEM handle(s) backing the framebuffer
+ * @mode_cmd: Metadata from the userspace framebuffer creation request
+ * @funcs: vtable to be used for the new framebuffer object
+ *
+ * This function can be used to set &drm_framebuffer_funcs for drivers that need
+ * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
+ * change &drm_framebuffer_funcs. The function does buffer size validation.
+ *
+ * Returns:
+ * Pointer to a &drm_framebuffer on success or an error pointer on failure.
+ */
+struct drm_framebuffer *
+drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
+			     const struct drm_mode_fb_cmd2 *mode_cmd,
+			     const struct drm_framebuffer_funcs *funcs)
+{
+	struct drm_framebuffer *fb;
+	int ret;
+
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb)
+		return ERR_PTR(-ENOMEM);
+
+	ret = drm_gem_fb_init_with_funcs(dev, fb, file, mode_cmd, funcs);
+	if (ret) {
+		kfree(fb);
+		return ERR_PTR(ret);
+	}
+
+	return fb;
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
 
diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
index d9f13fd25b0a..c029c1618661 100644
--- a/include/drm/drm_gem_framebuffer_helper.h
+++ b/include/drm/drm_gem_framebuffer_helper.h
@@ -18,6 +18,11 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb);
 int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
 			     unsigned int *handle);
 
+int drm_gem_fb_init_with_funcs(struct drm_device *dev,
+			       struct drm_framebuffer *fb,
+			       struct drm_file *file,
+			       const struct drm_mode_fb_cmd2 *mode_cmd,
+			       const struct drm_framebuffer_funcs *funcs);
 struct drm_framebuffer *
 drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
 			     const struct drm_mode_fb_cmd2 *mode_cmd,
-- 
2.17.1

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

Re: [PATCHv7 1/6] drm/core: Allow drivers allocate a subclass of struct drm_framebuffer

[james qian wang (Arm Technology China)]

On Wed, Mar 11, 2020 at 10:55:36PM +0800, Andrzej Pietrasiewicz wrote:
> Allow allocating a specialized version of struct drm_framebuffer
> by moving the actual fb allocation out of drm_gem_fb_create_with_funcs();
> the respective functions names are adjusted to reflect that fact.
> Please note, though, that standard size checks are performed on buffers,
> so the drm_gem_fb_init_with_funcs() is useful for cases where those
> standard size checks are appropriate or at least don't conflict the
> checks to be performed in the specialized case.
> 
> Thanks to this change the drivers can call drm_gem_fb_init_with_funcs()
> having allocated their special version of struct drm_framebuffer, exactly
> the way the new version of drm_gem_fb_create_with_funcs() does.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Looks good to me. :)

Reviewed-by: James Qian Wang <james.qian.wang@arm.com>

Thanks
James

> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 87 ++++++++++++++------
>  include/drm/drm_gem_framebuffer_helper.h     |  5 ++
>  2 files changed, 67 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 3a7ace19a902..86c1907c579a 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -54,19 +54,15 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
>  
> -static struct drm_framebuffer *
> -drm_gem_fb_alloc(struct drm_device *dev,
> +static int
> +drm_gem_fb_init(struct drm_device *dev,
> +		 struct drm_framebuffer *fb,
>  		 const struct drm_mode_fb_cmd2 *mode_cmd,
>  		 struct drm_gem_object **obj, unsigned int num_planes,
>  		 const struct drm_framebuffer_funcs *funcs)
>  {
> -	struct drm_framebuffer *fb;
>  	int ret, i;
>  
> -	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> -	if (!fb)
> -		return ERR_PTR(-ENOMEM);
> -
>  	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>  
>  	for (i = 0; i < num_planes; i++)
> @@ -76,10 +72,9 @@ drm_gem_fb_alloc(struct drm_device *dev,
>  	if (ret) {
>  		drm_err(dev, "Failed to init framebuffer: %d\n", ret);
>  		kfree(fb);
> -		return ERR_PTR(ret);
>  	}
>  
> -	return fb;
> +	return ret;
>  }
>  
>  /**
> @@ -123,10 +118,13 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
>  EXPORT_SYMBOL(drm_gem_fb_create_handle);
>  
>  /**
> - * drm_gem_fb_create_with_funcs() - Helper function for the
> - *                                  &drm_mode_config_funcs.fb_create
> - *                                  callback
> + * drm_gem_fb_init_with_funcs() - Helper function for implementing
> + *				  &drm_mode_config_funcs.fb_create
> + *				  callback in cases when the driver
> + *				  allocates a subclass of
> + *				  struct drm_framebuffer
>   * @dev: DRM device
> + * @fb: framebuffer object
>   * @file: DRM file that holds the GEM handle(s) backing the framebuffer
>   * @mode_cmd: Metadata from the userspace framebuffer creation request
>   * @funcs: vtable to be used for the new framebuffer object
> @@ -134,23 +132,26 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle);
>   * This function can be used to set &drm_framebuffer_funcs for drivers that need
>   * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
>   * change &drm_framebuffer_funcs. The function does buffer size validation.
> + * The buffer size validation is for a general case, though, so users should
> + * pay attention to the checks being appropriate for them or, at least,
> + * non-conflicting.
>   *
>   * Returns:
> - * Pointer to a &drm_framebuffer on success or an error pointer on failure.
> + * Zero or a negative error code.
>   */
> -struct drm_framebuffer *
> -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> -			     const struct drm_mode_fb_cmd2 *mode_cmd,
> -			     const struct drm_framebuffer_funcs *funcs)
> +int drm_gem_fb_init_with_funcs(struct drm_device *dev,
> +			       struct drm_framebuffer *fb,
> +			       struct drm_file *file,
> +			       const struct drm_mode_fb_cmd2 *mode_cmd,
> +			       const struct drm_framebuffer_funcs *funcs)
>  {
>  	const struct drm_format_info *info;
>  	struct drm_gem_object *objs[4];
> -	struct drm_framebuffer *fb;
>  	int ret, i;
>  
>  	info = drm_get_format_info(dev, mode_cmd);
>  	if (!info)
> -		return ERR_PTR(-EINVAL);
> +		return -EINVAL;
>  
>  	for (i = 0; i < info->num_planes; i++) {
>  		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> @@ -175,19 +176,55 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
>  		}
>  	}
>  
> -	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
> -	if (IS_ERR(fb)) {
> -		ret = PTR_ERR(fb);
> +	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
> +	if (ret)
>  		goto err_gem_object_put;
> -	}
>  
> -	return fb;
> +	return 0;
>  
>  err_gem_object_put:
>  	for (i--; i >= 0; i--)
>  		drm_gem_object_put_unlocked(objs[i]);
>  
> -	return ERR_PTR(ret);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
> +
> +/**
> + * drm_gem_fb_create_with_funcs() - Helper function for the
> + *                                  &drm_mode_config_funcs.fb_create
> + *                                  callback
> + * @dev: DRM device
> + * @file: DRM file that holds the GEM handle(s) backing the framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @funcs: vtable to be used for the new framebuffer object
> + *
> + * This function can be used to set &drm_framebuffer_funcs for drivers that need
> + * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
> + * change &drm_framebuffer_funcs. The function does buffer size validation.
> + *
> + * Returns:
> + * Pointer to a &drm_framebuffer on success or an error pointer on failure.
> + */
> +struct drm_framebuffer *
> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> +			     const struct drm_framebuffer_funcs *funcs)
> +{
> +	struct drm_framebuffer *fb;
> +	int ret;
> +
> +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> +	if (!fb)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = drm_gem_fb_init_with_funcs(dev, fb, file, mode_cmd, funcs);
> +	if (ret) {
> +		kfree(fb);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return fb;
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
>  
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index d9f13fd25b0a..c029c1618661 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -18,6 +18,11 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb);
>  int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
>  			     unsigned int *handle);
>  
> +int drm_gem_fb_init_with_funcs(struct drm_device *dev,
> +			       struct drm_framebuffer *fb,
> +			       struct drm_file *file,
> +			       const struct drm_mode_fb_cmd2 *mode_cmd,
> +			       const struct drm_framebuffer_funcs *funcs);
>  struct drm_framebuffer *
>  drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
>  			     const struct drm_mode_fb_cmd2 *mode_cmd,
> -- 
> 2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[Andrzej Pietrasiewicz]

The new struct contains afbc-specific data.

The new function can be used by drivers which support afbc to complete
the preparation of struct drm_afbc_framebuffer. It must be called after
allocating the said struct and calling drm_gem_fb_init_with_funcs().

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 Documentation/gpu/todo.rst                   |  15 +++
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
 include/drm/drm_framebuffer.h                |  45 ++++++++
 include/drm/drm_gem_framebuffer_helper.h     |  10 ++
 4 files changed, 178 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 439656f55c5d..37a3a023c114 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
 
 Level: Intermediate
 
+Encode cpp properly in malidp
+-----------------------------
+
+cpp (chars per pixel) is not encoded properly in malidp, zero is
+used instead. afbc implementation needs bpp or cpp, but if it is
+zero it needs to be provided elsewhere, and so the bpp field exists
+in struct drm_afbc_framebuffer.
+
+Properly encode cpp in malidp and remove the bpp field in struct
+drm_afbc_framebuffer.
+
+Contact: malidp maintainers
+
+Level: Intermediate
+
 Core refactorings
 =================
 
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 86c1907c579a..7e3982c36baa 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -21,6 +21,13 @@
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
+#define AFBC_HEADER_SIZE		16
+#define AFBC_TH_LAYOUT_ALIGNMENT	8
+#define AFBC_HDR_ALIGN			64
+#define AFBC_SUPERBLOCK_PIXELS		256
+#define AFBC_SUPERBLOCK_ALIGNMENT	128
+#define AFBC_TH_BODY_START_ALIGNMENT	4096
+
 /**
  * DOC: overview
  *
@@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
 
+static int drm_gem_afbc_min_size(struct drm_device *dev,
+				 const struct drm_mode_fb_cmd2 *mode_cmd,
+				 struct drm_afbc_framebuffer *afbc_fb)
+{
+	const struct drm_format_info *info;
+	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
+	/* remove bpp when all users properly encode cpp in drm_format_info */
+	__u32 bpp;
+
+	switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+		afbc_fb->block_width = 16;
+		afbc_fb->block_height = 16;
+		break;
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
+		afbc_fb->block_width = 32;
+		afbc_fb->block_height = 8;
+		break;
+	/* no user exists yet - fall through */
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
+	default:
+		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
+			      mode_cmd->modifier[0]
+			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
+		return -EINVAL;
+	}
+
+	/* tiled header afbc */
+	w_alignment = afbc_fb->block_width;
+	h_alignment = afbc_fb->block_height;
+	hdr_alignment = AFBC_HDR_ALIGN;
+	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
+		w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
+		h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
+		hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
+	}
+
+	afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
+	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
+	afbc_fb->offset = mode_cmd->offsets[0];
+
+	info = drm_get_format_info(dev, mode_cmd);
+	/*
+	 * Change to always using info->cpp[0]
+	 * when all users properly encode it
+	 */
+	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
+
+	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
+		   / AFBC_SUPERBLOCK_PIXELS;
+	afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
+	afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
+					       AFBC_SUPERBLOCK_ALIGNMENT);
+
+	return 0;
+}
+
+/**
+ * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
+ *			    fill and validate all the afbc-specific
+ *			    struct drm_afbc_framebuffer members
+ *
+ * @dev: DRM device
+ * @afbc_fb: afbc-specific framebuffer
+ * @mode_cmd: Metadata from the userspace framebuffer creation request
+ * @afbc_fb: afbc framebuffer
+ *
+ * This function can be used by drivers which support afbc to complete
+ * the preparation of struct drm_afbc_framebuffer. It must be called after
+ * allocating the said struct and calling drm_gem_fb_init_with_funcs().
+ * It is caller's responsibility to put afbc_fb->base.obj objects in case
+ * the call is unsuccessful.
+ *
+ * Returns:
+ * Zero on success or a negative error value on failure.
+ */
+int drm_gem_fb_afbc_init(struct drm_device *dev,
+			 const struct drm_mode_fb_cmd2 *mode_cmd,
+			 struct drm_afbc_framebuffer *afbc_fb)
+{
+	const struct drm_format_info *info;
+	struct drm_gem_object **objs;
+	int ret;
+
+	objs = afbc_fb->base.obj;
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return -EINVAL;
+
+	ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
+	if (ret < 0)
+		return ret;
+
+	if (objs[0]->size < afbc_fb->afbc_size)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
+
 /**
  * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
  * @plane: Plane
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index c0e0256e3e98..e9f1b0e2968d 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
 int drm_framebuffer_plane_height(int height,
 				 const struct drm_framebuffer *fb, int plane);
 
+/**
+ * struct drm_afbc_framebuffer - a special afbc frame buffer object
+ *
+ * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
+ */
+struct drm_afbc_framebuffer {
+	/**
+	 * @base: base framebuffer structure.
+	 */
+	struct drm_framebuffer base;
+	/**
+	 * @block_widht: width of a single afbc block
+	 */
+	u32 block_width;
+	/**
+	 * @block_widht: height of a single afbc block
+	 */
+	u32 block_height;
+	/**
+	 * @aligned_width: aligned frame buffer width
+	 */
+	u32 aligned_width;
+	/**
+	 * @aligned_height: aligned frame buffer height
+	 */
+	u32 aligned_height;
+	/**
+	 * @offset: offset of the first afbc header
+	 */
+	u32 offset;
+	/**
+	 * @afbc_size: minimum size of afbc buffer
+	 */
+	u32 afbc_size;
+	/**
+	 * @bpp: bpp value for this afbc buffer
+	 * To be removed when users such as malidp
+	 * properly store the cpp in drm_format_info.
+	 * New users should not start using this field.
+	 */
+	u32 bpp;
+};
+
+#define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
+
 #endif
diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
index c029c1618661..6b013154911d 100644
--- a/include/drm/drm_gem_framebuffer_helper.h
+++ b/include/drm/drm_gem_framebuffer_helper.h
@@ -1,6 +1,7 @@
 #ifndef __DRM_GEM_FB_HELPER_H__
 #define __DRM_GEM_FB_HELPER_H__
 
+struct drm_afbc_framebuffer;
 struct drm_device;
 struct drm_fb_helper_surface_size;
 struct drm_file;
@@ -12,6 +13,8 @@ struct drm_plane;
 struct drm_plane_state;
 struct drm_simple_display_pipe;
 
+#define AFBC_VENDOR_AND_TYPE_MASK	GENMASK_ULL(63, 52)
+
 struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
 					  unsigned int plane);
 void drm_gem_fb_destroy(struct drm_framebuffer *fb);
@@ -34,6 +37,13 @@ struct drm_framebuffer *
 drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 			     const struct drm_mode_fb_cmd2 *mode_cmd);
 
+#define drm_is_afbc(modifier) \
+	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
+
+int drm_gem_fb_afbc_init(struct drm_device *dev,
+			 const struct drm_mode_fb_cmd2 *mode_cmd,
+			 struct drm_afbc_framebuffer *afbc_fb);
+
 int drm_gem_fb_prepare_fb(struct drm_plane *plane,
 			  struct drm_plane_state *state);
 int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
-- 
2.17.1

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

Re: [PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[james qian wang (Arm Technology China)]

On Wed, Mar 11, 2020 at 10:55:37PM +0800, Andrzej Pietrasiewicz wrote:
> The new struct contains afbc-specific data.
> 
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Looks good to me.

Reviewed-by: James Qian Wang <james.qian.wang@arm.com>

Thanks
James
> ---
>  Documentation/gpu/todo.rst                   |  15 +++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
>  include/drm/drm_framebuffer.h                |  45 ++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  10 ++
>  4 files changed, 178 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
>  
>  Level: Intermediate
>  
> +Encode cpp properly in malidp
> +-----------------------------
> +
> +cpp (chars per pixel) is not encoded properly in malidp, zero is
> +used instead. afbc implementation needs bpp or cpp, but if it is
> +zero it needs to be provided elsewhere, and so the bpp field exists
> +in struct drm_afbc_framebuffer.
> +
> +Properly encode cpp in malidp and remove the bpp field in struct
> +drm_afbc_framebuffer.
> +
> +Contact: malidp maintainers
> +
> +Level: Intermediate
> +
>  Core refactorings
>  =================
>  
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_simple_kms_helper.h>
>  
> +#define AFBC_HEADER_SIZE		16
> +#define AFBC_TH_LAYOUT_ALIGNMENT	8
> +#define AFBC_HDR_ALIGN			64
> +#define AFBC_SUPERBLOCK_PIXELS		256
> +#define AFBC_SUPERBLOCK_ALIGNMENT	128
> +#define AFBC_TH_BODY_START_ALIGNMENT	4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> +				 const struct drm_mode_fb_cmd2 *mode_cmd,
> +				 struct drm_afbc_framebuffer *afbc_fb)
> +{
> +	const struct drm_format_info *info;
> +	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
> +	/* remove bpp when all users properly encode cpp in drm_format_info */
> +	__u32 bpp;
> +
> +	switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> +		afbc_fb->block_width = 16;
> +		afbc_fb->block_height = 16;
> +		break;
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> +		afbc_fb->block_width = 32;
> +		afbc_fb->block_height = 8;
> +		break;
> +	/* no user exists yet - fall through */
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> +	default:
> +		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +			      mode_cmd->modifier[0]
> +			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +		return -EINVAL;
> +	}
> +
> +	/* tiled header afbc */
> +	w_alignment = afbc_fb->block_width;
> +	h_alignment = afbc_fb->block_height;
> +	hdr_alignment = AFBC_HDR_ALIGN;
> +	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +		w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> +	}
> +
> +	afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> +	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
> +	afbc_fb->offset = mode_cmd->offsets[0];
> +
> +	info = drm_get_format_info(dev, mode_cmd);
> +	/*
> +	 * Change to always using info->cpp[0]
> +	 * when all users properly encode it
> +	 */
> +	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +
> +	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> +		   / AFBC_SUPERBLOCK_PIXELS;
> +	afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> +	afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +					       AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +	return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *			    fill and validate all the afbc-specific
> + *			    struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb)
> +{
> +	const struct drm_format_info *info;
> +	struct drm_gem_object **objs;
> +	int ret;
> +
> +	objs = afbc_fb->base.obj;
> +	info = drm_get_format_info(dev, mode_cmd);
> +	if (!info)
> +		return -EINVAL;
> +
> +	ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (objs[0]->size < afbc_fb->afbc_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
>   * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
>  int drm_framebuffer_plane_height(int height,
>  				 const struct drm_framebuffer *fb, int plane);
>  
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> +	/**
> +	 * @base: base framebuffer structure.
> +	 */
> +	struct drm_framebuffer base;
> +	/**
> +	 * @block_widht: width of a single afbc block
> +	 */
> +	u32 block_width;
> +	/**
> +	 * @block_widht: height of a single afbc block
> +	 */
> +	u32 block_height;
> +	/**
> +	 * @aligned_width: aligned frame buffer width
> +	 */
> +	u32 aligned_width;
> +	/**
> +	 * @aligned_height: aligned frame buffer height
> +	 */
> +	u32 aligned_height;
> +	/**
> +	 * @offset: offset of the first afbc header
> +	 */
> +	u32 offset;
> +	/**
> +	 * @afbc_size: minimum size of afbc buffer
> +	 */
> +	u32 afbc_size;
> +	/**
> +	 * @bpp: bpp value for this afbc buffer
> +	 * To be removed when users such as malidp
> +	 * properly store the cpp in drm_format_info.
> +	 * New users should not start using this field.
> +	 */
> +	u32 bpp;
> +};
> +
> +#define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
> +
>  #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>  
> +struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
>  
> +#define AFBC_VENDOR_AND_TYPE_MASK	GENMASK_ULL(63, 52)
> +
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  					  unsigned int plane);
>  void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  			     const struct drm_mode_fb_cmd2 *mode_cmd);
>  
> +#define drm_is_afbc(modifier) \
> +	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb);
> +
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state);
>  int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> -- 
> 2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[Daniel Vetter]

On Wed, Mar 11, 2020 at 03:55:37PM +0100, Andrzej Pietrasiewicz wrote:
> The new struct contains afbc-specific data.
> 
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Yeah I think this now looks like a fairly clean interface for drivers. On
the first two core patches:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks for sticking around through all the fairly major revisions here!

Cheers, Daniel

> ---
>  Documentation/gpu/todo.rst                   |  15 +++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
>  include/drm/drm_framebuffer.h                |  45 ++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  10 ++
>  4 files changed, 178 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
>  
>  Level: Intermediate
>  
> +Encode cpp properly in malidp
> +-----------------------------
> +
> +cpp (chars per pixel) is not encoded properly in malidp, zero is
> +used instead. afbc implementation needs bpp or cpp, but if it is
> +zero it needs to be provided elsewhere, and so the bpp field exists
> +in struct drm_afbc_framebuffer.
> +
> +Properly encode cpp in malidp and remove the bpp field in struct
> +drm_afbc_framebuffer.
> +
> +Contact: malidp maintainers
> +
> +Level: Intermediate
> +
>  Core refactorings
>  =================
>  
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_simple_kms_helper.h>
>  
> +#define AFBC_HEADER_SIZE		16
> +#define AFBC_TH_LAYOUT_ALIGNMENT	8
> +#define AFBC_HDR_ALIGN			64
> +#define AFBC_SUPERBLOCK_PIXELS		256
> +#define AFBC_SUPERBLOCK_ALIGNMENT	128
> +#define AFBC_TH_BODY_START_ALIGNMENT	4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> +				 const struct drm_mode_fb_cmd2 *mode_cmd,
> +				 struct drm_afbc_framebuffer *afbc_fb)
> +{
> +	const struct drm_format_info *info;
> +	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
> +	/* remove bpp when all users properly encode cpp in drm_format_info */
> +	__u32 bpp;
> +
> +	switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> +		afbc_fb->block_width = 16;
> +		afbc_fb->block_height = 16;
> +		break;
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> +		afbc_fb->block_width = 32;
> +		afbc_fb->block_height = 8;
> +		break;
> +	/* no user exists yet - fall through */
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> +	default:
> +		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +			      mode_cmd->modifier[0]
> +			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +		return -EINVAL;
> +	}
> +
> +	/* tiled header afbc */
> +	w_alignment = afbc_fb->block_width;
> +	h_alignment = afbc_fb->block_height;
> +	hdr_alignment = AFBC_HDR_ALIGN;
> +	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +		w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> +	}
> +
> +	afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> +	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
> +	afbc_fb->offset = mode_cmd->offsets[0];
> +
> +	info = drm_get_format_info(dev, mode_cmd);
> +	/*
> +	 * Change to always using info->cpp[0]
> +	 * when all users properly encode it
> +	 */
> +	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +
> +	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> +		   / AFBC_SUPERBLOCK_PIXELS;
> +	afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> +	afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +					       AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +	return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *			    fill and validate all the afbc-specific
> + *			    struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb)
> +{
> +	const struct drm_format_info *info;
> +	struct drm_gem_object **objs;
> +	int ret;
> +
> +	objs = afbc_fb->base.obj;
> +	info = drm_get_format_info(dev, mode_cmd);
> +	if (!info)
> +		return -EINVAL;
> +
> +	ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (objs[0]->size < afbc_fb->afbc_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
>   * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
>  int drm_framebuffer_plane_height(int height,
>  				 const struct drm_framebuffer *fb, int plane);
>  
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> +	/**
> +	 * @base: base framebuffer structure.
> +	 */
> +	struct drm_framebuffer base;
> +	/**
> +	 * @block_widht: width of a single afbc block
> +	 */
> +	u32 block_width;
> +	/**
> +	 * @block_widht: height of a single afbc block
> +	 */
> +	u32 block_height;
> +	/**
> +	 * @aligned_width: aligned frame buffer width
> +	 */
> +	u32 aligned_width;
> +	/**
> +	 * @aligned_height: aligned frame buffer height
> +	 */
> +	u32 aligned_height;
> +	/**
> +	 * @offset: offset of the first afbc header
> +	 */
> +	u32 offset;
> +	/**
> +	 * @afbc_size: minimum size of afbc buffer
> +	 */
> +	u32 afbc_size;
> +	/**
> +	 * @bpp: bpp value for this afbc buffer
> +	 * To be removed when users such as malidp
> +	 * properly store the cpp in drm_format_info.
> +	 * New users should not start using this field.
> +	 */
> +	u32 bpp;
> +};
> +
> +#define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
> +
>  #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>  
> +struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
>  
> +#define AFBC_VENDOR_AND_TYPE_MASK	GENMASK_ULL(63, 52)
> +
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  					  unsigned int plane);
>  void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  			     const struct drm_mode_fb_cmd2 *mode_cmd);
>  
> +#define drm_is_afbc(modifier) \
> +	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb);
> +
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state);
>  int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> -- 
> 2.17.1
> 

-- 
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: [PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[Daniel Vetter]

On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> The new struct contains afbc-specific data.
>
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  Documentation/gpu/todo.rst                   |  15 +++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
>  include/drm/drm_framebuffer.h                |  45 ++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  10 ++
>  4 files changed, 178 insertions(+)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
>
>  Level: Intermediate
>
> +Encode cpp properly in malidp
> +-----------------------------
> +
> +cpp (chars per pixel) is not encoded properly in malidp, zero is
> +used instead. afbc implementation needs bpp or cpp, but if it is
> +zero it needs to be provided elsewhere, and so the bpp field exists
> +in struct drm_afbc_framebuffer.
> +
> +Properly encode cpp in malidp and remove the bpp field in struct
> +drm_afbc_framebuffer.
> +
> +Contact: malidp maintainers
> +
> +Level: Intermediate

Just stumbled over this todo, which is really surprising. Also
definitely not something I wanted to ack, earlier versions at least
didn't have this.

Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
which has a pointer to drm_format_info, which we're always setting
(the core does that for all drivers, both for addfb and addfb2). Why
is that not good enough to get at cpp for everyone?

Cheers, Daniel

> +
>  Core refactorings
>  =================
>
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_simple_kms_helper.h>
>
> +#define AFBC_HEADER_SIZE               16
> +#define AFBC_TH_LAYOUT_ALIGNMENT       8
> +#define AFBC_HDR_ALIGN                 64
> +#define AFBC_SUPERBLOCK_PIXELS         256
> +#define AFBC_SUPERBLOCK_ALIGNMENT      128
> +#define AFBC_TH_BODY_START_ALIGNMENT   4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>
> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> +                                const struct drm_mode_fb_cmd2 *mode_cmd,
> +                                struct drm_afbc_framebuffer *afbc_fb)
> +{
> +       const struct drm_format_info *info;
> +       __u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
> +       /* remove bpp when all users properly encode cpp in drm_format_info */
> +       __u32 bpp;
> +
> +       switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> +               afbc_fb->block_width = 16;
> +               afbc_fb->block_height = 16;
> +               break;
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> +               afbc_fb->block_width = 32;
> +               afbc_fb->block_height = 8;
> +               break;
> +       /* no user exists yet - fall through */
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> +       default:
> +               DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +                             mode_cmd->modifier[0]
> +                             & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +               return -EINVAL;
> +       }
> +
> +       /* tiled header afbc */
> +       w_alignment = afbc_fb->block_width;
> +       h_alignment = afbc_fb->block_height;
> +       hdr_alignment = AFBC_HDR_ALIGN;
> +       if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +               w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +               h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +               hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> +       }
> +
> +       afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> +       afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
> +       afbc_fb->offset = mode_cmd->offsets[0];
> +
> +       info = drm_get_format_info(dev, mode_cmd);
> +       /*
> +        * Change to always using info->cpp[0]
> +        * when all users properly encode it
> +        */
> +       bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +
> +       n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> +                  / AFBC_SUPERBLOCK_PIXELS;
> +       afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> +       afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +                                              AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +       return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *                         fill and validate all the afbc-specific
> + *                         struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +                        const struct drm_mode_fb_cmd2 *mode_cmd,
> +                        struct drm_afbc_framebuffer *afbc_fb)
> +{
> +       const struct drm_format_info *info;
> +       struct drm_gem_object **objs;
> +       int ret;
> +
> +       objs = afbc_fb->base.obj;
> +       info = drm_get_format_info(dev, mode_cmd);
> +       if (!info)
> +               return -EINVAL;
> +
> +       ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (objs[0]->size < afbc_fb->afbc_size)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
>   * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
>  int drm_framebuffer_plane_height(int height,
>                                  const struct drm_framebuffer *fb, int plane);
>
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> +       /**
> +        * @base: base framebuffer structure.
> +        */
> +       struct drm_framebuffer base;
> +       /**
> +        * @block_widht: width of a single afbc block
> +        */
> +       u32 block_width;
> +       /**
> +        * @block_widht: height of a single afbc block
> +        */
> +       u32 block_height;
> +       /**
> +        * @aligned_width: aligned frame buffer width
> +        */
> +       u32 aligned_width;
> +       /**
> +        * @aligned_height: aligned frame buffer height
> +        */
> +       u32 aligned_height;
> +       /**
> +        * @offset: offset of the first afbc header
> +        */
> +       u32 offset;
> +       /**
> +        * @afbc_size: minimum size of afbc buffer
> +        */
> +       u32 afbc_size;
> +       /**
> +        * @bpp: bpp value for this afbc buffer
> +        * To be removed when users such as malidp
> +        * properly store the cpp in drm_format_info.
> +        * New users should not start using this field.
> +        */
> +       u32 bpp;
> +};
> +
> +#define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
> +
>  #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>
> +struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
>
> +#define AFBC_VENDOR_AND_TYPE_MASK      GENMASK_ULL(63, 52)
> +
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>                                           unsigned int plane);
>  void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>                              const struct drm_mode_fb_cmd2 *mode_cmd);
>
> +#define drm_is_afbc(modifier) \
> +       (((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +                        const struct drm_mode_fb_cmd2 *mode_cmd,
> +                        struct drm_afbc_framebuffer *afbc_fb);
> +
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>                           struct drm_plane_state *state);
>  int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> --
> 2.17.1
>


--
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: [PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[Andrzej Pietrasiewicz]

Hi Daniel,

W dniu 30.03.2020 o 19:01, Daniel Vetter pisze:
> On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
>>
>> The new struct contains afbc-specific data.

<snip>

>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 439656f55c5d..37a3a023c114 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
>>
>>   Level: Intermediate
>>
>> +Encode cpp properly in malidp
>> +-----------------------------
>> +
>> +cpp (chars per pixel) is not encoded properly in malidp, zero is
>> +used instead. afbc implementation needs bpp or cpp, but if it is
>> +zero it needs to be provided elsewhere, and so the bpp field exists
>> +in struct drm_afbc_framebuffer.
>> +
>> +Properly encode cpp in malidp and remove the bpp field in struct
>> +drm_afbc_framebuffer.
>> +
>> +Contact: malidp maintainers
>> +
>> +Level: Intermediate
> 
> Just stumbled over this todo, which is really surprising. Also
> definitely not something I wanted to ack, earlier versions at least
> didn't have this.
> 
> Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
> which has a pointer to drm_format_info, which we're always setting
> (the core does that for all drivers, both for addfb and addfb2). Why
> is that not good enough to get at cpp for everyone?
> 
> Cheers, Daniel
> 

Let me quote James https://patchwork.freedesktop.org/patch/345603/#comment_653081:

"Seems we can remove this bpp or no need to define it as a pass in argument
for size check, maybe the komeda/malidp get_afbc_bpp() function mislead
you that afbc formats may have vendor specific bpp.

But the story is:

for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set
nothing in drm_format_info, neither cpp nor block_size, so both malidp
or komeda introduce a get_bpp(), but actually the two funcs basically
are same.

So my suggestion is we can temporary use the get_afbc_bpp() in malidp
or komeda. and eventually I think we'd better set the block size
for these formats, then we can defines a common get_bpp() like pitch".

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

Re: [PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[Daniel Vetter]

On Mon, Mar 30, 2020 at 7:44 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Hi Daniel,
>
> W dniu 30.03.2020 o 19:01, Daniel Vetter pisze:
> > On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
> > <andrzej.p@collabora.com> wrote:
> >>
> >> The new struct contains afbc-specific data.
>
> <snip>
>
> >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> >> index 439656f55c5d..37a3a023c114 100644
> >> --- a/Documentation/gpu/todo.rst
> >> +++ b/Documentation/gpu/todo.rst
> >> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
> >>
> >>   Level: Intermediate
> >>
> >> +Encode cpp properly in malidp
> >> +-----------------------------
> >> +
> >> +cpp (chars per pixel) is not encoded properly in malidp, zero is
> >> +used instead. afbc implementation needs bpp or cpp, but if it is
> >> +zero it needs to be provided elsewhere, and so the bpp field exists
> >> +in struct drm_afbc_framebuffer.
> >> +
> >> +Properly encode cpp in malidp and remove the bpp field in struct
> >> +drm_afbc_framebuffer.
> >> +
> >> +Contact: malidp maintainers
> >> +
> >> +Level: Intermediate
> >
> > Just stumbled over this todo, which is really surprising. Also
> > definitely not something I wanted to ack, earlier versions at least
> > didn't have this.
> >
> > Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
> > which has a pointer to drm_format_info, which we're always setting
> > (the core does that for all drivers, both for addfb and addfb2). Why
> > is that not good enough to get at cpp for everyone?
> >
> > Cheers, Daniel
> >
>
> Let me quote James https://patchwork.freedesktop.org/patch/345603/#comment_653081:
>
> "Seems we can remove this bpp or no need to define it as a pass in argument
> for size check, maybe the komeda/malidp get_afbc_bpp() function mislead
> you that afbc formats may have vendor specific bpp.
>
> But the story is:
>
> for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set
> nothing in drm_format_info, neither cpp nor block_size, so both malidp
> or komeda introduce a get_bpp(), but actually the two funcs basically
> are same.
>
> So my suggestion is we can temporary use the get_afbc_bpp() in malidp
> or komeda. and eventually I think we'd better set the block size
> for these formats, then we can defines a common get_bpp() like pitch".

Hm iirc we had some good reasons not to set the block size for these,
but maybe with these afbc helpers that's changed. We could also
compute cpp/bpp in the helper, starting from the format_info/fourcc
code, for these special cases. Essentially move the exact computation
that komeda/malidp do right now to set afbc->bpp and move it into the
helper. Just kinda feels like unfinished work that we still leave this
to drivers, that's some very quirky api.
-Daniel
-- 
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

[PATCH 0/2] AFBC fixes

[Andrzej Pietrasiewicz]

This short series addresses the way bpp value is made available
for drm_gem_afbc_min_size(). It can be inferred from the format
if a driver hasn't set anything in cpp[0] (patch 2/2).

While at it, use proper debugging macro (patch 1/2).

Andrzej Pietrasiewicz (2):
  drm/core: Use proper debugging macro
  drm/core: Calculate bpp in afbc helper

 Documentation/gpu/todo.rst                   | 15 -------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 45 +++++++++++++++-----
 include/drm/drm_framebuffer.h                |  7 ---
 3 files changed, 35 insertions(+), 32 deletions(-)

-- 
2.17.1

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

[PATCH 1/2] drm/core: Use proper debugging macro

[Andrzej Pietrasiewicz]

Use drm_dbg_kms() instead of DRM_DEBUG_KMS.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 7e3982c36baa..6fb1841fa71c 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -331,9 +331,9 @@ static int drm_gem_afbc_min_size(struct drm_device *dev,
 	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
 	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
 	default:
-		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
-			      mode_cmd->modifier[0]
-			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
+		drm_dbg_kms(dev, "Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
+			    mode_cmd->modifier[0]
+			    & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
 		return -EINVAL;
 	}
 
-- 
2.17.1

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

Re: [PATCH 1/2] drm/core: Use proper debugging macro

[Daniel Vetter]

On Tue, Mar 31, 2020 at 05:53:07PM +0200, Andrzej Pietrasiewicz wrote:
> Use drm_dbg_kms() instead of DRM_DEBUG_KMS.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 7e3982c36baa..6fb1841fa71c 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -331,9 +331,9 @@ static int drm_gem_afbc_min_size(struct drm_device *dev,
>  	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
>  	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
>  	default:
> -		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> -			      mode_cmd->modifier[0]
> -			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +		drm_dbg_kms(dev, "Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +			    mode_cmd->modifier[0]
> +			    & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
>  		return -EINVAL;
>  	}
>  
> -- 
> 2.17.1
> 

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

[PATCH 2/2] drm/core: Calculate bpp in afbc helper

[Andrzej Pietrasiewicz]

Some drivers (komeda, malidp) don't set anything in cpp. If that is the
case the right value can be inferred from the format. Then the "bpp" member
can be eliminated from struct drm_afbc_framebuffer.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 Documentation/gpu/todo.rst                   | 15 --------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 39 ++++++++++++++++----
 include/drm/drm_framebuffer.h                |  7 ----
 3 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 37a3a023c114..439656f55c5d 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -404,21 +404,6 @@ Contact: Laurent Pinchart, respective driver maintainers
 
 Level: Intermediate
 
-Encode cpp properly in malidp
------------------------------
-
-cpp (chars per pixel) is not encoded properly in malidp, zero is
-used instead. afbc implementation needs bpp or cpp, but if it is
-zero it needs to be provided elsewhere, and so the bpp field exists
-in struct drm_afbc_framebuffer.
-
-Properly encode cpp in malidp and remove the bpp field in struct
-drm_afbc_framebuffer.
-
-Contact: malidp maintainers
-
-Level: Intermediate
-
 Core refactorings
 =================
 
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 6fb1841fa71c..cac15294aef6 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -309,11 +309,37 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
 
+static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev,
+				  const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	const struct drm_format_info *info;
+
+	info = drm_get_format_info(dev, mode_cmd);
+
+	/* use whatever a driver has set */
+	if (info->cpp[0])
+		return info->cpp[0] * 8;
+
+	/* guess otherwise */
+	switch (info->format) {
+	case DRM_FORMAT_YUV420_8BIT:
+		return 12;
+	case DRM_FORMAT_YUV420_10BIT:
+		return 15;
+	case DRM_FORMAT_VUY101010:
+		return 30;
+	default:
+		break;
+	}
+
+	/* all attempts failed */
+	return 0;
+}
+
 static int drm_gem_afbc_min_size(struct drm_device *dev,
 				 const struct drm_mode_fb_cmd2 *mode_cmd,
 				 struct drm_afbc_framebuffer *afbc_fb)
 {
-	const struct drm_format_info *info;
 	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
 	/* remove bpp when all users properly encode cpp in drm_format_info */
 	__u32 bpp;
@@ -351,12 +377,11 @@ static int drm_gem_afbc_min_size(struct drm_device *dev,
 	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
 	afbc_fb->offset = mode_cmd->offsets[0];
 
-	info = drm_get_format_info(dev, mode_cmd);
-	/*
-	 * Change to always using info->cpp[0]
-	 * when all users properly encode it
-	 */
-	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
+	bpp = drm_gem_afbc_get_bpp(dev, mode_cmd);
+	if (!bpp) {
+		drm_dbg_kms(dev, "Invalid AFBC bpp value: %d\n", bpp);
+		return -EINVAL;
+	}
 
 	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
 		   / AFBC_SUPERBLOCK_PIXELS;
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index b53c0332f040..be658ebbec72 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -331,13 +331,6 @@ struct drm_afbc_framebuffer {
 	 * @afbc_size: minimum size of afbc buffer
 	 */
 	u32 afbc_size;
-	/**
-	 * @bpp: bpp value for this afbc buffer
-	 * To be removed when users such as malidp
-	 * properly store the cpp in drm_format_info.
-	 * New users should not start using this field.
-	 */
-	u32 bpp;
 };
 
 #define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
-- 
2.17.1

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

Re: [PATCH 2/2] drm/core: Calculate bpp in afbc helper

[Daniel Vetter]

On Tue, Mar 31, 2020 at 05:53:08PM +0200, Andrzej Pietrasiewicz wrote:
> Some drivers (komeda, malidp) don't set anything in cpp. If that is the
> case the right value can be inferred from the format. Then the "bpp" member
> can be eliminated from struct drm_afbc_framebuffer.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Didn't check computations, but yup this matches what I had in mind.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/gpu/todo.rst                   | 15 --------
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 39 ++++++++++++++++----
>  include/drm/drm_framebuffer.h                |  7 ----
>  3 files changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 37a3a023c114..439656f55c5d 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,21 +404,6 @@ Contact: Laurent Pinchart, respective driver maintainers
>  
>  Level: Intermediate
>  
> -Encode cpp properly in malidp
> ------------------------------
> -
> -cpp (chars per pixel) is not encoded properly in malidp, zero is
> -used instead. afbc implementation needs bpp or cpp, but if it is
> -zero it needs to be provided elsewhere, and so the bpp field exists
> -in struct drm_afbc_framebuffer.
> -
> -Properly encode cpp in malidp and remove the bpp field in struct
> -drm_afbc_framebuffer.
> -
> -Contact: malidp maintainers
> -
> -Level: Intermediate
> -
>  Core refactorings
>  =================
>  
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 6fb1841fa71c..cac15294aef6 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -309,11 +309,37 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev,
> +				  const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_get_format_info(dev, mode_cmd);
> +
> +	/* use whatever a driver has set */
> +	if (info->cpp[0])
> +		return info->cpp[0] * 8;
> +
> +	/* guess otherwise */
> +	switch (info->format) {
> +	case DRM_FORMAT_YUV420_8BIT:
> +		return 12;
> +	case DRM_FORMAT_YUV420_10BIT:
> +		return 15;
> +	case DRM_FORMAT_VUY101010:
> +		return 30;
> +	default:
> +		break;
> +	}
> +
> +	/* all attempts failed */
> +	return 0;
> +}
> +
>  static int drm_gem_afbc_min_size(struct drm_device *dev,
>  				 const struct drm_mode_fb_cmd2 *mode_cmd,
>  				 struct drm_afbc_framebuffer *afbc_fb)
>  {
> -	const struct drm_format_info *info;
>  	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
>  	/* remove bpp when all users properly encode cpp in drm_format_info */
>  	__u32 bpp;
> @@ -351,12 +377,11 @@ static int drm_gem_afbc_min_size(struct drm_device *dev,
>  	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
>  	afbc_fb->offset = mode_cmd->offsets[0];
>  
> -	info = drm_get_format_info(dev, mode_cmd);
> -	/*
> -	 * Change to always using info->cpp[0]
> -	 * when all users properly encode it
> -	 */
> -	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +	bpp = drm_gem_afbc_get_bpp(dev, mode_cmd);
> +	if (!bpp) {
> +		drm_dbg_kms(dev, "Invalid AFBC bpp value: %d\n", bpp);
> +		return -EINVAL;
> +	}
>  
>  	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
>  		   / AFBC_SUPERBLOCK_PIXELS;
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index b53c0332f040..be658ebbec72 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -331,13 +331,6 @@ struct drm_afbc_framebuffer {
>  	 * @afbc_size: minimum size of afbc buffer
>  	 */
>  	u32 afbc_size;
> -	/**
> -	 * @bpp: bpp value for this afbc buffer
> -	 * To be removed when users such as malidp
> -	 * properly store the cpp in drm_format_info.
> -	 * New users should not start using this field.
> -	 */
> -	u32 bpp;
>  };
>  
>  #define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
> -- 
> 2.17.1
> 

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

[PATCHv7 3/6] drm/arm/malidp: Factor-in framebuffer creation

[Andrzej Pietrasiewicz]

Consolidating framebuffer creation into one function will make it easier
to transition to generic afbc-aware helpers.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/arm/malidp_drv.c | 157 +++++++++++++------------------
 1 file changed, 66 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 37d92a06318e..cafbd81e4c1e 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -269,112 +269,87 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
 	.atomic_commit_tail = malidp_atomic_commit_tail,
 };
 
-static bool
-malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
-				    const struct drm_mode_fb_cmd2 *mode_cmd)
+static struct drm_framebuffer *
+malidp_fb_create(struct drm_device *dev, struct drm_file *file,
+		 const struct drm_mode_fb_cmd2 *mode_cmd)
 {
-	if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
-					mode_cmd->modifier[0]) == false)
-		return false;
+	if (mode_cmd->modifier[0]) {
+		int n_superblocks = 0;
+		const struct drm_format_info *info;
+		struct drm_gem_object *objs = NULL;
+		u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
+		u32 afbc_superblock_width = 0, afbc_size = 0;
+		int bpp = 0;
+
+		if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
+						mode_cmd->modifier[0]) == false)
+			return ERR_PTR(-EINVAL);
 
-	if (mode_cmd->offsets[0] != 0) {
-		DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
-		return false;
-	}
+		if (mode_cmd->offsets[0] != 0) {
+			DRM_DEBUG_KMS("AFBC buffer plane offset should be 0\n");
+			return ERR_PTR(-EINVAL);
+		}
 
-	switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
-	case AFBC_SIZE_16X16:
-		if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
-			DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n");
-			return false;
+		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
+		case AFBC_SIZE_16X16:
+			if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
+				DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n");
+				return ERR_PTR(-EINVAL);
+			}
+			break;
+		default:
+			DRM_DEBUG_KMS("Unsupported AFBC block size\n");
+			return ERR_PTR(-EINVAL);
 		}
-		break;
-	default:
-		DRM_DEBUG_KMS("Unsupported AFBC block size\n");
-		return false;
-	}
 
-	return true;
-}
+		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
+		case AFBC_SIZE_16X16:
+			afbc_superblock_height = 16;
+			afbc_superblock_width = 16;
+			break;
+		default:
+			DRM_DEBUG_KMS("AFBC superblock size not supported\n");
+			return ERR_PTR(-EINVAL);
+		}
 
-static bool
-malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
-				    struct drm_file *file,
-				    const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	int n_superblocks = 0;
-	const struct drm_format_info *info;
-	struct drm_gem_object *objs = NULL;
-	u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
-	u32 afbc_superblock_width = 0, afbc_size = 0;
-	int bpp = 0;
-
-	switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
-	case AFBC_SIZE_16X16:
-		afbc_superblock_height = 16;
-		afbc_superblock_width = 16;
-		break;
-	default:
-		DRM_DEBUG_KMS("AFBC superblock size is not supported\n");
-		return false;
-	}
+		info = drm_get_format_info(dev, mode_cmd);
 
-	info = drm_get_format_info(dev, mode_cmd);
+		n_superblocks = (mode_cmd->width / afbc_superblock_width) *
+			(mode_cmd->height / afbc_superblock_height);
 
-	n_superblocks = (mode_cmd->width / afbc_superblock_width) *
-		(mode_cmd->height / afbc_superblock_height);
+		bpp = malidp_format_get_bpp(info->format);
 
-	bpp = malidp_format_get_bpp(info->format);
+		afbc_superblock_size =
+			(bpp * afbc_superblock_width * afbc_superblock_height)
+			/ BITS_PER_BYTE;
 
-	afbc_superblock_size = (bpp * afbc_superblock_width * afbc_superblock_height)
-				/ BITS_PER_BYTE;
+		afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE,
+				  AFBC_SUPERBLK_ALIGNMENT);
+		afbc_size += n_superblocks
+			* ALIGN(afbc_superblock_size, AFBC_SUPERBLK_ALIGNMENT);
 
-	afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE, AFBC_SUPERBLK_ALIGNMENT);
-	afbc_size += n_superblocks * ALIGN(afbc_superblock_size, AFBC_SUPERBLK_ALIGNMENT);
+		if ((mode_cmd->width * bpp) !=
+			(mode_cmd->pitches[0] * BITS_PER_BYTE)) {
+			DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n",
+				      (mode_cmd->pitches[0] * BITS_PER_BYTE),
+				      mode_cmd->width, bpp);
+			return ERR_PTR(-EINVAL);
+		}
 
-	if ((mode_cmd->width * bpp) != (mode_cmd->pitches[0] * BITS_PER_BYTE)) {
-		DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) "
-			      "should be same as width (=%u) * bpp (=%u)\n",
-			      (mode_cmd->pitches[0] * BITS_PER_BYTE),
-			      mode_cmd->width, bpp);
-		return false;
-	}
+		objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
+		if (!objs) {
+			DRM_DEBUG_KMS("Failed to lookup GEM object\n");
+			return ERR_PTR(-EINVAL);
+		}
 
-	objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
-	if (!objs) {
-		DRM_DEBUG_KMS("Failed to lookup GEM object\n");
-		return false;
-	}
+		if (objs->size < afbc_size) {
+			DRM_DEBUG_KMS("buffer size %zu/%u too small for AFBC\n",
+				      objs->size, afbc_size);
+			drm_gem_object_put_unlocked(objs);
+			return ERR_PTR(-EINVAL);
+		}
 
-	if (objs->size < afbc_size) {
-		DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n",
-			      objs->size, afbc_size);
 		drm_gem_object_put_unlocked(objs);
-		return false;
-	}
-
-	drm_gem_object_put_unlocked(objs);
-
-	return true;
-}
-
-static bool
-malidp_verify_afbc_framebuffer(struct drm_device *dev, struct drm_file *file,
-			       const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	if (malidp_verify_afbc_framebuffer_caps(dev, mode_cmd))
-		return malidp_verify_afbc_framebuffer_size(dev, file, mode_cmd);
-
-	return false;
-}
-
-static struct drm_framebuffer *
-malidp_fb_create(struct drm_device *dev, struct drm_file *file,
-		 const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	if (mode_cmd->modifier[0]) {
-		if (!malidp_verify_afbc_framebuffer(dev, file, mode_cmd))
-			return ERR_PTR(-EINVAL);
 	}
 
 	return drm_gem_fb_create(dev, file, mode_cmd);
-- 
2.17.1

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

[PATCHv7 4/6] drm/arm/malidp: Allocate an afbc-specific drm_framebuffer

[Andrzej Pietrasiewicz]

Prepare for using generic afbc helpers.

Use an existing helper which allows allocating a struct drm_framebuffer
in the driver.

afbc-specific checks should go after drm_gem_fb_init_with_funcs().

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/arm/malidp_drv.c | 50 +++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index cafbd81e4c1e..b9715b19af94 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -269,13 +269,36 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
 	.atomic_commit_tail = malidp_atomic_commit_tail,
 };
 
+static const struct drm_framebuffer_funcs malidp_fb_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+};
+
 static struct drm_framebuffer *
 malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 		 const struct drm_mode_fb_cmd2 *mode_cmd)
 {
+	struct drm_afbc_framebuffer *afbc_fb;
+	const struct drm_format_info *info;
+	int ret, i;
+
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL);
+	if (!afbc_fb)
+		return ERR_PTR(-ENOMEM);
+
+	ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd,
+					 &malidp_fb_funcs);
+	if (ret) {
+		kfree(afbc_fb);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	if (mode_cmd->modifier[0]) {
 		int n_superblocks = 0;
-		const struct drm_format_info *info;
 		struct drm_gem_object *objs = NULL;
 		u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
 		u32 afbc_superblock_width = 0, afbc_size = 0;
@@ -283,23 +306,23 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 
 		if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
 						mode_cmd->modifier[0]) == false)
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 
 		if (mode_cmd->offsets[0] != 0) {
 			DRM_DEBUG_KMS("AFBC buffer plane offset should be 0\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
 		case AFBC_SIZE_16X16:
 			if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
 				DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n");
-				return ERR_PTR(-EINVAL);
+				goto free_fb;
 			}
 			break;
 		default:
 			DRM_DEBUG_KMS("Unsupported AFBC block size\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
@@ -309,10 +332,9 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 			break;
 		default:
 			DRM_DEBUG_KMS("AFBC superblock size not supported\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
-		info = drm_get_format_info(dev, mode_cmd);
 
 		n_superblocks = (mode_cmd->width / afbc_superblock_width) *
 			(mode_cmd->height / afbc_superblock_height);
@@ -333,26 +355,32 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 			DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n",
 				      (mode_cmd->pitches[0] * BITS_PER_BYTE),
 				      mode_cmd->width, bpp);
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
 		if (!objs) {
 			DRM_DEBUG_KMS("Failed to lookup GEM object\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		if (objs->size < afbc_size) {
 			DRM_DEBUG_KMS("buffer size %zu/%u too small for AFBC\n",
 				      objs->size, afbc_size);
 			drm_gem_object_put_unlocked(objs);
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		drm_gem_object_put_unlocked(objs);
 	}
 
-	return drm_gem_fb_create(dev, file, mode_cmd);
+	return &afbc_fb->base;
+
+free_fb:
+	for (i = 0; i < info->num_planes; ++i)
+		drm_gem_object_put_unlocked(afbc_fb->base.obj[i]);
+	kfree(afbc_fb);
+	return ERR_PTR(-EINVAL);
 }
 
 static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
-- 
2.17.1

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

Re: [PATCHv7 4/6] drm/arm/malidp: Allocate an afbc-specific drm_framebuffer

[Emil Velikov]

On Wed, 11 Mar 2020 at 14:56, Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Prepare for using generic afbc helpers.
>
> Use an existing helper which allows allocating a struct drm_framebuffer
> in the driver.
>
> afbc-specific checks should go after drm_gem_fb_init_with_funcs().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 50 +++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index cafbd81e4c1e..b9715b19af94 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -269,13 +269,36 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
>         .atomic_commit_tail = malidp_atomic_commit_tail,
>  };
>
> +static const struct drm_framebuffer_funcs malidp_fb_funcs = {
> +       .destroy        = drm_gem_fb_destroy,
> +       .create_handle  = drm_gem_fb_create_handle,
> +};
> +
>  static struct drm_framebuffer *
>  malidp_fb_create(struct drm_device *dev, struct drm_file *file,
>                  const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> +       struct drm_afbc_framebuffer *afbc_fb;
> +       const struct drm_format_info *info;
> +       int ret, i;
> +
> +       info = drm_get_format_info(dev, mode_cmd);
> +       if (!info)
> +               return ERR_PTR(-ENOMEM);
> +
The underlying implementation in drm_get_format_info() will throw a
WARN_ON() if we return NULL.
Returning ENOMEM here is misleading and EINVAL sounds better IMHO.

Regardless, of the above 1-5 are:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

I don't know much about Rockchip HW to review 6/6.

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

[PATCHv7 5/6] drm/arm/malidp: Switch to afbc helpers

[Andrzej Pietrasiewicz]

Use available afbc helpers.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/arm/malidp_drv.c | 44 +++-----------------------------
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index b9715b19af94..bae532290c89 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -297,11 +297,7 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	if (mode_cmd->modifier[0]) {
-		int n_superblocks = 0;
-		struct drm_gem_object *objs = NULL;
-		u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
-		u32 afbc_superblock_width = 0, afbc_size = 0;
+	if (drm_is_afbc(mode_cmd->modifier[0])) {
 		int bpp = 0;
 
 		if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
@@ -325,31 +321,8 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 			goto free_fb;
 		}
 
-		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
-		case AFBC_SIZE_16X16:
-			afbc_superblock_height = 16;
-			afbc_superblock_width = 16;
-			break;
-		default:
-			DRM_DEBUG_KMS("AFBC superblock size not supported\n");
-			goto free_fb;
-		}
-
-
-		n_superblocks = (mode_cmd->width / afbc_superblock_width) *
-			(mode_cmd->height / afbc_superblock_height);
-
 		bpp = malidp_format_get_bpp(info->format);
 
-		afbc_superblock_size =
-			(bpp * afbc_superblock_width * afbc_superblock_height)
-			/ BITS_PER_BYTE;
-
-		afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE,
-				  AFBC_SUPERBLK_ALIGNMENT);
-		afbc_size += n_superblocks
-			* ALIGN(afbc_superblock_size, AFBC_SUPERBLK_ALIGNMENT);
-
 		if ((mode_cmd->width * bpp) !=
 			(mode_cmd->pitches[0] * BITS_PER_BYTE)) {
 			DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n",
@@ -358,20 +331,11 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 			goto free_fb;
 		}
 
-		objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
-		if (!objs) {
-			DRM_DEBUG_KMS("Failed to lookup GEM object\n");
-			goto free_fb;
-		}
+		/* eliminate when cpp is properly encoded in drm_format_info */
+		afbc_fb->bpp = bpp;
 
-		if (objs->size < afbc_size) {
-			DRM_DEBUG_KMS("buffer size %zu/%u too small for AFBC\n",
-				      objs->size, afbc_size);
-			drm_gem_object_put_unlocked(objs);
+		if (drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb))
 			goto free_fb;
-		}
-
-		drm_gem_object_put_unlocked(objs);
 	}
 
 	return &afbc_fb->base;
-- 
2.17.1

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

[PATCHv7 6/6] drm/rockchip: Add support for afbc

[Andrzej Pietrasiewicz]

This patch adds support for afbc handling. afbc is a compressed format
which reduces the necessary memory bandwidth.

Co-developed-by: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  43 +++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 137 +++++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  17 +++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |  83 +++++++++++-
 5 files changed, 276 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index c5b06048124e..e33c2dcd0d4b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -30,6 +30,7 @@ struct rockchip_crtc_state {
 	int output_mode;
 	int output_bpc;
 	int output_flags;
+	bool enable_afbc;
 };
 #define to_rockchip_crtc_state(s) \
 		container_of(s, struct rockchip_crtc_state, base)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 221e72e71432..9b13c784b347 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -57,8 +57,49 @@ static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers =
 	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
+static struct drm_framebuffer *
+rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
+		   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	struct drm_afbc_framebuffer *afbc_fb;
+	const struct drm_format_info *info;
+	int ret;
+
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL);
+	if (!afbc_fb)
+		return ERR_PTR(-ENOMEM);
+
+	ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd,
+					 &rockchip_drm_fb_funcs);
+	if (ret) {
+		kfree(afbc_fb);
+		return ERR_PTR(ret);
+	}
+
+	if (drm_is_afbc(mode_cmd->modifier[0])) {
+		int ret, i;
+
+		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);
+		if (ret) {
+			struct drm_gem_object **obj = afbc_fb->base.obj;
+
+			for (i = 0; i < info->num_planes; ++i)
+				drm_gem_object_put_unlocked(obj[i]);
+
+			kfree(afbc_fb);
+			return ERR_PTR(ret);
+		}
+	}
+
+	return &afbc_fb->base;
+}
+
 static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
-	.fb_create = drm_gem_fb_create_with_dirty,
+	.fb_create = rockchip_fb_create,
 	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index cecb2cc781f5..b87d22eb6ae1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -91,9 +91,22 @@
 #define VOP_WIN_TO_INDEX(vop_win) \
 	((vop_win) - (vop_win)->vop->win)
 
+#define VOP_AFBC_SET(vop, name, v) \
+	do { \
+		if ((vop)->data->afbc) \
+			vop_reg_set((vop), &(vop)->data->afbc->name, \
+				    0, ~0, v, #name); \
+	} while (0)
+
 #define to_vop(x) container_of(x, struct vop, crtc)
 #define to_vop_win(x) container_of(x, struct vop_win, base)
 
+#define AFBC_FMT_RGB565		0x0
+#define AFBC_FMT_U8U8U8U8	0x5
+#define AFBC_FMT_U8U8U8		0x4
+
+#define AFBC_TILE_16x16		BIT(4)
+
 /*
  * The coefficients of the following matrix are all fixed points.
  * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the offsets.
@@ -274,6 +287,29 @@ static enum vop_data_format vop_convert_format(uint32_t format)
 	}
 }
 
+static int vop_convert_afbc_format(uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		return AFBC_FMT_U8U8U8U8;
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_BGR888:
+		return AFBC_FMT_U8U8U8;
+	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_BGR565:
+		return AFBC_FMT_RGB565;
+	/* either of the below should not be reachable */
+	default:
+		DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format);
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
 static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
 				  uint32_t dst, bool is_horizontal,
 				  int vsu_mode, int *vskiplines)
@@ -598,6 +634,17 @@ static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state)
 			vop_win_disable(vop, vop_win);
 		}
 	}
+
+	if (vop->data->afbc) {
+		struct rockchip_crtc_state *s;
+		/*
+		 * Disable AFBC and forget there was a vop window with AFBC
+		 */
+		VOP_AFBC_SET(vop, enable, 0);
+		s = to_rockchip_crtc_state(crtc->state);
+		s->enable_afbc = false;
+	}
+
 	spin_unlock(&vop->reg_lock);
 
 	vop_cfg_done(vop);
@@ -710,6 +757,26 @@ static void vop_plane_destroy(struct drm_plane *plane)
 	drm_plane_cleanup(plane);
 }
 
+static inline bool rockchip_afbc(u64 modifier)
+{
+	return modifier == ROCKCHIP_AFBC_MOD;
+}
+
+static bool rockchip_mod_supported(struct drm_plane *plane,
+				   u32 format, u64 modifier)
+{
+	if (modifier == DRM_FORMAT_MOD_LINEAR)
+		return true;
+
+	if (!rockchip_afbc(modifier)) {
+		DRM_DEBUG_KMS("Unsupported format modifer 0x%llx\n", modifier);
+
+		return false;
+	}
+
+	return vop_convert_afbc_format(format) >= 0;
+}
+
 static int vop_plane_atomic_check(struct drm_plane *plane,
 			   struct drm_plane_state *state)
 {
@@ -758,6 +825,30 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	if (rockchip_afbc(fb->modifier)) {
+		struct vop *vop = to_vop(crtc);
+
+		if (!vop->data->afbc) {
+			DRM_ERROR("vop does not support AFBC\n");
+			return -EINVAL;
+		}
+
+		ret = vop_convert_afbc_format(fb->format->format);
+		if (ret < 0)
+			return ret;
+
+		if (state->src.x1 || state->src.y1) {
+			DRM_ERROR("AFBC does not support offset display, xpos=%d, ypos=%d, offset=%d\n", state->src.x1, state->src.y1, fb->offsets[0]);
+			return -EINVAL;
+		}
+
+		if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
+			DRM_ERROR("No rotation support in AFBC, rotation=%d\n",
+				  state->rotation);
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
@@ -846,6 +937,16 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
 
 	spin_lock(&vop->reg_lock);
 
+	if (rockchip_afbc(fb->modifier)) {
+		int afbc_format = vop_convert_afbc_format(fb->format->format);
+
+		VOP_AFBC_SET(vop, format, afbc_format | AFBC_TILE_16x16);
+		VOP_AFBC_SET(vop, hreg_block_split, 0);
+		VOP_AFBC_SET(vop, win_sel, VOP_WIN_TO_INDEX(vop_win));
+		VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
+		VOP_AFBC_SET(vop, pic_size, act_info);
+	}
+
 	VOP_WIN_SET(vop, win, format, format);
 	VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
 	VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
@@ -1001,6 +1102,7 @@ static const struct drm_plane_funcs vop_plane_funcs = {
 	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.format_mod_supported = rockchip_mod_supported,
 };
 
 static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
@@ -1310,6 +1412,10 @@ static int vop_crtc_atomic_check(struct drm_crtc *crtc,
 				 struct drm_crtc_state *crtc_state)
 {
 	struct vop *vop = to_vop(crtc);
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	struct rockchip_crtc_state *s;
+	int afbc_planes = 0;
 
 	if (vop->lut_regs && crtc_state->color_mgmt_changed &&
 	    crtc_state->gamma_lut) {
@@ -1323,6 +1429,27 @@ static int vop_crtc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	drm_atomic_crtc_state_for_each_plane(plane, crtc_state) {
+		plane_state =
+			drm_atomic_get_plane_state(crtc_state->state, plane);
+		if (IS_ERR(plane_state)) {
+			DRM_DEBUG_KMS("Cannot get plane state for plane %s\n",
+				      plane->name);
+			return PTR_ERR(plane_state);
+		}
+
+		if (drm_is_afbc(plane_state->fb->modifier))
+			++afbc_planes;
+	}
+
+	if (afbc_planes > 1) {
+		DRM_DEBUG_KMS("Invalid number of AFBC planes; got %d, expected at most 1\n", afbc_planes);
+		return -EINVAL;
+	}
+
+	s = to_rockchip_crtc_state(crtc_state);
+	s->enable_afbc = afbc_planes > 0;
+
 	return 0;
 }
 
@@ -1333,6 +1460,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
 	struct drm_plane_state *old_plane_state, *new_plane_state;
 	struct vop *vop = to_vop(crtc);
 	struct drm_plane *plane;
+	struct rockchip_crtc_state *s;
 	int i;
 
 	if (WARN_ON(!vop->is_enabled))
@@ -1340,6 +1468,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	spin_lock(&vop->reg_lock);
 
+	/* Enable AFBC if there is some AFBC window, disable otherwise. */
+	s = to_rockchip_crtc_state(crtc->state);
+	VOP_AFBC_SET(vop, enable, s->enable_afbc);
 	vop_cfg_done(vop);
 
 	spin_unlock(&vop->reg_lock);
@@ -1634,7 +1765,8 @@ static int vop_create_crtc(struct vop *vop)
 					       0, &vop_plane_funcs,
 					       win_data->phy->data_formats,
 					       win_data->phy->nformats,
-					       NULL, win_data->type, NULL);
+					       win_data->phy->format_modifiers,
+					       win_data->type, NULL);
 		if (ret) {
 			DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
 				      ret);
@@ -1678,7 +1810,8 @@ static int vop_create_crtc(struct vop *vop)
 					       &vop_plane_funcs,
 					       win_data->phy->data_formats,
 					       win_data->phy->nformats,
-					       NULL, win_data->type, NULL);
+					       win_data->phy->format_modifiers,
+					       win_data->type, NULL);
 		if (ret) {
 			DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
 				      ret);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index cc672620d6e0..d03bdb531ef2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -17,6 +17,11 @@
 
 #define NUM_YUV2YUV_COEFFICIENTS 12
 
+#define ROCKCHIP_AFBC_MOD \
+	DRM_FORMAT_MOD_ARM_AFBC( \
+		AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE \
+	)
+
 enum vop_data_format {
 	VOP_FMT_ARGB8888 = 0,
 	VOP_FMT_RGB888,
@@ -34,6 +39,16 @@ struct vop_reg {
 	bool relaxed;
 };
 
+struct vop_afbc {
+	struct vop_reg enable;
+	struct vop_reg win_sel;
+	struct vop_reg format;
+	struct vop_reg hreg_block_split;
+	struct vop_reg pic_size;
+	struct vop_reg hdr_ptr;
+	struct vop_reg rstn;
+};
+
 struct vop_modeset {
 	struct vop_reg htotal_pw;
 	struct vop_reg hact_st_end;
@@ -134,6 +149,7 @@ struct vop_win_phy {
 	const struct vop_scl_regs *scl;
 	const uint32_t *data_formats;
 	uint32_t nformats;
+	const uint64_t *format_modifiers;
 
 	struct vop_reg enable;
 	struct vop_reg gate;
@@ -173,6 +189,7 @@ struct vop_data {
 	const struct vop_misc *misc;
 	const struct vop_modeset *modeset;
 	const struct vop_output *output;
+	const struct vop_afbc *afbc;
 	const struct vop_win_yuv2yuv_data *win_yuv2yuv;
 	const struct vop_win_data *win;
 	unsigned int win_size;
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 7a9d979c8d5d..2413deded22c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -50,6 +50,17 @@ static const uint32_t formats_win_full[] = {
 	DRM_FORMAT_NV24,
 };
 
+static const uint64_t format_modifiers_win_full[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID,
+};
+
+static const uint64_t format_modifiers_win_full_afbc[] = {
+	ROCKCHIP_AFBC_MOD,
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID,
+};
+
 static const uint32_t formats_win_lite[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ARGB8888,
@@ -61,6 +72,11 @@ static const uint32_t formats_win_lite[] = {
 	DRM_FORMAT_BGR565,
 };
 
+static const uint64_t format_modifiers_win_lite[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID,
+};
+
 static const struct vop_scl_regs rk3036_win_scl = {
 	.scale_yrgb_x = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 0x0),
 	.scale_yrgb_y = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 16),
@@ -72,6 +88,7 @@ static const struct vop_win_phy rk3036_win0_data = {
 	.scl = &rk3036_win_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 0),
 	.format = VOP_REG(RK3036_SYS_CTRL, 0x7, 3),
 	.rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 15),
@@ -87,6 +104,7 @@ static const struct vop_win_phy rk3036_win0_data = {
 static const struct vop_win_phy rk3036_win1_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 1),
 	.format = VOP_REG(RK3036_SYS_CTRL, 0x7, 6),
 	.rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 19),
@@ -153,6 +171,7 @@ static const struct vop_data rk3036_vop = {
 static const struct vop_win_phy rk3126_win1_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 1),
 	.format = VOP_REG(RK3036_SYS_CTRL, 0x7, 6),
 	.rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 19),
@@ -234,6 +253,7 @@ static const struct vop_win_phy px30_win0_data = {
 	.scl = &px30_win_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(PX30_WIN0_CTRL0, 0x1, 0),
 	.format = VOP_REG(PX30_WIN0_CTRL0, 0x7, 1),
 	.rb_swap = VOP_REG(PX30_WIN0_CTRL0, 0x1, 12),
@@ -249,6 +269,7 @@ static const struct vop_win_phy px30_win0_data = {
 static const struct vop_win_phy px30_win1_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.enable = VOP_REG(PX30_WIN1_CTRL0, 0x1, 0),
 	.format = VOP_REG(PX30_WIN1_CTRL0, 0x7, 4),
 	.rb_swap = VOP_REG(PX30_WIN1_CTRL0, 0x1, 12),
@@ -261,6 +282,7 @@ static const struct vop_win_phy px30_win1_data = {
 static const struct vop_win_phy px30_win2_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.gate = VOP_REG(PX30_WIN2_CTRL0, 0x1, 4),
 	.enable = VOP_REG(PX30_WIN2_CTRL0, 0x1, 0),
 	.format = VOP_REG(PX30_WIN2_CTRL0, 0x3, 5),
@@ -316,6 +338,7 @@ static const struct vop_win_phy rk3066_win0_data = {
 	.scl = &rk3066_win_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 0),
 	.format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 4),
 	.rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 19),
@@ -332,6 +355,7 @@ static const struct vop_win_phy rk3066_win1_data = {
 	.scl = &rk3066_win_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 1),
 	.format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 7),
 	.rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 23),
@@ -347,6 +371,7 @@ static const struct vop_win_phy rk3066_win1_data = {
 static const struct vop_win_phy rk3066_win2_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 2),
 	.format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 10),
 	.rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 27),
@@ -426,6 +451,7 @@ static const struct vop_win_phy rk3188_win0_data = {
 	.scl = &rk3188_win_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(RK3188_SYS_CTRL, 0x1, 0),
 	.format = VOP_REG(RK3188_SYS_CTRL, 0x7, 3),
 	.rb_swap = VOP_REG(RK3188_SYS_CTRL, 0x1, 15),
@@ -440,6 +466,7 @@ static const struct vop_win_phy rk3188_win0_data = {
 static const struct vop_win_phy rk3188_win1_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.enable = VOP_REG(RK3188_SYS_CTRL, 0x1, 1),
 	.format = VOP_REG(RK3188_SYS_CTRL, 0x7, 6),
 	.rb_swap = VOP_REG(RK3188_SYS_CTRL, 0x1, 19),
@@ -545,6 +572,7 @@ static const struct vop_win_phy rk3288_win01_data = {
 	.scl = &rk3288_win_full_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
 	.rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
@@ -563,6 +591,7 @@ static const struct vop_win_phy rk3288_win01_data = {
 static const struct vop_win_phy rk3288_win23_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
 	.gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
@@ -677,6 +706,7 @@ static const struct vop_win_phy rk3368_win01_data = {
 	.scl = &rk3288_win_full_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full,
 	.enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
 	.rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
@@ -697,6 +727,7 @@ static const struct vop_win_phy rk3368_win01_data = {
 static const struct vop_win_phy rk3368_win23_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
+	.format_modifiers = format_modifiers_win_lite,
 	.gate = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 0),
 	.enable = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 4),
 	.format = VOP_REG(RK3368_WIN2_CTRL0, 0x3, 5),
@@ -817,6 +848,53 @@ static const struct vop_win_yuv2yuv_data rk3399_vop_big_win_yuv2yuv_data[] = {
 	  .y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 9) },
 	{ .base = 0xC0, .phy = &rk3399_yuv2yuv_win23_data },
 	{ .base = 0x120, .phy = &rk3399_yuv2yuv_win23_data },
+
+};
+
+static const struct vop_win_phy rk3399_win01_data = {
+	.scl = &rk3288_win_full_scl,
+	.data_formats = formats_win_full,
+	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_win_full_afbc,
+	.enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
+	.format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
+	.rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
+	.y_mir_en = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 22),
+	.act_info = VOP_REG(RK3288_WIN0_ACT_INFO, 0x1fff1fff, 0),
+	.dsp_info = VOP_REG(RK3288_WIN0_DSP_INFO, 0x0fff0fff, 0),
+	.dsp_st = VOP_REG(RK3288_WIN0_DSP_ST, 0x1fff1fff, 0),
+	.yrgb_mst = VOP_REG(RK3288_WIN0_YRGB_MST, 0xffffffff, 0),
+	.uv_mst = VOP_REG(RK3288_WIN0_CBR_MST, 0xffffffff, 0),
+	.yrgb_vir = VOP_REG(RK3288_WIN0_VIR, 0x3fff, 0),
+	.uv_vir = VOP_REG(RK3288_WIN0_VIR, 0x3fff, 16),
+	.src_alpha_ctl = VOP_REG(RK3288_WIN0_SRC_ALPHA_CTRL, 0xff, 0),
+	.dst_alpha_ctl = VOP_REG(RK3288_WIN0_DST_ALPHA_CTRL, 0xff, 0),
+};
+
+/*
+ * rk3399 vop big windows register layout is same as rk3288, but we
+ * have a separate rk3399 win data array here so that we can advertise
+ * AFBC on the primary plane.
+ */
+static const struct vop_win_data rk3399_vop_win_data[] = {
+	{ .base = 0x00, .phy = &rk3399_win01_data,
+	  .type = DRM_PLANE_TYPE_PRIMARY },
+	{ .base = 0x40, .phy = &rk3288_win01_data,
+	  .type = DRM_PLANE_TYPE_OVERLAY },
+	{ .base = 0x00, .phy = &rk3288_win23_data,
+	  .type = DRM_PLANE_TYPE_OVERLAY },
+	{ .base = 0x50, .phy = &rk3288_win23_data,
+	  .type = DRM_PLANE_TYPE_CURSOR },
+};
+
+static const struct vop_afbc rk3399_vop_afbc = {
+	.rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
+	.enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
+	.win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
+	.format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
+	.hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
+	.hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
+	.pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
 };
 
 static const struct vop_data rk3399_vop_big = {
@@ -826,9 +904,10 @@ static const struct vop_data rk3399_vop_big = {
 	.common = &rk3288_common,
 	.modeset = &rk3288_modeset,
 	.output = &rk3399_output,
+	.afbc = &rk3399_vop_afbc,
 	.misc = &rk3368_misc,
-	.win = rk3368_vop_win_data,
-	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+	.win = rk3399_vop_win_data,
+	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
 	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
 };
 
-- 
2.17.1

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

Re: [PATCHv7 6/6] drm/rockchip: Add support for afbc

[Emil Velikov]

On Wed, 11 Mar 2020 at 14:56, Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> This patch adds support for afbc handling. afbc is a compressed format
> which reduces the necessary memory bandwidth.
>
> Co-developed-by: Mark Yao <mark.yao@rock-chips.com>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  43 +++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 137 +++++++++++++++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  17 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |  83 +++++++++++-
>  5 files changed, 276 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index c5b06048124e..e33c2dcd0d4b 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -30,6 +30,7 @@ struct rockchip_crtc_state {
>         int output_mode;
>         int output_bpc;
>         int output_flags;
> +       bool enable_afbc;
>  };
>  #define to_rockchip_crtc_state(s) \
>                 container_of(s, struct rockchip_crtc_state, base)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 221e72e71432..9b13c784b347 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -57,8 +57,49 @@ static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers =
>         .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>  };
>
> +static struct drm_framebuffer *
> +rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
> +                  const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +       struct drm_afbc_framebuffer *afbc_fb;
> +       const struct drm_format_info *info;
> +       int ret;
> +
> +       info = drm_get_format_info(dev, mode_cmd);
> +       if (!info)
> +               return ERR_PTR(-ENOMEM);
> +
> +       afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL);
> +       if (!afbc_fb)
> +               return ERR_PTR(-ENOMEM);
> +
> +       ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd,
> +                                        &rockchip_drm_fb_funcs);
> +       if (ret) {
> +               kfree(afbc_fb);
> +               return ERR_PTR(ret);
> +       }
> +
> +       if (drm_is_afbc(mode_cmd->modifier[0])) {
> +               int ret, i;
> +
> +               ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);
> +               if (ret) {
> +                       struct drm_gem_object **obj = afbc_fb->base.obj;
> +
> +                       for (i = 0; i < info->num_planes; ++i)
> +                               drm_gem_object_put_unlocked(obj[i]);
> +
> +                       kfree(afbc_fb);
> +                       return ERR_PTR(ret);
> +               }
> +       }
> +
> +       return &afbc_fb->base;
> +}
> +
>  static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
> -       .fb_create = drm_gem_fb_create_with_dirty,
> +       .fb_create = rockchip_fb_create,
>         .output_poll_changed = drm_fb_helper_output_poll_changed,
>         .atomic_check = drm_atomic_helper_check,
>         .atomic_commit = drm_atomic_helper_commit,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index cecb2cc781f5..b87d22eb6ae1 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -91,9 +91,22 @@
>  #define VOP_WIN_TO_INDEX(vop_win) \
>         ((vop_win) - (vop_win)->vop->win)
>
> +#define VOP_AFBC_SET(vop, name, v) \
> +       do { \
> +               if ((vop)->data->afbc) \
> +                       vop_reg_set((vop), &(vop)->data->afbc->name, \
> +                                   0, ~0, v, #name); \
> +       } while (0)
> +
>  #define to_vop(x) container_of(x, struct vop, crtc)
>  #define to_vop_win(x) container_of(x, struct vop_win, base)
>
> +#define AFBC_FMT_RGB565                0x0
> +#define AFBC_FMT_U8U8U8U8      0x5
> +#define AFBC_FMT_U8U8U8                0x4
> +
> +#define AFBC_TILE_16x16                BIT(4)
> +
>  /*
>   * The coefficients of the following matrix are all fixed points.
>   * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the offsets.
> @@ -274,6 +287,29 @@ static enum vop_data_format vop_convert_format(uint32_t format)
>         }
>  }
>
> +static int vop_convert_afbc_format(uint32_t format)
> +{
> +       switch (format) {
> +       case DRM_FORMAT_XRGB8888:
> +       case DRM_FORMAT_ARGB8888:
> +       case DRM_FORMAT_XBGR8888:
> +       case DRM_FORMAT_ABGR8888:
> +               return AFBC_FMT_U8U8U8U8;
> +       case DRM_FORMAT_RGB888:
> +       case DRM_FORMAT_BGR888:
> +               return AFBC_FMT_U8U8U8;
> +       case DRM_FORMAT_RGB565:
> +       case DRM_FORMAT_BGR565:
> +               return AFBC_FMT_RGB565;
> +       /* either of the below should not be reachable */
> +       default:
> +               DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format);
> +               return -EINVAL;
> +       }
> +
> +       return -EINVAL;
> +}
> +
>  static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
>                                   uint32_t dst, bool is_horizontal,
>                                   int vsu_mode, int *vskiplines)
> @@ -598,6 +634,17 @@ static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state)
>                         vop_win_disable(vop, vop_win);
>                 }
>         }
> +
> +       if (vop->data->afbc) {
> +               struct rockchip_crtc_state *s;
> +               /*
> +                * Disable AFBC and forget there was a vop window with AFBC
> +                */
> +               VOP_AFBC_SET(vop, enable, 0);
> +               s = to_rockchip_crtc_state(crtc->state);
> +               s->enable_afbc = false;
> +       }
> +
>         spin_unlock(&vop->reg_lock);
>
>         vop_cfg_done(vop);
> @@ -710,6 +757,26 @@ static void vop_plane_destroy(struct drm_plane *plane)
>         drm_plane_cleanup(plane);
>  }
>
> +static inline bool rockchip_afbc(u64 modifier)
> +{
> +       return modifier == ROCKCHIP_AFBC_MOD;
> +}
> +
> +static bool rockchip_mod_supported(struct drm_plane *plane,
> +                                  u32 format, u64 modifier)
> +{
> +       if (modifier == DRM_FORMAT_MOD_LINEAR)
> +               return true;
> +
> +       if (!rockchip_afbc(modifier)) {
> +               DRM_DEBUG_KMS("Unsupported format modifer 0x%llx\n", modifier);
> +
> +               return false;
> +       }
> +
> +       return vop_convert_afbc_format(format) >= 0;
> +}
> +
>  static int vop_plane_atomic_check(struct drm_plane *plane,
>                            struct drm_plane_state *state)
>  {
> @@ -758,6 +825,30 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
>                 return -EINVAL;
>         }
>
> +       if (rockchip_afbc(fb->modifier)) {
> +               struct vop *vop = to_vop(crtc);
> +
> +               if (!vop->data->afbc) {
> +                       DRM_ERROR("vop does not support AFBC\n");
> +                       return -EINVAL;
> +               }
> +
> +               ret = vop_convert_afbc_format(fb->format->format);
> +               if (ret < 0)
> +                       return ret;
> +
> +               if (state->src.x1 || state->src.y1) {
> +                       DRM_ERROR("AFBC does not support offset display, xpos=%d, ypos=%d, offset=%d\n", state->src.x1, state->src.y1, fb->offsets[0]);
> +                       return -EINVAL;
> +               }
> +
> +               if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
> +                       DRM_ERROR("No rotation support in AFBC, rotation=%d\n",
> +                                 state->rotation);
> +                       return -EINVAL;
> +               }
> +       }
> +
>         return 0;
>  }
>
> @@ -846,6 +937,16 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>
>         spin_lock(&vop->reg_lock);
>
> +       if (rockchip_afbc(fb->modifier)) {
> +               int afbc_format = vop_convert_afbc_format(fb->format->format);
> +
> +               VOP_AFBC_SET(vop, format, afbc_format | AFBC_TILE_16x16);
> +               VOP_AFBC_SET(vop, hreg_block_split, 0);
> +               VOP_AFBC_SET(vop, win_sel, VOP_WIN_TO_INDEX(vop_win));
> +               VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
> +               VOP_AFBC_SET(vop, pic_size, act_info);
> +       }
> +
>         VOP_WIN_SET(vop, win, format, format);
>         VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
>         VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
> @@ -1001,6 +1102,7 @@ static const struct drm_plane_funcs vop_plane_funcs = {
>         .reset = drm_atomic_helper_plane_reset,
>         .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>         .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +       .format_mod_supported = rockchip_mod_supported,
>  };
>
>  static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
> @@ -1310,6 +1412,10 @@ static int vop_crtc_atomic_check(struct drm_crtc *crtc,
>                                  struct drm_crtc_state *crtc_state)
>  {
>         struct vop *vop = to_vop(crtc);
> +       struct drm_plane *plane;
> +       struct drm_plane_state *plane_state;
> +       struct rockchip_crtc_state *s;
> +       int afbc_planes = 0;
>
>         if (vop->lut_regs && crtc_state->color_mgmt_changed &&
>             crtc_state->gamma_lut) {
> @@ -1323,6 +1429,27 @@ static int vop_crtc_atomic_check(struct drm_crtc *crtc,
>                 }
>         }
>
> +       drm_atomic_crtc_state_for_each_plane(plane, crtc_state) {
> +               plane_state =
> +                       drm_atomic_get_plane_state(crtc_state->state, plane);
> +               if (IS_ERR(plane_state)) {
> +                       DRM_DEBUG_KMS("Cannot get plane state for plane %s\n",
> +                                     plane->name);
> +                       return PTR_ERR(plane_state);
> +               }
> +
> +               if (drm_is_afbc(plane_state->fb->modifier))
> +                       ++afbc_planes;
> +       }
> +
> +       if (afbc_planes > 1) {
> +               DRM_DEBUG_KMS("Invalid number of AFBC planes; got %d, expected at most 1\n", afbc_planes);
> +               return -EINVAL;
> +       }
> +
> +       s = to_rockchip_crtc_state(crtc_state);
> +       s->enable_afbc = afbc_planes > 0;
> +
>         return 0;
>  }
>
> @@ -1333,6 +1460,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>         struct drm_plane_state *old_plane_state, *new_plane_state;
>         struct vop *vop = to_vop(crtc);
>         struct drm_plane *plane;
> +       struct rockchip_crtc_state *s;
>         int i;
>
>         if (WARN_ON(!vop->is_enabled))
> @@ -1340,6 +1468,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>
>         spin_lock(&vop->reg_lock);
>
> +       /* Enable AFBC if there is some AFBC window, disable otherwise. */
> +       s = to_rockchip_crtc_state(crtc->state);
> +       VOP_AFBC_SET(vop, enable, s->enable_afbc);
>         vop_cfg_done(vop);
>
>         spin_unlock(&vop->reg_lock);
> @@ -1634,7 +1765,8 @@ static int vop_create_crtc(struct vop *vop)
>                                                0, &vop_plane_funcs,
>                                                win_data->phy->data_formats,
>                                                win_data->phy->nformats,
> -                                              NULL, win_data->type, NULL);
> +                                              win_data->phy->format_modifiers,
> +                                              win_data->type, NULL);
>                 if (ret) {
>                         DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
>                                       ret);
> @@ -1678,7 +1810,8 @@ static int vop_create_crtc(struct vop *vop)
>                                                &vop_plane_funcs,
>                                                win_data->phy->data_formats,
>                                                win_data->phy->nformats,
> -                                              NULL, win_data->type, NULL);
> +                                              win_data->phy->format_modifiers,
> +                                              win_data->type, NULL);
>                 if (ret) {
>                         DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
>                                       ret);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index cc672620d6e0..d03bdb531ef2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -17,6 +17,11 @@
>
>  #define NUM_YUV2YUV_COEFFICIENTS 12
>
> +#define ROCKCHIP_AFBC_MOD \
> +       DRM_FORMAT_MOD_ARM_AFBC( \
> +               AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE \
> +       )
> +
>  enum vop_data_format {
>         VOP_FMT_ARGB8888 = 0,
>         VOP_FMT_RGB888,
> @@ -34,6 +39,16 @@ struct vop_reg {
>         bool relaxed;
>  };
>
> +struct vop_afbc {
> +       struct vop_reg enable;
> +       struct vop_reg win_sel;
> +       struct vop_reg format;
> +       struct vop_reg hreg_block_split;
> +       struct vop_reg pic_size;
> +       struct vop_reg hdr_ptr;
> +       struct vop_reg rstn;
> +};
> +
>  struct vop_modeset {
>         struct vop_reg htotal_pw;
>         struct vop_reg hact_st_end;
> @@ -134,6 +149,7 @@ struct vop_win_phy {
>         const struct vop_scl_regs *scl;
>         const uint32_t *data_formats;
>         uint32_t nformats;
> +       const uint64_t *format_modifiers;
>
>         struct vop_reg enable;
>         struct vop_reg gate;
> @@ -173,6 +189,7 @@ struct vop_data {
>         const struct vop_misc *misc;
>         const struct vop_modeset *modeset;
>         const struct vop_output *output;
> +       const struct vop_afbc *afbc;
>         const struct vop_win_yuv2yuv_data *win_yuv2yuv;
>         const struct vop_win_data *win;
>         unsigned int win_size;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 7a9d979c8d5d..2413deded22c 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -50,6 +50,17 @@ static const uint32_t formats_win_full[] = {
>         DRM_FORMAT_NV24,
>  };
>
> +static const uint64_t format_modifiers_win_full[] = {
> +       DRM_FORMAT_MOD_LINEAR,
> +       DRM_FORMAT_MOD_INVALID,
> +};
> +
> +static const uint64_t format_modifiers_win_full_afbc[] = {
> +       ROCKCHIP_AFBC_MOD,
> +       DRM_FORMAT_MOD_LINEAR,
> +       DRM_FORMAT_MOD_INVALID,
> +};
> +
>  static const uint32_t formats_win_lite[] = {
>         DRM_FORMAT_XRGB8888,
>         DRM_FORMAT_ARGB8888,
> @@ -61,6 +72,11 @@ static const uint32_t formats_win_lite[] = {
>         DRM_FORMAT_BGR565,
>  };
>
> +static const uint64_t format_modifiers_win_lite[] = {
> +       DRM_FORMAT_MOD_LINEAR,
> +       DRM_FORMAT_MOD_INVALID,
> +};
> +
>  static const struct vop_scl_regs rk3036_win_scl = {
>         .scale_yrgb_x = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 0x0),
>         .scale_yrgb_y = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 16),
> @@ -72,6 +88,7 @@ static const struct vop_win_phy rk3036_win0_data = {
>         .scl = &rk3036_win_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 0),
>         .format = VOP_REG(RK3036_SYS_CTRL, 0x7, 3),
>         .rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 15),
> @@ -87,6 +104,7 @@ static const struct vop_win_phy rk3036_win0_data = {
>  static const struct vop_win_phy rk3036_win1_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 1),
>         .format = VOP_REG(RK3036_SYS_CTRL, 0x7, 6),
>         .rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 19),
> @@ -153,6 +171,7 @@ static const struct vop_data rk3036_vop = {
>  static const struct vop_win_phy rk3126_win1_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 1),
>         .format = VOP_REG(RK3036_SYS_CTRL, 0x7, 6),
>         .rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 19),
> @@ -234,6 +253,7 @@ static const struct vop_win_phy px30_win0_data = {
>         .scl = &px30_win_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(PX30_WIN0_CTRL0, 0x1, 0),
>         .format = VOP_REG(PX30_WIN0_CTRL0, 0x7, 1),
>         .rb_swap = VOP_REG(PX30_WIN0_CTRL0, 0x1, 12),
> @@ -249,6 +269,7 @@ static const struct vop_win_phy px30_win0_data = {
>  static const struct vop_win_phy px30_win1_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .enable = VOP_REG(PX30_WIN1_CTRL0, 0x1, 0),
>         .format = VOP_REG(PX30_WIN1_CTRL0, 0x7, 4),
>         .rb_swap = VOP_REG(PX30_WIN1_CTRL0, 0x1, 12),
> @@ -261,6 +282,7 @@ static const struct vop_win_phy px30_win1_data = {
>  static const struct vop_win_phy px30_win2_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .gate = VOP_REG(PX30_WIN2_CTRL0, 0x1, 4),
>         .enable = VOP_REG(PX30_WIN2_CTRL0, 0x1, 0),
>         .format = VOP_REG(PX30_WIN2_CTRL0, 0x3, 5),
> @@ -316,6 +338,7 @@ static const struct vop_win_phy rk3066_win0_data = {
>         .scl = &rk3066_win_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 0),
>         .format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 4),
>         .rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 19),
> @@ -332,6 +355,7 @@ static const struct vop_win_phy rk3066_win1_data = {
>         .scl = &rk3066_win_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 1),
>         .format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 7),
>         .rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 23),
> @@ -347,6 +371,7 @@ static const struct vop_win_phy rk3066_win1_data = {
>  static const struct vop_win_phy rk3066_win2_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 2),
>         .format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 10),
>         .rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 27),
> @@ -426,6 +451,7 @@ static const struct vop_win_phy rk3188_win0_data = {
>         .scl = &rk3188_win_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(RK3188_SYS_CTRL, 0x1, 0),
>         .format = VOP_REG(RK3188_SYS_CTRL, 0x7, 3),
>         .rb_swap = VOP_REG(RK3188_SYS_CTRL, 0x1, 15),
> @@ -440,6 +466,7 @@ static const struct vop_win_phy rk3188_win0_data = {
>  static const struct vop_win_phy rk3188_win1_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .enable = VOP_REG(RK3188_SYS_CTRL, 0x1, 1),
>         .format = VOP_REG(RK3188_SYS_CTRL, 0x7, 6),
>         .rb_swap = VOP_REG(RK3188_SYS_CTRL, 0x1, 19),
> @@ -545,6 +572,7 @@ static const struct vop_win_phy rk3288_win01_data = {
>         .scl = &rk3288_win_full_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
>         .format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
>         .rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
> @@ -563,6 +591,7 @@ static const struct vop_win_phy rk3288_win01_data = {
>  static const struct vop_win_phy rk3288_win23_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
>         .gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>         .format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
> @@ -677,6 +706,7 @@ static const struct vop_win_phy rk3368_win01_data = {
>         .scl = &rk3288_win_full_scl,
>         .data_formats = formats_win_full,
>         .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full,
>         .enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
>         .format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
>         .rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
> @@ -697,6 +727,7 @@ static const struct vop_win_phy rk3368_win01_data = {
>  static const struct vop_win_phy rk3368_win23_data = {
>         .data_formats = formats_win_lite,
>         .nformats = ARRAY_SIZE(formats_win_lite),
> +       .format_modifiers = format_modifiers_win_lite,
>         .gate = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 0),
>         .enable = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 4),
>         .format = VOP_REG(RK3368_WIN2_CTRL0, 0x3, 5),
> @@ -817,6 +848,53 @@ static const struct vop_win_yuv2yuv_data rk3399_vop_big_win_yuv2yuv_data[] = {
>           .y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 9) },
>         { .base = 0xC0, .phy = &rk3399_yuv2yuv_win23_data },
>         { .base = 0x120, .phy = &rk3399_yuv2yuv_win23_data },
> +
> +};
> +
> +static const struct vop_win_phy rk3399_win01_data = {
> +       .scl = &rk3288_win_full_scl,
> +       .data_formats = formats_win_full,
> +       .nformats = ARRAY_SIZE(formats_win_full),
> +       .format_modifiers = format_modifiers_win_full_afbc,
> +       .enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
> +       .format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
> +       .rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
> +       .y_mir_en = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 22),
> +       .act_info = VOP_REG(RK3288_WIN0_ACT_INFO, 0x1fff1fff, 0),
> +       .dsp_info = VOP_REG(RK3288_WIN0_DSP_INFO, 0x0fff0fff, 0),
> +       .dsp_st = VOP_REG(RK3288_WIN0_DSP_ST, 0x1fff1fff, 0),
> +       .yrgb_mst = VOP_REG(RK3288_WIN0_YRGB_MST, 0xffffffff, 0),
> +       .uv_mst = VOP_REG(RK3288_WIN0_CBR_MST, 0xffffffff, 0),
> +       .yrgb_vir = VOP_REG(RK3288_WIN0_VIR, 0x3fff, 0),
> +       .uv_vir = VOP_REG(RK3288_WIN0_VIR, 0x3fff, 16),
> +       .src_alpha_ctl = VOP_REG(RK3288_WIN0_SRC_ALPHA_CTRL, 0xff, 0),
> +       .dst_alpha_ctl = VOP_REG(RK3288_WIN0_DST_ALPHA_CTRL, 0xff, 0),
> +};
> +
> +/*
> + * rk3399 vop big windows register layout is same as rk3288, but we
> + * have a separate rk3399 win data array here so that we can advertise
> + * AFBC on the primary plane.
> + */
> +static const struct vop_win_data rk3399_vop_win_data[] = {
> +       { .base = 0x00, .phy = &rk3399_win01_data,
> +         .type = DRM_PLANE_TYPE_PRIMARY },
> +       { .base = 0x40, .phy = &rk3288_win01_data,
> +         .type = DRM_PLANE_TYPE_OVERLAY },
> +       { .base = 0x00, .phy = &rk3288_win23_data,
> +         .type = DRM_PLANE_TYPE_OVERLAY },
> +       { .base = 0x50, .phy = &rk3288_win23_data,
> +         .type = DRM_PLANE_TYPE_CURSOR },
> +};
> +
> +static const struct vop_afbc rk3399_vop_afbc = {
> +       .rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
> +       .enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
> +       .win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
> +       .format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
> +       .hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
> +       .hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
> +       .pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
>  };
>
>  static const struct vop_data rk3399_vop_big = {
> @@ -826,9 +904,10 @@ static const struct vop_data rk3399_vop_big = {
>         .common = &rk3288_common,
>         .modeset = &rk3288_modeset,
>         .output = &rk3399_output,
> +       .afbc = &rk3399_vop_afbc,
>         .misc = &rk3368_misc,
> -       .win = rk3368_vop_win_data,
> -       .win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +       .win = rk3399_vop_win_data,
> +       .win_size = ARRAY_SIZE(rk3399_vop_win_data),
>         .win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>  };
>
> --
> 2.17.1

Heiko, Sandy, being the maintainers of the Rockchip driver, can you
review/ack this patch?

I believe the intention is to merge the series via drm-misc. Andrzej
already has commit access.

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

Re: [PATCHv7 6/6] drm/rockchip: Add support for afbc

[Sandy Huang]

Hi Emil,

在 2020/3/16 下午10:10, Emil Velikov 写道:
> On Wed, 11 Mar 2020 at 14:56, Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
>> This patch adds support for afbc handling. afbc is a compressed format
>> which reduces the necessary memory bandwidth.
>>
>> Co-developed-by: Mark Yao <mark.yao@rock-chips.com>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  43 +++++-
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 137 +++++++++++++++++++-
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  17 +++
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c |  83 +++++++++++-
>>   5 files changed, 276 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index c5b06048124e..e33c2dcd0d4b 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -30,6 +30,7 @@ struct rockchip_crtc_state {
>>          int output_mode;
>>          int output_bpc;
>>          int output_flags;
>> +       bool enable_afbc;
>>   };
>>   #define to_rockchip_crtc_state(s) \
>>                  container_of(s, struct rockchip_crtc_state, base)
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> index 221e72e71432..9b13c784b347 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> @@ -57,8 +57,49 @@ static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers =
>>          .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>>   };
>>
>> +static struct drm_framebuffer *
>> +rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
>> +                  const struct drm_mode_fb_cmd2 *mode_cmd)
>> +{
>> +       struct drm_afbc_framebuffer *afbc_fb;
>> +       const struct drm_format_info *info;
>> +       int ret;
>> +
>> +       info = drm_get_format_info(dev, mode_cmd);
>> +       if (!info)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL);
>> +       if (!afbc_fb)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd,
>> +                                        &rockchip_drm_fb_funcs);
>> +       if (ret) {
>> +               kfree(afbc_fb);
>> +               return ERR_PTR(ret);
>> +       }
>> +
>> +       if (drm_is_afbc(mode_cmd->modifier[0])) {
>> +               int ret, i;
>> +
>> +               ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);
>> +               if (ret) {
>> +                       struct drm_gem_object **obj = afbc_fb->base.obj;
>> +
>> +                       for (i = 0; i < info->num_planes; ++i)
>> +                               drm_gem_object_put_unlocked(obj[i]);
>> +
>> +                       kfree(afbc_fb);
>> +                       return ERR_PTR(ret);
>> +               }
>> +       }
>> +
>> +       return &afbc_fb->base;
>> +}
>> +
>>   static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
>> -       .fb_create = drm_gem_fb_create_with_dirty,
>> +       .fb_create = rockchip_fb_create,
>>          .output_poll_changed = drm_fb_helper_output_poll_changed,
>>          .atomic_check = drm_atomic_helper_check,
>>          .atomic_commit = drm_atomic_helper_commit,
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index cecb2cc781f5..b87d22eb6ae1 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -91,9 +91,22 @@
>>   #define VOP_WIN_TO_INDEX(vop_win) \
>>          ((vop_win) - (vop_win)->vop->win)
>>
>> +#define VOP_AFBC_SET(vop, name, v) \
>> +       do { \
>> +               if ((vop)->data->afbc) \
>> +                       vop_reg_set((vop), &(vop)->data->afbc->name, \
>> +                                   0, ~0, v, #name); \
>> +       } while (0)
>> +
>>   #define to_vop(x) container_of(x, struct vop, crtc)
>>   #define to_vop_win(x) container_of(x, struct vop_win, base)
>>
>> +#define AFBC_FMT_RGB565                0x0
>> +#define AFBC_FMT_U8U8U8U8      0x5
>> +#define AFBC_FMT_U8U8U8                0x4
>> +
>> +#define AFBC_TILE_16x16                BIT(4)
>> +
>>   /*
>>    * The coefficients of the following matrix are all fixed points.
>>    * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the offsets.
>> @@ -274,6 +287,29 @@ static enum vop_data_format vop_convert_format(uint32_t format)
>>          }
>>   }
>>
>> +static int vop_convert_afbc_format(uint32_t format)
>> +{
>> +       switch (format) {
>> +       case DRM_FORMAT_XRGB8888:
>> +       case DRM_FORMAT_ARGB8888:
>> +       case DRM_FORMAT_XBGR8888:
>> +       case DRM_FORMAT_ABGR8888:
>> +               return AFBC_FMT_U8U8U8U8;
>> +       case DRM_FORMAT_RGB888:
>> +       case DRM_FORMAT_BGR888:
>> +               return AFBC_FMT_U8U8U8;
>> +       case DRM_FORMAT_RGB565:
>> +       case DRM_FORMAT_BGR565:
>> +               return AFBC_FMT_RGB565;
>> +       /* either of the below should not be reachable */
>> +       default:
>> +               DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return -EINVAL;
>> +}
>> +
>>   static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
>>                                    uint32_t dst, bool is_horizontal,
>>                                    int vsu_mode, int *vskiplines)
>> @@ -598,6 +634,17 @@ static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state)
>>                          vop_win_disable(vop, vop_win);
>>                  }
>>          }
>> +
>> +       if (vop->data->afbc) {
>> +               struct rockchip_crtc_state *s;
>> +               /*
>> +                * Disable AFBC and forget there was a vop window with AFBC
>> +                */
>> +               VOP_AFBC_SET(vop, enable, 0);
>> +               s = to_rockchip_crtc_state(crtc->state);
>> +               s->enable_afbc = false;
>> +       }
>> +
>>          spin_unlock(&vop->reg_lock);
>>
>>          vop_cfg_done(vop);
>> @@ -710,6 +757,26 @@ static void vop_plane_destroy(struct drm_plane *plane)
>>          drm_plane_cleanup(plane);
>>   }
>>
>> +static inline bool rockchip_afbc(u64 modifier)
>> +{
>> +       return modifier == ROCKCHIP_AFBC_MOD;
>> +}
>> +
>> +static bool rockchip_mod_supported(struct drm_plane *plane,
>> +                                  u32 format, u64 modifier)
>> +{
>> +       if (modifier == DRM_FORMAT_MOD_LINEAR)
>> +               return true;
>> +
>> +       if (!rockchip_afbc(modifier)) {
>> +               DRM_DEBUG_KMS("Unsupported format modifer 0x%llx\n", modifier);
>> +
>> +               return false;
>> +       }
>> +
>> +       return vop_convert_afbc_format(format) >= 0;
>> +}
>> +
>>   static int vop_plane_atomic_check(struct drm_plane *plane,
>>                             struct drm_plane_state *state)
>>   {
>> @@ -758,6 +825,30 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
>>                  return -EINVAL;
>>          }
>>
>> +       if (rockchip_afbc(fb->modifier)) {
>> +               struct vop *vop = to_vop(crtc);
>> +
>> +               if (!vop->data->afbc) {
>> +                       DRM_ERROR("vop does not support AFBC\n");
>> +                       return -EINVAL;
>> +               }
>> +
>> +               ret = vop_convert_afbc_format(fb->format->format);
>> +               if (ret < 0)
>> +                       return ret;
>> +
>> +               if (state->src.x1 || state->src.y1) {
>> +                       DRM_ERROR("AFBC does not support offset display, xpos=%d, ypos=%d, offset=%d\n", state->src.x1, state->src.y1, fb->offsets[0]);
>> +                       return -EINVAL;
>> +               }
>> +
>> +               if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
>> +                       DRM_ERROR("No rotation support in AFBC, rotation=%d\n",
>> +                                 state->rotation);
>> +                       return -EINVAL;
>> +               }
>> +       }
>> +
>>          return 0;
>>   }
>>
>> @@ -846,6 +937,16 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>>
>>          spin_lock(&vop->reg_lock);
>>
>> +       if (rockchip_afbc(fb->modifier)) {
>> +               int afbc_format = vop_convert_afbc_format(fb->format->format);
>> +
>> +               VOP_AFBC_SET(vop, format, afbc_format | AFBC_TILE_16x16);
>> +               VOP_AFBC_SET(vop, hreg_block_split, 0);
>> +               VOP_AFBC_SET(vop, win_sel, VOP_WIN_TO_INDEX(vop_win));
>> +               VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
>> +               VOP_AFBC_SET(vop, pic_size, act_info);
>> +       }
>> +
>>          VOP_WIN_SET(vop, win, format, format);
>>          VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
>>          VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
>> @@ -1001,6 +1102,7 @@ static const struct drm_plane_funcs vop_plane_funcs = {
>>          .reset = drm_atomic_helper_plane_reset,
>>          .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>>          .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>> +       .format_mod_supported = rockchip_mod_supported,
>>   };
>>
>>   static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>> @@ -1310,6 +1412,10 @@ static int vop_crtc_atomic_check(struct drm_crtc *crtc,
>>                                   struct drm_crtc_state *crtc_state)
>>   {
>>          struct vop *vop = to_vop(crtc);
>> +       struct drm_plane *plane;
>> +       struct drm_plane_state *plane_state;
>> +       struct rockchip_crtc_state *s;
>> +       int afbc_planes = 0;
>>
>>          if (vop->lut_regs && crtc_state->color_mgmt_changed &&
>>              crtc_state->gamma_lut) {
>> @@ -1323,6 +1429,27 @@ static int vop_crtc_atomic_check(struct drm_crtc *crtc,
>>                  }
>>          }
>>
>> +       drm_atomic_crtc_state_for_each_plane(plane, crtc_state) {
>> +               plane_state =
>> +                       drm_atomic_get_plane_state(crtc_state->state, plane);
>> +               if (IS_ERR(plane_state)) {
>> +                       DRM_DEBUG_KMS("Cannot get plane state for plane %s\n",
>> +                                     plane->name);
>> +                       return PTR_ERR(plane_state);
>> +               }
>> +
>> +               if (drm_is_afbc(plane_state->fb->modifier))
>> +                       ++afbc_planes;
>> +       }
>> +
>> +       if (afbc_planes > 1) {
>> +               DRM_DEBUG_KMS("Invalid number of AFBC planes; got %d, expected at most 1\n", afbc_planes);
>> +               return -EINVAL;
>> +       }
>> +
>> +       s = to_rockchip_crtc_state(crtc_state);
>> +       s->enable_afbc = afbc_planes > 0;
>> +
>>          return 0;
>>   }
>>
>> @@ -1333,6 +1460,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>>          struct drm_plane_state *old_plane_state, *new_plane_state;
>>          struct vop *vop = to_vop(crtc);
>>          struct drm_plane *plane;
>> +       struct rockchip_crtc_state *s;
>>          int i;
>>
>>          if (WARN_ON(!vop->is_enabled))
>> @@ -1340,6 +1468,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>>
>>          spin_lock(&vop->reg_lock);
>>
>> +       /* Enable AFBC if there is some AFBC window, disable otherwise. */
>> +       s = to_rockchip_crtc_state(crtc->state);
>> +       VOP_AFBC_SET(vop, enable, s->enable_afbc);
>>          vop_cfg_done(vop);
>>
>>          spin_unlock(&vop->reg_lock);
>> @@ -1634,7 +1765,8 @@ static int vop_create_crtc(struct vop *vop)
>>                                                 0, &vop_plane_funcs,
>>                                                 win_data->phy->data_formats,
>>                                                 win_data->phy->nformats,
>> -                                              NULL, win_data->type, NULL);
>> +                                              win_data->phy->format_modifiers,
>> +                                              win_data->type, NULL);
>>                  if (ret) {
>>                          DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
>>                                        ret);
>> @@ -1678,7 +1810,8 @@ static int vop_create_crtc(struct vop *vop)
>>                                                 &vop_plane_funcs,
>>                                                 win_data->phy->data_formats,
>>                                                 win_data->phy->nformats,
>> -                                              NULL, win_data->type, NULL);
>> +                                              win_data->phy->format_modifiers,
>> +                                              win_data->type, NULL);
>>                  if (ret) {
>>                          DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
>>                                        ret);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> index cc672620d6e0..d03bdb531ef2 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> @@ -17,6 +17,11 @@
>>
>>   #define NUM_YUV2YUV_COEFFICIENTS 12
>>
>> +#define ROCKCHIP_AFBC_MOD \
>> +       DRM_FORMAT_MOD_ARM_AFBC( \
>> +               AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE \
>> +       )
>> +
>>   enum vop_data_format {
>>          VOP_FMT_ARGB8888 = 0,
>>          VOP_FMT_RGB888,
>> @@ -34,6 +39,16 @@ struct vop_reg {
>>          bool relaxed;
>>   };
>>
>> +struct vop_afbc {
>> +       struct vop_reg enable;
>> +       struct vop_reg win_sel;
>> +       struct vop_reg format;
>> +       struct vop_reg hreg_block_split;
>> +       struct vop_reg pic_size;
>> +       struct vop_reg hdr_ptr;
>> +       struct vop_reg rstn;
>> +};
>> +
>>   struct vop_modeset {
>>          struct vop_reg htotal_pw;
>>          struct vop_reg hact_st_end;
>> @@ -134,6 +149,7 @@ struct vop_win_phy {
>>          const struct vop_scl_regs *scl;
>>          const uint32_t *data_formats;
>>          uint32_t nformats;
>> +       const uint64_t *format_modifiers;
>>
>>          struct vop_reg enable;
>>          struct vop_reg gate;
>> @@ -173,6 +189,7 @@ struct vop_data {
>>          const struct vop_misc *misc;
>>          const struct vop_modeset *modeset;
>>          const struct vop_output *output;
>> +       const struct vop_afbc *afbc;
>>          const struct vop_win_yuv2yuv_data *win_yuv2yuv;
>>          const struct vop_win_data *win;
>>          unsigned int win_size;
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 7a9d979c8d5d..2413deded22c 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -50,6 +50,17 @@ static const uint32_t formats_win_full[] = {
>>          DRM_FORMAT_NV24,
>>   };
>>
>> +static const uint64_t format_modifiers_win_full[] = {
>> +       DRM_FORMAT_MOD_LINEAR,
>> +       DRM_FORMAT_MOD_INVALID,
>> +};
>> +
>> +static const uint64_t format_modifiers_win_full_afbc[] = {
>> +       ROCKCHIP_AFBC_MOD,
>> +       DRM_FORMAT_MOD_LINEAR,
>> +       DRM_FORMAT_MOD_INVALID,
>> +};
>> +
>>   static const uint32_t formats_win_lite[] = {
>>          DRM_FORMAT_XRGB8888,
>>          DRM_FORMAT_ARGB8888,
>> @@ -61,6 +72,11 @@ static const uint32_t formats_win_lite[] = {
>>          DRM_FORMAT_BGR565,
>>   };
>>
>> +static const uint64_t format_modifiers_win_lite[] = {
>> +       DRM_FORMAT_MOD_LINEAR,
>> +       DRM_FORMAT_MOD_INVALID,
>> +};
>> +
>>   static const struct vop_scl_regs rk3036_win_scl = {
>>          .scale_yrgb_x = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 0x0),
>>          .scale_yrgb_y = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 16),
>> @@ -72,6 +88,7 @@ static const struct vop_win_phy rk3036_win0_data = {
>>          .scl = &rk3036_win_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 0),
>>          .format = VOP_REG(RK3036_SYS_CTRL, 0x7, 3),
>>          .rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 15),
>> @@ -87,6 +104,7 @@ static const struct vop_win_phy rk3036_win0_data = {
>>   static const struct vop_win_phy rk3036_win1_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 1),
>>          .format = VOP_REG(RK3036_SYS_CTRL, 0x7, 6),
>>          .rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 19),
>> @@ -153,6 +171,7 @@ static const struct vop_data rk3036_vop = {
>>   static const struct vop_win_phy rk3126_win1_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .enable = VOP_REG(RK3036_SYS_CTRL, 0x1, 1),
>>          .format = VOP_REG(RK3036_SYS_CTRL, 0x7, 6),
>>          .rb_swap = VOP_REG(RK3036_SYS_CTRL, 0x1, 19),
>> @@ -234,6 +253,7 @@ static const struct vop_win_phy px30_win0_data = {
>>          .scl = &px30_win_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(PX30_WIN0_CTRL0, 0x1, 0),
>>          .format = VOP_REG(PX30_WIN0_CTRL0, 0x7, 1),
>>          .rb_swap = VOP_REG(PX30_WIN0_CTRL0, 0x1, 12),
>> @@ -249,6 +269,7 @@ static const struct vop_win_phy px30_win0_data = {
>>   static const struct vop_win_phy px30_win1_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .enable = VOP_REG(PX30_WIN1_CTRL0, 0x1, 0),
>>          .format = VOP_REG(PX30_WIN1_CTRL0, 0x7, 4),
>>          .rb_swap = VOP_REG(PX30_WIN1_CTRL0, 0x1, 12),
>> @@ -261,6 +282,7 @@ static const struct vop_win_phy px30_win1_data = {
>>   static const struct vop_win_phy px30_win2_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .gate = VOP_REG(PX30_WIN2_CTRL0, 0x1, 4),
>>          .enable = VOP_REG(PX30_WIN2_CTRL0, 0x1, 0),
>>          .format = VOP_REG(PX30_WIN2_CTRL0, 0x3, 5),
>> @@ -316,6 +338,7 @@ static const struct vop_win_phy rk3066_win0_data = {
>>          .scl = &rk3066_win_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 0),
>>          .format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 4),
>>          .rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 19),
>> @@ -332,6 +355,7 @@ static const struct vop_win_phy rk3066_win1_data = {
>>          .scl = &rk3066_win_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 1),
>>          .format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 7),
>>          .rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 23),
>> @@ -347,6 +371,7 @@ static const struct vop_win_phy rk3066_win1_data = {
>>   static const struct vop_win_phy rk3066_win2_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .enable = VOP_REG(RK3066_SYS_CTRL1, 0x1, 2),
>>          .format = VOP_REG(RK3066_SYS_CTRL0, 0x7, 10),
>>          .rb_swap = VOP_REG(RK3066_SYS_CTRL0, 0x1, 27),
>> @@ -426,6 +451,7 @@ static const struct vop_win_phy rk3188_win0_data = {
>>          .scl = &rk3188_win_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(RK3188_SYS_CTRL, 0x1, 0),
>>          .format = VOP_REG(RK3188_SYS_CTRL, 0x7, 3),
>>          .rb_swap = VOP_REG(RK3188_SYS_CTRL, 0x1, 15),
>> @@ -440,6 +466,7 @@ static const struct vop_win_phy rk3188_win0_data = {
>>   static const struct vop_win_phy rk3188_win1_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .enable = VOP_REG(RK3188_SYS_CTRL, 0x1, 1),
>>          .format = VOP_REG(RK3188_SYS_CTRL, 0x7, 6),
>>          .rb_swap = VOP_REG(RK3188_SYS_CTRL, 0x1, 19),
>> @@ -545,6 +572,7 @@ static const struct vop_win_phy rk3288_win01_data = {
>>          .scl = &rk3288_win_full_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
>>          .format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
>>          .rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
>> @@ -563,6 +591,7 @@ static const struct vop_win_phy rk3288_win01_data = {
>>   static const struct vop_win_phy rk3288_win23_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
>>          .gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>>          .format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
>> @@ -677,6 +706,7 @@ static const struct vop_win_phy rk3368_win01_data = {
>>          .scl = &rk3288_win_full_scl,
>>          .data_formats = formats_win_full,
>>          .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full,
>>          .enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
>>          .format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
>>          .rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
>> @@ -697,6 +727,7 @@ static const struct vop_win_phy rk3368_win01_data = {
>>   static const struct vop_win_phy rk3368_win23_data = {
>>          .data_formats = formats_win_lite,
>>          .nformats = ARRAY_SIZE(formats_win_lite),
>> +       .format_modifiers = format_modifiers_win_lite,
>>          .gate = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 0),
>>          .enable = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 4),
>>          .format = VOP_REG(RK3368_WIN2_CTRL0, 0x3, 5),
>> @@ -817,6 +848,53 @@ static const struct vop_win_yuv2yuv_data rk3399_vop_big_win_yuv2yuv_data[] = {
>>            .y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 9) },
>>          { .base = 0xC0, .phy = &rk3399_yuv2yuv_win23_data },
>>          { .base = 0x120, .phy = &rk3399_yuv2yuv_win23_data },
>> +
>> +};
>> +
>> +static const struct vop_win_phy rk3399_win01_data = {
>> +       .scl = &rk3288_win_full_scl,
>> +       .data_formats = formats_win_full,
>> +       .nformats = ARRAY_SIZE(formats_win_full),
>> +       .format_modifiers = format_modifiers_win_full_afbc,
>> +       .enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
>> +       .format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
>> +       .rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
>> +       .y_mir_en = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 22),
>> +       .act_info = VOP_REG(RK3288_WIN0_ACT_INFO, 0x1fff1fff, 0),
>> +       .dsp_info = VOP_REG(RK3288_WIN0_DSP_INFO, 0x0fff0fff, 0),
>> +       .dsp_st = VOP_REG(RK3288_WIN0_DSP_ST, 0x1fff1fff, 0),
>> +       .yrgb_mst = VOP_REG(RK3288_WIN0_YRGB_MST, 0xffffffff, 0),
>> +       .uv_mst = VOP_REG(RK3288_WIN0_CBR_MST, 0xffffffff, 0),
>> +       .yrgb_vir = VOP_REG(RK3288_WIN0_VIR, 0x3fff, 0),
>> +       .uv_vir = VOP_REG(RK3288_WIN0_VIR, 0x3fff, 16),
>> +       .src_alpha_ctl = VOP_REG(RK3288_WIN0_SRC_ALPHA_CTRL, 0xff, 0),
>> +       .dst_alpha_ctl = VOP_REG(RK3288_WIN0_DST_ALPHA_CTRL, 0xff, 0),
>> +};
>> +
>> +/*
>> + * rk3399 vop big windows register layout is same as rk3288, but we
>> + * have a separate rk3399 win data array here so that we can advertise
>> + * AFBC on the primary plane.
>> + */
>> +static const struct vop_win_data rk3399_vop_win_data[] = {
>> +       { .base = 0x00, .phy = &rk3399_win01_data,
>> +         .type = DRM_PLANE_TYPE_PRIMARY },
>> +       { .base = 0x40, .phy = &rk3288_win01_data,
>> +         .type = DRM_PLANE_TYPE_OVERLAY },
>> +       { .base = 0x00, .phy = &rk3288_win23_data,
>> +         .type = DRM_PLANE_TYPE_OVERLAY },
>> +       { .base = 0x50, .phy = &rk3288_win23_data,
>> +         .type = DRM_PLANE_TYPE_CURSOR },
>> +};
>> +
>> +static const struct vop_afbc rk3399_vop_afbc = {
>> +       .rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
>> +       .enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
>> +       .win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
>> +       .format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
>> +       .hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
>> +       .hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
>> +       .pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
>>   };
>>
>>   static const struct vop_data rk3399_vop_big = {
>> @@ -826,9 +904,10 @@ static const struct vop_data rk3399_vop_big = {
>>          .common = &rk3288_common,
>>          .modeset = &rk3288_modeset,
>>          .output = &rk3399_output,
>> +       .afbc = &rk3399_vop_afbc,
>>          .misc = &rk3368_misc,
>> -       .win = rk3368_vop_win_data,
>> -       .win_size = ARRAY_SIZE(rk3368_vop_win_data),
>> +       .win = rk3399_vop_win_data,
>> +       .win_size = ARRAY_SIZE(rk3399_vop_win_data),
>>          .win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>>   };
>>
>> --
>> 2.17.1
> Heiko, Sandy, being the maintainers of the Rockchip driver, can you
> review/ack this patch?
>
> I believe the intention is to merge the series via drm-misc. Andrzej
> already has commit access.
>
> -Emil
>
>
Thanks for you patch, maybe we need to consider the compatibility with 
px30 platform afbc features,the main difference is:

1.  px30 support x offset and y offset display, and the state->src.x1 / 
state->src.y1  should be alibegned as 16pixel/line;

2.px30 only win1 can support afbdc format;


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

Re: [PATCHv7 6/6] drm/rockchip: Add support for afbc

[Andrzej Pietrasiewicz]

Hi Sandy,


W dniu 19.03.2020 o 03:57, Sandy Huang pisze:
> Hi Emil,
> 

<snip>

>>> -- 
>>> 2.17.1
>> Heiko, Sandy, being the maintainers of the Rockchip driver, can you
>> review/ack this patch?
>>
>> I believe the intention is to merge the series via drm-misc. Andrzej
>> already has commit access.
>>
>> -Emil
>>
>>
> Thanks for you patch, maybe we need to consider the compatibility with px30 
> platform afbc features,the main difference is:
> 
> 1.  px30 support x offset and y offset display, and the state->src.x1 / 
> state->src.y1  should be alibegned as 16pixel/line;
> 
> 2.px30 only win1 can support afbdc format;
> 
> 

Actually I sent the patch, Emil kindly forwarded it to you.

Unfortunately I don't have px30 hardware, so I can't test. Can we
merge the patch as is (if you are ok with the way it is now) and then
I will help reviewing the patch adding px30 support on top of it?

Regards,

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

Re: [PATCHv7 6/6] drm/rockchip: Add support for afbc

[Sandy Huang]

Hi Andrzej ,


在 2020/3/19 下午5:54, Andrzej Pietrasiewicz 写道:
> Hi Sandy,
>
>
> W dniu 19.03.2020 o 03:57, Sandy Huang pisze:
>> Hi Emil,
>>
>
> <snip>
>
>>>> -- 
>>>> 2.17.1
>>> Heiko, Sandy, being the maintainers of the Rockchip driver, can you
>>> review/ack this patch?
>>>
>>> I believe the intention is to merge the series via drm-misc. Andrzej
>>> already has commit access.
>>>
>>> -Emil
>>>
>>>
>> Thanks for you patch, maybe we need to consider the compatibility 
>> with px30 platform afbc features,the main difference is:
>>
>> 1.  px30 support x offset and y offset display, and the state->src.x1 
>> / state->src.y1  should be alibegned as 16pixel/line;
>>
>> 2.px30 only win1 can support afbdc format;
>>
>>
>
> Actually I sent the patch, Emil kindly forwarded it to you.
>
> Unfortunately I don't have px30 hardware, so I can't test. Can we
> merge the patch as is (if you are ok with the way it is now) and then
> I will help reviewing the patch adding px30 support on top of it?
>
> Regards,
>
> Andrzej

OK, Let's add px30 AFBC support later.

Reviewed-by: Sandy Huang <hjc@rock-chips.com>

>
>
>


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