Linux Samsung SOC development
 help / color / mirror / Atom feed
From: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
To: Marek Szyprowski <m.szyprowski@samsung.com>,
	dri-devel@lists.freedesktop.org,
	linux-samsung-soc@vger.kernel.org
Cc: Enrico Weigelt <enrico.weigelt@gr13.net>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>
Subject: Re: [RFC 0/2] New feature: Framebuffer processors
Date: Mon, 22 Aug 2016 13:38:48 +0200	[thread overview]
Message-ID: <57BAE448.3040704@math.uni-bielefeld.de> (raw)
In-Reply-To: <45ba4f39-f47a-3cfe-c2d2-b0ea229353de@samsung.com>

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2016-08-22 11:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-22  9:44 [RFC 0/2] New feature: Framebuffer processors Marek Szyprowski
2016-08-22  9:44 ` [RFC 1/2] drm: add support for framebuffer processors Marek Szyprowski
2016-08-22  9:44 ` [RFC 2/2] drm/exynos: register rotator as fbproc instead of custom ipp framework Marek Szyprowski
2016-08-22  9:44 ` [RFC libdrm] add support for framebuffer processor (fbproc) objects Marek Szyprowski
2016-08-22  9:44 ` [RFC code example] example code for testing fbproc drivers Marek Szyprowski
2016-08-22  9:59 ` [RFC 0/2] New feature: Framebuffer processors Christian König
2016-08-22 14:59   ` Daniel Vetter
2016-08-22 15:23   ` Rob Clark
2016-08-22 15:30     ` Benjamin Gaignard
2016-08-23  9:41     ` Daniel Stone
2016-08-24 11:44       ` Inki Dae
2016-08-24 11:57         ` Daniel Vetter
2016-08-25  8:06           ` Inki Dae
2016-08-25  8:42             ` Daniel Vetter
2016-08-25 11:45               ` Inki Dae
2016-08-25 12:14                 ` Daniel Vetter
2016-08-30  5:52                   ` Inki Dae
2016-08-22 10:07 ` Tobias Jakobi
2016-08-22 10:50   ` Marek Szyprowski
2016-08-22 11:38     ` Tobias Jakobi [this message]
2016-08-22 20:05 ` Dave Airlie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57BAE448.3040704@math.uni-bielefeld.de \
    --to=tjakobi@math.uni-bielefeld.de \
    --cc=b.zolnierkie@samsung.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=enrico.weigelt@gr13.net \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=sw0312.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox