dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Thomas Zimmermann <tzimmermann@suse.de>,
	oushixiong <oushixiong@kylinos.cn>,
	Dave Airlie <airlied@redhat.com>
Cc: David Airlie <airlied@linux.ie>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linaro-mm-sig@lists.linaro.org,
	Sumit Semwal <sumit.semwal@linaro.org>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v3] drm/ast: add dmabuf/prime buffer sharing support
Date: Wed, 7 Sep 2022 10:10:52 +0200	[thread overview]
Message-ID: <ee27c832-a1fd-bc93-9f1b-33f828195e83@amd.com> (raw)
In-Reply-To: <5bc77d8f-928c-1d41-9b3c-eaad1bf3cfff@suse.de>

Hi Thomas,

I was wondering pretty much the same thing, but then thought that this 
might be the first step to direct scanout from DMA-bufs.

If this isn't the case then I to see this rather critically since that 
functionality belongs into userspace.

Regards,
Christian.

Am 07.09.22 um 09:50 schrieb Thomas Zimmermann:
> Hi,
>
> on a more general note, let me say that your patch doesn't seem to fit 
> the ideas of how buffer sharing is supposed to work. Your patch does 
> the BMC screen update 'behind the scenes.'
>
> Shouldn't userspace set up the DRM state for mirroring the output of 
> the discrete card to the BMC?
>
> Best regards
> Thomas
>
> Am 01.09.22 um 14:44 schrieb oushixiong:
>>
>> This patch adds ast specific codes for DRM prime feature, this is to
>> allow for offloading of rending in one direction and outputs in other.
>>
>> This patch is designed to solve the problem that the AST is not 
>> displayed
>> when the server plug in a discrete graphics card at the same time.
>> We call the dirty callback function to copy the rendering results of the
>> discrete graphics card to the ast side by dma-buf.
>>
>> v1->v2:
>>    - Fix the comment.
>> v2->v3:
>>    - we remove the gem_prime_import_sg_table callback and use the
>>      gem_prime_import callback, because it just map and access the 
>> buffer
>>      with the CPU. and do not to pin the buffer.
>>
>> Signed-off-by: oushixiong <oushixiong@kylinos.cn>
>> Acked-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/ast/ast_drv.c  |  27 +++++++
>>   drivers/gpu/drm/ast/ast_mode.c | 125 ++++++++++++++++++++++++++++++++-
>>   2 files changed, 151 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.c 
>> b/drivers/gpu/drm/ast/ast_drv.c
>> index 7465c4f0156a..fd3c4bad2eb4 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.c
>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>> @@ -28,6 +28,7 @@
>>     #include <linux/module.h>
>>   #include <linux/pci.h>
>> +#include <linux/dma-buf.h>
>>     #include <drm/drm_aperture.h>
>>   #include <drm/drm_atomic_helper.h>
>> @@ -50,6 +51,29 @@ module_param_named(modeset, ast_modeset, int, 0400);
>>     DEFINE_DRM_GEM_FOPS(ast_fops);
>>   +static struct drm_gem_object *ast_gem_prime_import(struct 
>> drm_device *dev,
>> +                        struct dma_buf *dma_buf)
>> +{
>> +    struct drm_gem_vram_object *gbo;
>> +
>> +    gbo = drm_gem_vram_of_gem(dma_buf->priv);
>> +    if (gbo->bo.base.dev == dev) {
>> +        /*
>> +         * Importing dmabuf exported from out own gem increases
>> +         * refcount on gem itself instead of f_count of dmabuf.
>> +         */
>> +        drm_gem_object_get(&gbo->bo.base);
>> +        return &gbo->bo.base;
>> +    }
>> +
>> +    gbo = drm_gem_vram_create(dev, dma_buf->size, 0);
>> +    if (IS_ERR(gbo))
>> +        return NULL;
>> +
>> +    get_dma_buf(dma_buf);
>> +    return &gbo->bo.base;
>> +}
>> +
>>   static const struct drm_driver ast_driver = {
>>       .driver_features = DRIVER_ATOMIC |
>>                  DRIVER_GEM |
>> @@ -63,6 +87,9 @@ static const struct drm_driver ast_driver = {
>>       .minor = DRIVER_MINOR,
>>       .patchlevel = DRIVER_PATCHLEVEL,
>>   +    .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> +    .gem_prime_import = ast_gem_prime_import,
>> +
>>       DRM_GEM_VRAM_DRIVER
>>   };
>>   diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>> b/drivers/gpu/drm/ast/ast_mode.c
>> index 45b56b39ad47..65a4342c5622 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -48,6 +48,8 @@
>>   #include "ast_drv.h"
>>   #include "ast_tables.h"
>>   +MODULE_IMPORT_NS(DMA_BUF);
>> +
>>   static inline void ast_load_palette_index(struct ast_private *ast,
>>                        u8 index, u8 red, u8 green,
>>                        u8 blue)
>> @@ -1535,8 +1537,129 @@ static const struct 
>> drm_mode_config_helper_funcs ast_mode_config_helper_funcs =
>>       .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>>   };
>>   +static int ast_handle_damage(struct drm_framebuffer *fb, int x, 
>> int y,
>> +                    int width, int height)
>> +{
>> +    struct drm_gem_vram_object *dst_bo = NULL;
>> +    void *dst = NULL;
>> +    int ret = 0, i;
>> +    unsigned long offset = 0;
>> +    bool unmap = false;
>> +    unsigned int bytesPerPixel;
>> +    struct iosys_map map;
>> +    struct iosys_map dmabuf_map;
>> +
>> +    bytesPerPixel = fb->format->cpp[0];
>> +
>> +    if (!fb->obj[0]->dma_buf)
>> +        return -EINVAL;
>> +
>> +    if (!fb->obj[0]->dma_buf->vmap_ptr.vaddr) {
>> +        ret = dma_buf_vmap(fb->obj[0]->dma_buf, &dmabuf_map);
>> +        if (ret)
>> +            return ret;
>> +    } else
>> +        dmabuf_map.vaddr = fb->obj[0]->dma_buf->vmap_ptr.vaddr;
>> +
>> +    dst_bo = drm_gem_vram_of_gem(fb->obj[0]);
>> +
>> +    ret = drm_gem_vram_pin(dst_bo, 0);
>> +    if (ret) {
>> +        DRM_ERROR("ast_bo_pin failed\n");
>> +        return ret;
>> +    }
>> +
>> +    if (!dst_bo->map.vaddr) {
>> +        ret = drm_gem_vram_vmap(dst_bo, &map);
>> +        if (ret) {
>> +            drm_gem_vram_unpin(dst_bo);
>> +            DRM_ERROR("failed to vmap fbcon\n");
>> +            return ret;
>> +        }
>> +        unmap = true;
>> +    }
>> +    dst = dst_bo->map.vaddr;
>> +
>> +    for (i = y; i < y + height; i++) {
>> +        offset = i * fb->pitches[0] + (x * bytesPerPixel);
>> +        memcpy_toio(dst + offset, dmabuf_map.vaddr + offset,
>> +            width * bytesPerPixel);
>> +    }
>> +
>> +    if (unmap)
>> +        drm_gem_vram_vunmap(dst_bo, &map);
>> +
>> +    drm_gem_vram_unpin(dst_bo);
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +static int ast_user_framebuffer_dirty(struct drm_framebuffer *fb,
>> +                struct drm_file *file,
>> +                unsigned int flags,
>> +                unsigned int color,
>> +                struct drm_clip_rect *clips,
>> +                unsigned int num_clips)
>> +{
>> +    int i, ret = 0;
>> +
>> +    drm_modeset_lock_all(fb->dev);
>> +    if (fb->obj[0]->dma_buf) {
>> +        ret = dma_buf_begin_cpu_access(fb->obj[0]->dma_buf,
>> +                DMA_FROM_DEVICE);
>> +        if (ret)
>> +            goto unlock;
>> +    }
>> +
>> +    for (i = 0; i < num_clips; i++) {
>> +        ret = ast_handle_damage(fb, clips[i].x1, clips[i].y1,
>> +                clips[i].x2 - clips[i].x1, clips[i].y2 - clips[i].y1);
>> +        if (ret)
>> +            break;
>> +    }
>> +
>> +    if (fb->obj[0]->dma_buf) {
>> +        dma_buf_end_cpu_access(fb->obj[0]->dma_buf,
>> +                DMA_FROM_DEVICE);
>> +    }
>> +
>> +unlock:
>> +    drm_modeset_unlock_all(fb->dev);
>> +
>> +    return ret;
>> +}
>> +
>> +static void ast_user_framebuffer_destroy(struct drm_framebuffer *fb)
>> +{
>> +    struct iosys_map dmabuf_map;
>> +
>> +    if (fb->obj[0]->dma_buf) {
>> +        dmabuf_map.is_iomem = fb->obj[0]->dma_buf->vmap_ptr.is_iomem;
>> +        dmabuf_map.vaddr = fb->obj[0]->dma_buf->vmap_ptr.vaddr;
>> +        if (dmabuf_map.vaddr)
>> +            dma_buf_vunmap(fb->obj[0]->dma_buf, &dmabuf_map);
>> +    }
>> +
>> +    drm_gem_fb_destroy(fb);
>> +}
>> +
>> +static const struct drm_framebuffer_funcs ast_gem_fb_funcs_dirtyfb = {
>> +    .destroy    = ast_user_framebuffer_destroy,
>> +    .create_handle    = drm_gem_fb_create_handle,
>> +    .dirty        = ast_user_framebuffer_dirty,
>> +};
>> +
>> +static struct drm_framebuffer *
>> +ast_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file 
>> *file,
>> +                const struct drm_mode_fb_cmd2 *mode_cmd)
>> +{
>> +    return drm_gem_fb_create_with_funcs(dev, file, mode_cmd,
>> +                    &ast_gem_fb_funcs_dirtyfb);
>> +}
>> +
>>   static const struct drm_mode_config_funcs ast_mode_config_funcs = {
>> -    .fb_create = drm_gem_fb_create,
>> +    .fb_create = ast_gem_fb_create_with_dirty,
>>       .mode_valid = drm_vram_helper_mode_valid,
>>       .atomic_check = drm_atomic_helper_check,
>>       .atomic_commit = drm_atomic_helper_commit,
>>
>>
>> Content-type: Text/plain
>>
>> No virus found
>>         Checked by Hillstone Network AntiVirus
>


  reply	other threads:[~2022-09-07  8:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-01 12:44 [PATCH v3] drm/ast: add dmabuf/prime buffer sharing support oushixiong
2022-09-05  9:19 ` Thomas Zimmermann
2022-09-07  7:50 ` Thomas Zimmermann
2022-09-07  8:10   ` Christian König [this message]
2022-09-07  9:40     ` Thomas Zimmermann
2022-09-07 10:14       ` oushixiong
  -- strict thread matches above, loose matches on Subject: below --
2022-08-29  5:59 oushixiong

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=ee27c832-a1fd-bc93-9f1b-33f828195e83@amd.com \
    --to=christian.koenig@amd.com \
    --cc=airlied@linux.ie \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=oushixiong@kylinos.cn \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    /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;
as well as URLs for NNTP newsgroup(s).