[RFC 0/2] New feature: Framebuffer processors

Mon Aug 22 11:38:48 UTC 2016

Hello Marek,

Marek Szyprowski wrote:
> Dear Tobias
> 
> 
> On 2016-08-22 12:07, Tobias Jakobi wrote:
>> Hey Marek,
>>
>> I had a quick look at the series and I really like the new approach.
>>
>> I was wondering about the following though. If I understand this
>> correctly I can only perform m2m operations on buffers which are
>> registered as framebuffers. Is this possible to weaken that requirements
>> such that arbitrary GEM objects can be used as input and output?
> 
> Thanks for you comment.
> 
> I'm open for discussion if the API should be based on framebuffers or
> GEM objects.
> 
> Initially I thought that GEM objects would be enough, but later I
> noticed that in such case user would need to provide at least width,
> height, stride, start offset and pixel format - all parameters that
> are already used to create framebuffer object. Operating on GEM
> buffers will also make support for images composed from multiple
> buffers (like separate GEM objects for luma/chroma parts in case of
> planar formats) a bit harder. Same about already introduced API for
> fb-modifiers. I just don't want to duplicate all of it in fbproc API.
yes, that makes perfectly sense. Passing the buffer parameters
(geometry, pixel format, etc.) each time is probably not a good (and
efficient) idea.

I'm still wondering if there can't arise a situation where I simply
can't register a buffer as framebuffer.

I'm specifically looking at internal_framebuffer_create() and the driver
specific fb_create(). Now internal_framebuffer_create() itself only does
some minimal checking, but e.g. it does check against certain
minimum/maximum geometry parameters. How do we know that these
parameters match the ones for the block that performs the m2m operations?

I could image that a block could perform scaling operations on buffers
with a much larger geometry than the core allows to bind as
framebuffers. So if I have such a buffer with large geometry I might
want to scale it down, so that I can display it. But that's not possible
since I can't even bind the src as fb.

Does that make sense?

With best wishes,
Tobias

