Re: [PATCH v2] drm/arm/malidp: Validate rotations for compressed/uncompressed framebuffers for each layer

2018-09-24 Thread Liviu Dudau
On Mon, Sep 24, 2018 at 03:40:15PM +0100, Ayan Halder wrote:
> Hi Liviu,

Hi Ayan,

> 
> On Fri, Sep 21, 2018 at 03:33:53PM +0100, Liviu Dudau wrote:
> > From: Ayan Kumar Halder 
> > 
> > Add support for compressed framebuffers that are described using
> > the framebuffer's modifier field. Mali DP uses the rotation memory for
> > the decompressor of the format, so we need to check for space when
> > the modifiers are present.
> > 
> > Signed-off-by: Ayan Kumar Halder 
> > Reviewed-by: Brian Starkey 
> > [re-worded commit, rebased, cleaned up duplicated checks for
> >  RGB888 and BGR888 and removed additional parameter for
> >  rotmem_required function hook]
> > Signed-off-by: Liviu Dudau 
> > ---
> >  drivers/gpu/drm/arm/malidp_crtc.c   | 28 -
> >  drivers/gpu/drm/arm/malidp_hw.c | 38 -
> >  drivers/gpu/drm/arm/malidp_hw.h |  7 ++
> >  drivers/gpu/drm/arm/malidp_planes.c | 19 +++
> >  4 files changed, 53 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/malidp_crtc.c 
> > b/drivers/gpu/drm/arm/malidp_crtc.c
> > index ef44202fb43f8..e1b72782848c3 100644
> > --- a/drivers/gpu/drm/arm/malidp_crtc.c
> > +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> > @@ -348,19 +348,20 @@ static int malidp_crtc_atomic_check(struct drm_crtc 
> > *crtc,
> >  
> > /*
> >  * check if there is enough rotation memory available for planes
> > -* that need 90?? and 270?? rotation. Each plane has set its required
> > -* memory size in the ->plane_check() callback, here we only make
> > -* sure that the sums are less that the total usable memory.
> > +* that need 90?? and 270?? rotion or planes that are compressed.
> > +* Each plane has set its required memory size in the ->plane_check()
> > +* callback, here we only make sure that the sums are less that the
> > +* total usable memory.
> >  *
> >  * The rotation memory allocation algorithm (for each plane):
> > -*  a. If no more rotated planes exist, all remaining rotate
> > -* memory in the bank is available for use by the plane.
> > -*  b. If other rotated planes exist, and plane's layer ID is
> > -* DE_VIDEO1, it can use all the memory from first bank if
> > -* secondary rotation memory bank is available, otherwise it can
> > +*  a. If no more rotated or compressed planes exist, all remaining
> > +* rotate memory in the bank is available for use by the plane.
> > +*  b. If other rotated or compressed planes exist, and plane's
> > +* layer ID is DE_VIDEO1, it can use all the memory from first bank
> > +* if secondary rotation memory bank is available, otherwise it can
> >  * use up to half the bank's memory.
> > -*  c. If other rotated planes exist, and plane's layer ID is not
> > -* DE_VIDEO1, it can use half of the available memory
> > +*  c. If other rotated or compressed planes exist, and plane's layer ID
> > +* is not DE_VIDEO1, it can use half of the available memory.
> >  *
> >  * Note: this algorithm assumes that the order in which the planes are
> >  * checked always has DE_VIDEO1 plane first in the list if it is
> > @@ -372,7 +373,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc 
> > *crtc,
> >  
> > /* first count the number of rotated planes */
> > drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> > -   if (pstate->rotation & MALIDP_ROTATED_MASK)
> > +   struct drm_framebuffer *fb = pstate->fb;
> > +
> > +   if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier)
> > rotated_planes++;
> > }
> >  
> > @@ -388,8 +391,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc 
> > *crtc,
> > drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> > struct malidp_plane *mp = to_malidp_plane(plane);
> > struct malidp_plane_state *ms = to_malidp_plane_state(pstate);
> > +   struct drm_framebuffer *fb = pstate->fb;
> >  
> > -   if (pstate->rotation & MALIDP_ROTATED_MASK) {
> > +   if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) {
> > /* process current plane */
> > rotated_planes--;
> >  
> > diff --git a/drivers/gpu/drm/arm/malidp_hw.c 
> > b/drivers/gpu/drm/arm/malidp_hw.c
> > index b33000634a4ee..5549092e6c36a 100644
> > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > @@ -85,41 +85,43 @@ static const struct malidp_format_id 
> > malidp550_de_formats[] = {
> >  
> >  static const struct malidp_layer malidp500_layers[] = {
> > /*  id, base address, fb pointer address base, stride offset,
> > -   yuv2rgb matrix offset, mmu control register offset */
> > +   yuv2rgb matrix offset, mmu control register offset, 
> > rotation_features */
> > { DE_VIDEO1, MALIDP500_DE_LV_BASE, 

Re: [PATCH v2] drm/arm/malidp: Validate rotations for compressed/uncompressed framebuffers for each layer

2018-09-24 Thread Ayan Halder
Hi Liviu,

On Fri, Sep 21, 2018 at 03:33:53PM +0100, Liviu Dudau wrote:
> From: Ayan Kumar Halder 
> 
> Add support for compressed framebuffers that are described using
> the framebuffer's modifier field. Mali DP uses the rotation memory for
> the decompressor of the format, so we need to check for space when
> the modifiers are present.
> 
> Signed-off-by: Ayan Kumar Halder 
> Reviewed-by: Brian Starkey 
> [re-worded commit, rebased, cleaned up duplicated checks for
>  RGB888 and BGR888 and removed additional parameter for
>  rotmem_required function hook]
> Signed-off-by: Liviu Dudau 
> ---
>  drivers/gpu/drm/arm/malidp_crtc.c   | 28 -
>  drivers/gpu/drm/arm/malidp_hw.c | 38 -
>  drivers/gpu/drm/arm/malidp_hw.h |  7 ++
>  drivers/gpu/drm/arm/malidp_planes.c | 19 +++
>  4 files changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c 
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index ef44202fb43f8..e1b72782848c3 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -348,19 +348,20 @@ static int malidp_crtc_atomic_check(struct drm_crtc 
> *crtc,
>  
>   /*
>* check if there is enough rotation memory available for planes
> -  * that need 90?? and 270?? rotation. Each plane has set its required
> -  * memory size in the ->plane_check() callback, here we only make
> -  * sure that the sums are less that the total usable memory.
> +  * that need 90?? and 270?? rotion or planes that are compressed.
> +  * Each plane has set its required memory size in the ->plane_check()
> +  * callback, here we only make sure that the sums are less that the
> +  * total usable memory.
>*
>* The rotation memory allocation algorithm (for each plane):
> -  *  a. If no more rotated planes exist, all remaining rotate
> -  * memory in the bank is available for use by the plane.
> -  *  b. If other rotated planes exist, and plane's layer ID is
> -  * DE_VIDEO1, it can use all the memory from first bank if
> -  * secondary rotation memory bank is available, otherwise it can
> +  *  a. If no more rotated or compressed planes exist, all remaining
> +  * rotate memory in the bank is available for use by the plane.
> +  *  b. If other rotated or compressed planes exist, and plane's
> +  * layer ID is DE_VIDEO1, it can use all the memory from first bank
> +  * if secondary rotation memory bank is available, otherwise it can
>* use up to half the bank's memory.
> -  *  c. If other rotated planes exist, and plane's layer ID is not
> -  * DE_VIDEO1, it can use half of the available memory
> +  *  c. If other rotated or compressed planes exist, and plane's layer ID
> +  * is not DE_VIDEO1, it can use half of the available memory.
>*
>* Note: this algorithm assumes that the order in which the planes are
>* checked always has DE_VIDEO1 plane first in the list if it is
> @@ -372,7 +373,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
>  
>   /* first count the number of rotated planes */
>   drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> - if (pstate->rotation & MALIDP_ROTATED_MASK)
> + struct drm_framebuffer *fb = pstate->fb;
> +
> + if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier)
>   rotated_planes++;
>   }
>  
> @@ -388,8 +391,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
>   drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
>   struct malidp_plane *mp = to_malidp_plane(plane);
>   struct malidp_plane_state *ms = to_malidp_plane_state(pstate);
> + struct drm_framebuffer *fb = pstate->fb;
>  
> - if (pstate->rotation & MALIDP_ROTATED_MASK) {
> + if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) {
>   /* process current plane */
>   rotated_planes--;
>  
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index b33000634a4ee..5549092e6c36a 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -85,41 +85,43 @@ static const struct malidp_format_id 
> malidp550_de_formats[] = {
>  
>  static const struct malidp_layer malidp500_layers[] = {
>   /*  id, base address, fb pointer address base, stride offset,
> - yuv2rgb matrix offset, mmu control register offset */
> + yuv2rgb matrix offset, mmu control register offset, 
> rotation_features */
>   { DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
> - MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0 },
> + MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0, ROTATE_ANY },
>   { DE_GRAPHICS1, 

[PATCH v2] drm/arm/malidp: Validate rotations for compressed/uncompressed framebuffers for each layer

2018-09-21 Thread Liviu Dudau
From: Ayan Kumar Halder 

Add support for compressed framebuffers that are described using
the framebuffer's modifier field. Mali DP uses the rotation memory for
the decompressor of the format, so we need to check for space when
the modifiers are present.

Signed-off-by: Ayan Kumar Halder 
Reviewed-by: Brian Starkey 
[re-worded commit, rebased, cleaned up duplicated checks for
 RGB888 and BGR888 and removed additional parameter for
 rotmem_required function hook]
Signed-off-by: Liviu Dudau 
---
 drivers/gpu/drm/arm/malidp_crtc.c   | 28 -
 drivers/gpu/drm/arm/malidp_hw.c | 38 -
 drivers/gpu/drm/arm/malidp_hw.h |  7 ++
 drivers/gpu/drm/arm/malidp_planes.c | 19 +++
 4 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_crtc.c 
b/drivers/gpu/drm/arm/malidp_crtc.c
index ef44202fb43f8..e1b72782848c3 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -348,19 +348,20 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
 
/*
 * check if there is enough rotation memory available for planes
-* that need 90° and 270° rotation. Each plane has set its required
-* memory size in the ->plane_check() callback, here we only make
-* sure that the sums are less that the total usable memory.
+* that need 90° and 270° rotion or planes that are compressed.
+* Each plane has set its required memory size in the ->plane_check()
+* callback, here we only make sure that the sums are less that the
+* total usable memory.
 *
 * The rotation memory allocation algorithm (for each plane):
-*  a. If no more rotated planes exist, all remaining rotate
-* memory in the bank is available for use by the plane.
-*  b. If other rotated planes exist, and plane's layer ID is
-* DE_VIDEO1, it can use all the memory from first bank if
-* secondary rotation memory bank is available, otherwise it can
+*  a. If no more rotated or compressed planes exist, all remaining
+* rotate memory in the bank is available for use by the plane.
+*  b. If other rotated or compressed planes exist, and plane's
+* layer ID is DE_VIDEO1, it can use all the memory from first bank
+* if secondary rotation memory bank is available, otherwise it can
 * use up to half the bank's memory.
-*  c. If other rotated planes exist, and plane's layer ID is not
-* DE_VIDEO1, it can use half of the available memory
+*  c. If other rotated or compressed planes exist, and plane's layer ID
+* is not DE_VIDEO1, it can use half of the available memory.
 *
 * Note: this algorithm assumes that the order in which the planes are
 * checked always has DE_VIDEO1 plane first in the list if it is
@@ -372,7 +373,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
 
/* first count the number of rotated planes */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
-   if (pstate->rotation & MALIDP_ROTATED_MASK)
+   struct drm_framebuffer *fb = pstate->fb;
+
+   if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier)
rotated_planes++;
}
 
@@ -388,8 +391,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
struct malidp_plane *mp = to_malidp_plane(plane);
struct malidp_plane_state *ms = to_malidp_plane_state(pstate);
+   struct drm_framebuffer *fb = pstate->fb;
 
-   if (pstate->rotation & MALIDP_ROTATED_MASK) {
+   if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) {
/* process current plane */
rotated_planes--;
 
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index b33000634a4ee..5549092e6c36a 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -85,41 +85,43 @@ static const struct malidp_format_id malidp550_de_formats[] 
= {
 
 static const struct malidp_layer malidp500_layers[] = {
/*  id, base address, fb pointer address base, stride offset,
-   yuv2rgb matrix offset, mmu control register offset */
+   yuv2rgb matrix offset, mmu control register offset, 
rotation_features */
{ DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
-   MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0 },
+   MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0, ROTATE_ANY },
{ DE_GRAPHICS1, MALIDP500_DE_LG1_BASE, MALIDP500_DE_LG1_PTR_BASE,
-   MALIDP_DE_LG_STRIDE, 0, 0 },
+   MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY },
{