All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.