> Operating on framebuffer objects also helps to reduce errors in
> userspace. One can already queue the result of processing to the
> display hardware and this way avoid common issues related to debugging
> why the processed image is not displayed correctly due to incorrectly
> defined pitch/fourcc/start offset/etc. This is however not really a
> strong advantage of framebuffers.
> 
> 
>>
>> Anyway, great work!
>>
>> With best wishes,
>> Tobias
>>
>>
>> Marek Szyprowski wrote:
>>> Dear all,
>>>
>>> This is the initial proposal for extending DRM API with generic
>>> support for
>>> hardware modules, which can be used for processing image data from
>>> the one
>>> memory buffer to another. Typical memory-to-memory operations are:
>>> rotation, scaling, colour space conversion or mix of them. In this
>>> proposal
>>> I named such hardware modules a framebuffer processors.
>>>
>>> Embedded SoCs are known to have a number of hardware blocks, which
>>> perform
>>> such operations. They can be used in paralel to the main GPU module to
>>> offload CPU from processing grapics or video data. One of example use of
>>> such modules is implementing video overlay, which usually requires color
>>> space conversion from NV12 (or similar) to RGB32 color space and
>>> scaling to
>>> target window size.
>>>
>>> Till now there was no generic, hardware independent API for
>>> performing such
>>> operations. Exynos DRM driver has its own custom extension called IPP
>>> (Image Post Processing), but frankly speaking, it is over-engineered
>>> and not
>>> really used in open-source. I didn't indentify similar API in other DRM
>>> drivers, besides those which expose complete support for the whole GPU.
>>>
>>> However, the need for commmon API has been already mentioned on the
>>> mailing
>>> list. Here are some example threads:
>>> 1. "RFC: hardware accelerated bitblt using dma engine"
>>> http://www.spinics.net/lists/dri-devel/msg114250.html
>>> 2. "[PATCH 00/25] Exynos DRM: new life of IPP (Image Post Processing)
>>> subsystem"
>>> https://lists.freedesktop.org/archives/dri-devel/2015-November/094115.html
>>>
>>> https://lists.freedesktop.org/archives/dri-devel/2015-November/094533.html
>>>
>>>
>>> The proposed API is heavily inspired by atomic KMS approach - it is also
>>> based on DRM objects and their properties. A new DRM object is
>>> introduced:
>>> framebuffer processor (called fbproc for convenience). Such fbproc
>>> objects
>>> have a set of standard DRM properties, which describes the operation
>>> to be
>>> performed by respective hardware module. In typical case those
>>> properties
>>> are a source fb id and rectangle (x, y, width, height) and
>>> destination fb
>>> id and rectangle. Optionally a rotation property can be also
>>> specified if
>>> supported by the given hardware. To perform an operation on image data,
>>> userspace provides a set of properties and their values for given fbproc
>>> object in a similar way as object and properties are provided for
>>> performing atomic page flip / mode setting.
>>>
>>> The proposed API consists of the 3 new ioctls:
>>> - DRM_IOCTL_MODE_GETFBPROCRESOURCES: to enumerate all available fbproc
>>>    objects,
>>> - DRM_IOCTL_MODE_GETFBPROC: to query capabilities of given fbproc
>>> object,
>>> - DRM_IOCTL_MODE_FBPROC: to perform operation described by given
>>> property
>>>    set.
>>>
>>> The proposed API is extensible. Drivers can attach their own, custom
>>> properties to add support for more advanced picture processing (for
>>> example
>>> blending).
>>>
>>> Please note that this API is intended to be used for simple
>>> memory-to-memory
>>> image processing hardware not the full-blown GPU blitters, which usually
>>> have more features. Typically blitters provides much more operations
>>> beside
>>> simple pixel copying and operate best if its command queue is
>>> controlled from
>>> respective dedicated code in userspace.
>>>
>>> The patchset consist of 4 parts:
>>> 1. generic code for DRM core for handling fbproc objects and ioctls
>>> 2. example, quick conversion of Exynos Rotator driver to fbproc API
>>> 3. libdrm extensions for handling fbproc objects
>>> 4. simple example of userspace code for performing 180 degree
>>> rotation of the
>>>     framebuffer
>>>
>>> Patches were tested on Exynos 4412-based Odroid U3 board, on top
>>> of Linux v4.8-rc1 kernel.
>>>
>>> TODO:
>>> 1. agree on the API shape
>>> 2. add more documentation, especially to the kernel docs
>>> 3. add more userspace examples
>>>
>>> Best regards
>>> Marek Szyprowski
>>> Samsung R&D Institute Poland
>>>
>>>
>>> Marek Szyprowski (2):
>>>    drm: add support for framebuffer processor objects
>>>    drm/exynos: register rotator as fbproc instead of custom ipp
>>> framework
>>>
>>>   drivers/gpu/drm/Makefile                    |   3 +-
>>>   drivers/gpu/drm/drm_atomic.c                |   5 +
>>>   drivers/gpu/drm/drm_crtc.c                  |   6 +
>>>   drivers/gpu/drm/drm_crtc_internal.h         |  12 +
>>>   drivers/gpu/drm/drm_fbproc.c                | 754
>>> ++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/drm_ioctl.c                 |   3 +
>>>   drivers/gpu/drm/exynos/Kconfig              |   1 -
>>>   drivers/gpu/drm/exynos/exynos_drm_drv.c     |   3 +-
>>>   drivers/gpu/drm/exynos/exynos_drm_rotator.c | 353 +++++++------
>>>   drivers/gpu/drm/exynos/exynos_drm_rotator.h |  19 -
>>>   include/drm/drmP.h                          |  10 +
>>>   include/drm/drm_crtc.h                      | 211 ++++++++
>>>   include/drm/drm_irq.h                       |  14 +
>>>   include/uapi/drm/drm.h                      |  13 +
>>>   include/uapi/drm/drm_mode.h                 |  39 ++
>>>   15 files changed, 1263 insertions(+), 183 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/drm_fbproc.c
>>>   delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_rotator.h
>>>
>>
>>
> 
> Best regards