Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 5 Jul 2018 17:20:56 +0100
From:   Ayan Halder <ayan.halder@arm.com>
To:     liviu.dudau@arm.com, brian.starkey@arm.com, malidp@foss.arm.com,
        airlied@linux.ie, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, nd@arm.com
Subject: Re: [RFC PATCH v2] drm/arm/malidp: Added support for AFBC modifiers
 for all layers except DE_SMART
Message-ID: <20180705162056.GA18005@arm.com>
References: <1530800743-2250-1-git-send-email-ayan.halder@arm.com>
 <20180705154822.GC4273@phenom.ffwll.local>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180705154822.GC4273@phenom.ffwll.local>
User-Agent: Mutt/1.5.24 (2015-08-30)
Content-Transfer-Encoding: quoted-printable
Received-SPF: None (protection.outlook.com: arm.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2018 16:21:00.5219
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: aac86b3a-afa9-4078-75cd-08d5e2934c07
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0801MB1388
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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 BGR8=
88.
> >
> > 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_suppor=
ted()
> > 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/mal=
idp_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 =3D hwdev->max_line_size;
> >     drm->mode_config.funcs =3D &malidp_mode_config_funcs;
> >     drm->mode_config.helper_private =3D &malidp_mode_config_helpers;
> > +   drm->mode_config.allow_fb_modifiers =3D true;
> >
> >     ret =3D 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=3D%u\n", ms->n_planes);
> >  }
> >
> > +static bool malidp_format_mod_supported(struct drm_plane *plane,
> > +                                   u32 format, u64 modifier)
> > +{
> > +   if (WARN_ON(modifier =3D=3D DRM_FORMAT_MOD_INVALID))
> > +           return false;
> > +
> > +   /* All the pixel formats are supported without any modifier */
> > +   if (modifier =3D=3D DRM_FORMAT_MOD_LINEAR)
> > +           return true;
> > +
> > +   if ((modifier >> 56) !=3D 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 =3D=3D DRM_FORMAT_BGR888)
> > +                   return true;
> > +   }
> > +   return false;
> > +}
> > +
> >  static const struct drm_plane_funcs malidp_de_plane_funcs =3D {
> >     .update_plane =3D drm_atomic_helper_update_plane,
> >     .disable_plane =3D drm_atomic_helper_disable_plane,
> > @@ -132,6 +161,7 @@ static const struct drm_plane_funcs malidp_de_plane=
_funcs =3D {
> >     .atomic_duplicate_state =3D malidp_duplicate_plane_state,
> >     .atomic_destroy_state =3D malidp_destroy_plane_state,
> >     .atomic_print_state =3D malidp_plane_atomic_print_state,
> > +   .format_mod_supported =3D 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[] =3D {
> > +           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 =3D kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KER=
NEL);
> >     if (!formats) {
> >             ret =3D -ENOMEM;
> > @@ -547,9 +584,14 @@ int malidp_de_planes_init(struct drm_device *drm)
> >
> >             plane_type =3D (i =3D=3D 0) ? DRM_PLANE_TYPE_PRIMARY :
> >                                     DRM_PLANE_TYPE_OVERLAY;
> > +
> > +           /*
> > +            * All the layers except smart layer supports AFBC modifier=
s.
> > +            */
> >             ret =3D drm_universal_plane_init(drm, &plane->base, crtcs,
> > -                                          &malidp_de_plane_funcs, form=
ats,
> > -                                          n, NULL, plane_type, NULL);
> > +                           &malidp_de_plane_funcs, formats, n,
> > +                           (id =3D=3D DE_SMART) ? NULL : modifiers, pl=
ane_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 confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease 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.