lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list] 
Date:   Tue, 15 Jan 2019 14:29:19 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     Liviu Dudau <Liviu.Dudau@....com>
Cc:     Ayan Halder <Ayan.Halder@....com>,
        Brian Starkey <Brian.Starkey@....com>,
        "john.stultz@...aro.org" <john.stultz@...aro.org>,
        "seanpaul@...gle.com" <seanpaul@...gle.com>,
        "malidp@...s.arm.com" <malidp@...s.arm.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        nd <nd@....com>
Subject: Re: [PATCH] [RFC] drm_hwcomposer: Add support for Arm Framebuffer
 Compression (AFBC) modifiers.

On Tue, Jan 15, 2019 at 2:27 PM Liviu Dudau <Liviu.Dudau@....com> wrote:
>
> On Tue, Jan 15, 2019 at 01:38:19PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 15, 2019 at 1:24 PM Liviu Dudau <Liviu.Dudau@....com> wrote:
> > >
> > > On Tue, Jan 15, 2019 at 01:05:47PM +0100, Daniel Vetter wrote:
> > > > On Mon, Jan 14, 2019 at 03:28:27PM +0000, Ayan Halder wrote:
> > > > > One needs to translate the Gralloc buffer flags for AFBC (eg
> > > > > MALI_GRALLOC_INTFMT_AFBC_BASIC) to the corresponding linux kernel drm modifiers.
> > > > > This gets passed to libdrm via drmModeAddFB2WithModifiers.
> > > > >
> > > > > Signed-off-by: Ayan Kumar Halder <ayan.halder@....com>
> > > > >
> > > > > /-- Note for reviewer
> > > > > I was able to get this working for Android P on Juno with Mali DP650 and Mali
> > > > > T860 gpu(with some additional hacks). I have not yet validated this hikey960.
> > > > >
> > > > > I have used the following components:-
> > > > > 1. Gralloc (from https://android.googlesource.com/device/linaro/hikey/+/master/gralloc960)
> > > > >   - Built with MALI_MMSS=1
> > > > > 2. Libdrm (from git://anongit.freedesktop.org/mesa/drm)
> > > > >   - You would need drm_fourcc.h and gralloc_handle.h
> > > > > --/
> > > >
> > > > I thought drm_hwcomposer has switched over to gitlab merge requests?
> > > > README at least says so:
> > > >
> > > > https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer
> > >
> > > So are we to send pull requests for RFCs as well?
> >
> > Hm, not sure you should stuff RFC material into your linux-next
> > branches. I thought that's only for patches which by all intents are
> > ready for the next merge window, i.e. reviewed&tested and all that.
> > Not for in-flight stuff still under discussion.
>
> I think you're confusing the documentation patch with the drm_hwcomposer one (this one).

Indeed. And yes everything even RFC. Gitlab even has some special
support for work in progress, just start your pull request summary
with WIP.
-Daniel

>
> Best regards,
> Liviu
>
> > -Daniel
> >
> >
> > > Best regards,
> > > Liviu
> > >
> > > >
> > > > Cheers, Daniel
> > > >
> > > > > ---
> > > > >  platformdrmgeneric.cpp | 42 ++++++++++++++++++++++++++++++++++++++++++
> > > > >  platformdrmgeneric.h   |  2 ++
> > > > >  platformhisi.cpp       | 14 ++++++++++++--
> > > > >  3 files changed, 56 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/platformdrmgeneric.cpp b/platformdrmgeneric.cpp
> > > > > index 503c04a..a520224 100644
> > > > > --- a/platformdrmgeneric.cpp
> > > > > +++ b/platformdrmgeneric.cpp
> > > > > @@ -18,6 +18,7 @@
> > > > >
> > > > >  #include "platformdrmgeneric.h"
> > > > >  #include "drmdevice.h"
> > > > > +#include "mali_gralloc_formats.h"
> > > > >  #include "platform.h"
> > > > >
> > > > >  #include <drm/drm_fourcc.h>
> > > > > @@ -83,6 +84,31 @@ uint32_t DrmGenericImporter::ConvertHalFormatToDrm(uint32_t hal_format) {
> > > > >    }
> > > > >  }
> > > > >
> > > > > +uint64_t DrmGenericImporter::ConvertGrallocFormatToDrmModifiers(uint64_t flags,
> > > > > +                                                                bool is_rgb) {
> > > > > +  uint64_t features = 0;
> > > > > +
> > > > > +  if (flags & MALI_GRALLOC_INTFMT_AFBC_BASIC)
> > > > > +    features |= AFBC_FORMAT_MOD_BLOCK_SIZE_16x16;
> > > > > +
> > > > > +  if (is_rgb)
> > > > > +    features |= AFBC_FORMAT_MOD_YTR;
> > > > > +
> > > > > +  if (flags & MALI_GRALLOC_INTFMT_AFBC_SPLITBLK)
> > > > > +    features |= (AFBC_FORMAT_MOD_SPLIT | AFBC_FORMAT_MOD_SPARSE);
> > > > > +
> > > > > +  if (flags & MALI_GRALLOC_INTFMT_AFBC_WIDEBLK)
> > > > > +    features |= AFBC_FORMAT_MOD_BLOCK_SIZE_32x8;
> > > > > +
> > > > > +  if (flags & MALI_GRALLOC_INTFMT_AFBC_TILED_HEADERS)
> > > > > +    features |= AFBC_FORMAT_MOD_TILED;
> > > > > +
> > > > > +  if (features)
> > > > > +    return DRM_FORMAT_MOD_ARM_AFBC(features);
> > > > > +  else
> > > > > +    return 0;
> > > > > +}
> > > > > +
> > > > >  uint32_t DrmGenericImporter::DrmFormatToBitsPerPixel(uint32_t drm_format) {
> > > > >    switch (drm_format) {
> > > > >      case DRM_FORMAT_ARGB8888:
> > > > > @@ -101,6 +127,22 @@ uint32_t DrmGenericImporter::DrmFormatToBitsPerPixel(uint32_t drm_format) {
> > > > >    }
> > > > >  }
> > > > >
> > > > > +bool DrmGenericImporter::IsDrmFormatRgb(uint32_t drm_format) {
> > > > > +  switch (drm_format) {
> > > > > +    case DRM_FORMAT_ARGB8888:
> > > > > +    case DRM_FORMAT_XBGR8888:
> > > > > +    case DRM_FORMAT_ABGR8888:
> > > > > +    case DRM_FORMAT_BGR888:
> > > > > +    case DRM_FORMAT_BGR565:
> > > > > +      return true;
> > > > > +    case DRM_FORMAT_YVU420:
> > > > > +      return false;
> > > > > +    default:
> > > > > +      ALOGE("Unsupported format %u assuming rgb?", drm_format);
> > > > > +      return true;
> > > > > +  }
> > > > > +}
> > > > > +
> > > > >  int DrmGenericImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) {
> > > > >    gralloc_handle_t *gr_handle = gralloc_handle(handle);
> > > > >    if (!gr_handle)
> > > > > diff --git a/platformdrmgeneric.h b/platformdrmgeneric.h
> > > > > index 233ba55..43cb618 100644
> > > > > --- a/platformdrmgeneric.h
> > > > > +++ b/platformdrmgeneric.h
> > > > > @@ -36,6 +36,8 @@ class DrmGenericImporter : public Importer {
> > > > >    bool CanImportBuffer(buffer_handle_t handle) override;
> > > > >
> > > > >    uint32_t ConvertHalFormatToDrm(uint32_t hal_format);
> > > > > +  uint64_t ConvertGrallocFormatToDrmModifiers(uint64_t flags, bool is_rgb);
> > > > > +  bool IsDrmFormatRgb(uint32_t drm_format);
> > > > >    uint32_t DrmFormatToBitsPerPixel(uint32_t drm_format);
> > > > >
> > > > >   private:
> > > > > diff --git a/platformhisi.cpp b/platformhisi.cpp
> > > > > index 76fe1e7..1cb7e2c 100644
> > > > > --- a/platformhisi.cpp
> > > > > +++ b/platformhisi.cpp
> > > > > @@ -71,6 +71,9 @@ int HisiImporter::Init() {
> > > > >  }
> > > > >
> > > > >  int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) {
> > > > > +  bool is_rgb;
> > > > > +  uint64_t modifiers[4] = {0};
> > > > > +
> > > > >    memset(bo, 0, sizeof(hwc_drm_bo_t));
> > > > >
> > > > >    private_handle_t const *hnd = reinterpret_cast<private_handle_t const *>(
> > > > > @@ -94,6 +97,10 @@ int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) {
> > > > >    if (fmt < 0)
> > > > >      return fmt;
> > > > >
> > > > > +  is_rgb = IsDrmFormatRgb(fmt);
> > > > > +  modifiers[0] = ConvertGrallocFormatToDrmModifiers(hnd->internal_format,
> > > > > +                                                    is_rgb);
> > > > > +
> > > > >    bo->width = hnd->width;
> > > > >    bo->height = hnd->height;
> > > > >    bo->hal_format = hnd->req_format;
> > > > > @@ -129,8 +136,11 @@ int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) {
> > > > >        break;
> > > > >    }
> > > > >
> > > > > -  ret = drmModeAddFB2(drm_->fd(), bo->width, bo->height, bo->format,
> > > > > -                      bo->gem_handles, bo->pitches, bo->offsets, &bo->fb_id, 0);
> > > > > +  ret = drmModeAddFB2WithModifiers(drm_->fd(), bo->width, bo->height,
> > > > > +                                   bo->format, bo->gem_handles, bo->pitches,
> > > > > +                                   bo->offsets, modifiers, &bo->fb_id,
> > > > > +                                   modifiers[0] ? DRM_MODE_FB_MODIFIERS : 0);
> > > > > +
> > > > >    if (ret) {
> > > > >      ALOGE("could not create drm fb %d", ret);
> > > > >      return ret;
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@...ts.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > >
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > >
> > > --
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@...ts.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