diff mbox

[RFC,v2] drm/arm/malidp: Added support for AFBC modifiers for all layers except DE_SMART

Message ID 1530800743-2250-1-git-send-email-ayan.halder@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ayan Halder July 5, 2018, 2:25 p.m. UTC
On planes which support AFBC, expose an AFBC modifier for use with BGR888.

Signed-off-by: Ayan Kumar halder <ayan.halder@arm.com>
Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Changes from v2:
- Removed the gerrit change-id
- Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
to report unsupported modifiers.
---
 drivers/gpu/drm/arm/malidp_drv.c    |  1 +
 drivers/gpu/drm/arm/malidp_planes.c | 46 +++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

Comments

Daniel Vetter July 5, 2018, 3:48 p.m. UTC | #1
On Thu, Jul 05, 2018 at 03:25:43PM +0100, Ayan Kumar Halder wrote:
> On planes which support AFBC, expose an AFBC modifier for use with BGR888.
> 
> Signed-off-by: Ayan Kumar halder <ayan.halder@arm.com>
> Reviewed-by: Brian Starkey <brian.starkey@arm.com>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> Changes from v2:
> - Removed the gerrit change-id
> - Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
> to report unsupported modifiers.
> ---
>  drivers/gpu/drm/arm/malidp_drv.c    |  1 +
>  drivers/gpu/drm/arm/malidp_planes.c | 46 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 7b6a848..7bcd679 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -392,6 +392,7 @@ static int malidp_init(struct drm_device *drm)
>  	drm->mode_config.max_height = hwdev->max_line_size;
>  	drm->mode_config.funcs = &malidp_mode_config_funcs;
>  	drm->mode_config.helper_private = &malidp_mode_config_helpers;
> +	drm->mode_config.allow_fb_modifiers = true;
>  
>  	ret = malidp_crtc_init(drm);
>  	if (ret) {
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 3950504..914cc58 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -124,6 +124,35 @@ static void malidp_plane_atomic_print_state(struct drm_printer *p,
>  	drm_printf(p, "\tn_planes=%u\n", ms->n_planes);
>  }
>  
> +static bool malidp_format_mod_supported(struct drm_plane *plane,
> +					u32 format, u64 modifier)
> +{
> +	if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> +		return false;
> +
> +	/* All the pixel formats are supported without any modifier */
> +	if (modifier == DRM_FORMAT_MOD_LINEAR)
> +		return true;
> +
> +	if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_ARM)
> +		return false;
> +
> +	if (modifier &
> +	    ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
> +		DRM_DEBUG_KMS("Unsupported modifiers\n");
> +		return false;
> +	}

I think the core checks for that by now, or at least Ville had some
patches to make that happen ...

I also don't see the ARM_AFBC modifier in upstream yet, is that still
in-flight somewhere?
-Daniel

> +
> +	switch (modifier) {
> +	case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> +				AFBC_FORMAT_MOD_YTR |
> +				AFBC_FORMAT_MOD_SPARSE):
> +		if (format == DRM_FORMAT_BGR888)
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static const struct drm_plane_funcs malidp_de_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
> @@ -132,6 +161,7 @@ static const struct drm_plane_funcs malidp_de_plane_funcs = {
>  	.atomic_duplicate_state = malidp_duplicate_plane_state,
>  	.atomic_destroy_state = malidp_destroy_plane_state,
>  	.atomic_print_state = malidp_plane_atomic_print_state,
> +	.format_mod_supported = malidp_format_mod_supported,
>  };
>  
>  static int malidp_se_check_scaling(struct malidp_plane *mp,
> @@ -524,6 +554,13 @@ int malidp_de_planes_init(struct drm_device *drm)
>  	u32 *formats;
>  	int ret, i, j, n;
>  
> +	static const u64 modifiers[] = {
> +		DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> +			AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPARSE),
> +		DRM_FORMAT_MOD_LINEAR,
> +		DRM_FORMAT_MOD_INVALID
> +	};
> +
>  	formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
>  	if (!formats) {
>  		ret = -ENOMEM;
> @@ -547,9 +584,14 @@ int malidp_de_planes_init(struct drm_device *drm)
>  
>  		plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
>  					DRM_PLANE_TYPE_OVERLAY;
> +
> +		/*
> +		 * All the layers except smart layer supports AFBC modifiers.
> +		 */
>  		ret = drm_universal_plane_init(drm, &plane->base, crtcs,
> -					       &malidp_de_plane_funcs, formats,
> -					       n, NULL, plane_type, NULL);
> +				&malidp_de_plane_funcs, formats, n,
> +				(id == DE_SMART) ? NULL : modifiers, plane_type, NULL);
> +
>  		if (ret < 0)
>  			goto cleanup;
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ayan Halder July 5, 2018, 4:20 p.m. UTC | #2
On Thu, Jul 05, 2018 at 05:48:22PM +0200, Daniel Vetter wrote:
> On Thu, Jul 05, 2018 at 03:25:43PM +0100, Ayan Kumar Halder wrote:
> > On planes which support AFBC, expose an AFBC modifier for use with BGR888.
> >
> > Signed-off-by: Ayan Kumar halder <ayan.halder@arm.com>
> > Reviewed-by: Brian Starkey <brian.starkey@arm.com>
> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> >
> > Changes from v2:
> > - Removed the gerrit change-id
> > - Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
> > to report unsupported modifiers.
> > ---
> >  drivers/gpu/drm/arm/malidp_drv.c    |  1 +
> >  drivers/gpu/drm/arm/malidp_planes.c | 46 +++++++++++++++++++++++++++++++++++--
> >  2 files changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> > index 7b6a848..7bcd679 100644
> > --- a/drivers/gpu/drm/arm/malidp_drv.c
> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
> > @@ -392,6 +392,7 @@ static int malidp_init(struct drm_device *drm)
> >     drm->mode_config.max_height = hwdev->max_line_size;
> >     drm->mode_config.funcs = &malidp_mode_config_funcs;
> >     drm->mode_config.helper_private = &malidp_mode_config_helpers;
> > +   drm->mode_config.allow_fb_modifiers = true;
> >
> >     ret = malidp_crtc_init(drm);
> >     if (ret) {
> > diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> > index 3950504..914cc58 100644
> > --- a/drivers/gpu/drm/arm/malidp_planes.c
> > +++ b/drivers/gpu/drm/arm/malidp_planes.c
> > @@ -124,6 +124,35 @@ static void malidp_plane_atomic_print_state(struct drm_printer *p,
> >     drm_printf(p, "\tn_planes=%u\n", ms->n_planes);
> >  }
> >
> > +static bool malidp_format_mod_supported(struct drm_plane *plane,
> > +                                   u32 format, u64 modifier)
> > +{
> > +   if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> > +           return false;
> > +
> > +   /* All the pixel formats are supported without any modifier */
> > +   if (modifier == DRM_FORMAT_MOD_LINEAR)
> > +           return true;
> > +
> > +   if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_ARM)
> > +           return false;
> > +
> > +   if (modifier &
> > +       ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
> > +           DRM_DEBUG_KMS("Unsupported modifiers\n");
> > +           return false;
> > +   }
>
> I think the core checks for that by now, or at least Ville had some
> patches to make that happen ...

Can you paste the link (or atleast the title) of the patches that you
are referring to ? I might then have to rework on this.
> I also don't see the ARM_AFBC modifier in upstream yet, is that still
> in-flight somewhere?

I realized that I was missing the ack on my first patch of the series
(ie
https://lists.freedesktop.org/archives/dri-devel/2018-June/180125.html)
and the reason I believe is that I did not put the correct email
addresses in "--to" for the drm_fourcc.h maintainers. I will resend the
patch and wait for their ack.

> -Daniel
>
> > +
> > +   switch (modifier) {
> > +   case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> > +                           AFBC_FORMAT_MOD_YTR |
> > +                           AFBC_FORMAT_MOD_SPARSE):
> > +           if (format == DRM_FORMAT_BGR888)
> > +                   return true;
> > +   }
> > +   return false;
> > +}
> > +
> >  static const struct drm_plane_funcs malidp_de_plane_funcs = {
> >     .update_plane = drm_atomic_helper_update_plane,
> >     .disable_plane = drm_atomic_helper_disable_plane,
> > @@ -132,6 +161,7 @@ static const struct drm_plane_funcs malidp_de_plane_funcs = {
> >     .atomic_duplicate_state = malidp_duplicate_plane_state,
> >     .atomic_destroy_state = malidp_destroy_plane_state,
> >     .atomic_print_state = malidp_plane_atomic_print_state,
> > +   .format_mod_supported = malidp_format_mod_supported,
> >  };
> >
> >  static int malidp_se_check_scaling(struct malidp_plane *mp,
> > @@ -524,6 +554,13 @@ int malidp_de_planes_init(struct drm_device *drm)
> >     u32 *formats;
> >     int ret, i, j, n;
> >
> > +   static const u64 modifiers[] = {
> > +           DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> > +                   AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPARSE),
> > +           DRM_FORMAT_MOD_LINEAR,
> > +           DRM_FORMAT_MOD_INVALID
> > +   };
> > +
> >     formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
> >     if (!formats) {
> >             ret = -ENOMEM;
> > @@ -547,9 +584,14 @@ int malidp_de_planes_init(struct drm_device *drm)
> >
> >             plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
> >                                     DRM_PLANE_TYPE_OVERLAY;
> > +
> > +           /*
> > +            * All the layers except smart layer supports AFBC modifiers.
> > +            */
> >             ret = drm_universal_plane_init(drm, &plane->base, crtcs,
> > -                                          &malidp_de_plane_funcs, formats,
> > -                                          n, NULL, plane_type, NULL);
> > +                           &malidp_de_plane_funcs, formats, n,
> > +                           (id == DE_SMART) ? NULL : modifiers, plane_type, NULL);
> > +
> >             if (ret < 0)
> >                     goto cleanup;
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff mbox

Patch

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 7b6a848..7bcd679 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -392,6 +392,7 @@  static int malidp_init(struct drm_device *drm)
 	drm->mode_config.max_height = hwdev->max_line_size;
 	drm->mode_config.funcs = &malidp_mode_config_funcs;
 	drm->mode_config.helper_private = &malidp_mode_config_helpers;
+	drm->mode_config.allow_fb_modifiers = true;
 
 	ret = malidp_crtc_init(drm);
 	if (ret) {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 3950504..914cc58 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -124,6 +124,35 @@  static void malidp_plane_atomic_print_state(struct drm_printer *p,
 	drm_printf(p, "\tn_planes=%u\n", ms->n_planes);
 }
 
+static bool malidp_format_mod_supported(struct drm_plane *plane,
+					u32 format, u64 modifier)
+{
+	if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
+		return false;
+
+	/* All the pixel formats are supported without any modifier */
+	if (modifier == DRM_FORMAT_MOD_LINEAR)
+		return true;
+
+	if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_ARM)
+		return false;
+
+	if (modifier &
+	    ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
+		DRM_DEBUG_KMS("Unsupported modifiers\n");
+		return false;
+	}
+
+	switch (modifier) {
+	case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
+				AFBC_FORMAT_MOD_YTR |
+				AFBC_FORMAT_MOD_SPARSE):
+		if (format == DRM_FORMAT_BGR888)
+			return true;
+	}
+	return false;
+}
+
 static const struct drm_plane_funcs malidp_de_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
@@ -132,6 +161,7 @@  static const struct drm_plane_funcs malidp_de_plane_funcs = {
 	.atomic_duplicate_state = malidp_duplicate_plane_state,
 	.atomic_destroy_state = malidp_destroy_plane_state,
 	.atomic_print_state = malidp_plane_atomic_print_state,
+	.format_mod_supported = malidp_format_mod_supported,
 };
 
 static int malidp_se_check_scaling(struct malidp_plane *mp,
@@ -524,6 +554,13 @@  int malidp_de_planes_init(struct drm_device *drm)
 	u32 *formats;
 	int ret, i, j, n;
 
+	static const u64 modifiers[] = {
+		DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
+			AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPARSE),
+		DRM_FORMAT_MOD_LINEAR,
+		DRM_FORMAT_MOD_INVALID
+	};
+
 	formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
 	if (!formats) {
 		ret = -ENOMEM;
@@ -547,9 +584,14 @@  int malidp_de_planes_init(struct drm_device *drm)
 
 		plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
 					DRM_PLANE_TYPE_OVERLAY;
+
+		/*
+		 * All the layers except smart layer supports AFBC modifiers.
+		 */
 		ret = drm_universal_plane_init(drm, &plane->base, crtcs,
-					       &malidp_de_plane_funcs, formats,
-					       n, NULL, plane_type, NULL);
+				&malidp_de_plane_funcs, formats, n,
+				(id == DE_SMART) ? NULL : modifiers, plane_type, NULL);
+
 		if (ret < 0)
 			goto cleanup;