Re: [PATCH v2 2/4] drm/gem: Update client API to use struct drm_gem_membuf
Hi
Am 13.08.20 um 12:26 schrieb Daniel Vetter:
> On Thu, Aug 06, 2020 at 10:52:37AM +0200, Thomas Zimmermann wrote:
>> GEM's vmap interface now wraps memory pointers in struct drm_gem_membuf.
>> The structure represents a pointer into the framebuffer, which is either
>> in I/O memory or in system memory. The structure contains a flag that
>> distinguishes these cases.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/drm_client.c| 25 -
>> drivers/gpu/drm/drm_fb_helper.c | 18 +-
>> drivers/gpu/drm/drm_gem.c | 19 +++
>> drivers/gpu/drm/drm_internal.h | 5 +++--
>> drivers/gpu/drm/drm_prime.c | 16 ++--
>> include/drm/drm_client.h| 7 ---
>> include/drm/drm_device.h| 26 ++
>> 7 files changed, 75 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index 495f47d23d87..0359b82928c1 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -234,7 +234,7 @@ static void drm_client_buffer_delete(struct
>> drm_client_buffer *buffer)
>> {
>> struct drm_device *dev = buffer->client->dev;
>>
>> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
>> +drm_gem_vunmap(buffer->gem, &buffer->membuf);
>>
>> if (buffer->gem)
>> drm_gem_object_put(buffer->gem);
>> @@ -302,12 +302,13 @@ drm_client_buffer_create(struct drm_client_dev
>> *client, u32 width, u32 height, u
>> * Returns:
>> * The mapped memory's address
>> */
>> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
>> +const struct drm_gem_membuf *
>> +drm_client_buffer_vmap(struct drm_client_buffer *buffer)
>> {
>> -void *vaddr;
>> +int ret;
>>
>> -if (buffer->vaddr)
>> -return buffer->vaddr;
>> +if (buffer->membuf.vaddr)
>> +return &buffer->membuf;
>>
>> /*
>> * FIXME: The dependency on GEM here isn't required, we could
>> @@ -317,13 +318,11 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
>> *buffer)
>> * fd_install step out of the driver backend hooks, to make that
>> * final step optional for internal users.
>> */
>> -vaddr = drm_gem_vmap(buffer->gem);
>> -if (IS_ERR(vaddr))
>> -return vaddr;
>> -
>> -buffer->vaddr = vaddr;
>> +ret = drm_gem_vmap(buffer->gem, &buffer->membuf);
>> +if (ret)
>> +return ERR_PTR(ret);
>>
>> -return vaddr;
>> +return &buffer->membuf;
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vmap);
>>
>> @@ -337,8 +336,8 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
>> */
>> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>> {
>> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
>> -buffer->vaddr = NULL;
>> +drm_gem_vunmap(buffer->gem, &buffer->membuf);
>> +buffer->membuf.vaddr = NULL;
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 8697554ccd41..da24874247e7 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
>> drm_fb_helper *fb_helper,
>> unsigned int cpp = fb->format->cpp[0];
>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
>> void *src = fb_helper->fbdev->screen_buffer + offset;
>> -void *dst = fb_helper->buffer->vaddr + offset;
>> +void *dst = fb_helper->buffer->membuf.vaddr + offset;
>> size_t len = (clip->x2 - clip->x1) * cpp;
>> unsigned int y;
>>
>> @@ -416,7 +416,7 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>> struct drm_clip_rect *clip = &helper->dirty_clip;
>> struct drm_clip_rect clip_copy;
>> unsigned long flags;
>> -void *vaddr;
>> +const struct drm_gem_membuf *buf;
>>
>> spin_lock_irqsave(&helper->dirty_lock, flags);
>> clip_copy = *clip;
>> @@ -429,8 +429,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>>
>> /* Generic fbdev uses a shadow buffer */
>> if (helper->buffer) {
>> -vaddr = drm_client_buffer_vmap(helper->buffer);
>> -if (IS_ERR(vaddr))
>> +buf = drm_client_buffer_vmap(helper->buffer);
>> +if (IS_ERR(buf))
>> return;
>> drm_fb_helper_dirty_blit_real(helper, &clip_copy);
>> }
>> @@ -2076,7 +2076,7 @@ static int drm_fb_helper_generic_probe(struct
>> drm_fb_helper *fb_helper,
>> struct drm_framebuffer *fb;
>> struct fb_info *fbi;
>> u32 format;
>> -void *vaddr;
>> +const struct drm_gem_membuf *membuf;
>>
>> drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
>> sizes->surface_width, sizes->surface_height,
>> @@
Re: [PATCH v2 2/4] drm/gem: Update client API to use struct drm_gem_membuf
On Thu, Aug 13, 2020 at 12:46 PM Thomas Zimmermann wrote:
>
> Hi
>
> Am 13.08.20 um 12:26 schrieb Daniel Vetter:
> > On Thu, Aug 06, 2020 at 10:52:37AM +0200, Thomas Zimmermann wrote:
> >> GEM's vmap interface now wraps memory pointers in struct drm_gem_membuf.
> >> The structure represents a pointer into the framebuffer, which is either
> >> in I/O memory or in system memory. The structure contains a flag that
> >> distinguishes these cases.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> ---
> >> drivers/gpu/drm/drm_client.c| 25 -
> >> drivers/gpu/drm/drm_fb_helper.c | 18 +-
> >> drivers/gpu/drm/drm_gem.c | 19 +++
> >> drivers/gpu/drm/drm_internal.h | 5 +++--
> >> drivers/gpu/drm/drm_prime.c | 16 ++--
> >> include/drm/drm_client.h| 7 ---
> >> include/drm/drm_device.h| 26 ++
> >> 7 files changed, 75 insertions(+), 41 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> >> index 495f47d23d87..0359b82928c1 100644
> >> --- a/drivers/gpu/drm/drm_client.c
> >> +++ b/drivers/gpu/drm/drm_client.c
> >> @@ -234,7 +234,7 @@ static void drm_client_buffer_delete(struct
> >> drm_client_buffer *buffer)
> >> {
> >> struct drm_device *dev = buffer->client->dev;
> >>
> >> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
> >> +drm_gem_vunmap(buffer->gem, &buffer->membuf);
> >>
> >> if (buffer->gem)
> >> drm_gem_object_put(buffer->gem);
> >> @@ -302,12 +302,13 @@ drm_client_buffer_create(struct drm_client_dev
> >> *client, u32 width, u32 height, u
> >> * Returns:
> >> * The mapped memory's address
> >> */
> >> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> >> +const struct drm_gem_membuf *
> >> +drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> >> {
> >> -void *vaddr;
> >> +int ret;
> >>
> >> -if (buffer->vaddr)
> >> -return buffer->vaddr;
> >> +if (buffer->membuf.vaddr)
> >> +return &buffer->membuf;
> >>
> >> /*
> >> * FIXME: The dependency on GEM here isn't required, we could
> >> @@ -317,13 +318,11 @@ void *drm_client_buffer_vmap(struct
> >> drm_client_buffer *buffer)
> >> * fd_install step out of the driver backend hooks, to make that
> >> * final step optional for internal users.
> >> */
> >> -vaddr = drm_gem_vmap(buffer->gem);
> >> -if (IS_ERR(vaddr))
> >> -return vaddr;
> >> -
> >> -buffer->vaddr = vaddr;
> >> +ret = drm_gem_vmap(buffer->gem, &buffer->membuf);
> >> +if (ret)
> >> +return ERR_PTR(ret);
> >>
> >> -return vaddr;
> >> +return &buffer->membuf;
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vmap);
> >>
> >> @@ -337,8 +336,8 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> >> */
> >> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> >> {
> >> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
> >> -buffer->vaddr = NULL;
> >> +drm_gem_vunmap(buffer->gem, &buffer->membuf);
> >> +buffer->membuf.vaddr = NULL;
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vunmap);
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index 8697554ccd41..da24874247e7 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> >> drm_fb_helper *fb_helper,
> >> unsigned int cpp = fb->format->cpp[0];
> >> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >> void *src = fb_helper->fbdev->screen_buffer + offset;
> >> -void *dst = fb_helper->buffer->vaddr + offset;
> >> +void *dst = fb_helper->buffer->membuf.vaddr + offset;
> >> size_t len = (clip->x2 - clip->x1) * cpp;
> >> unsigned int y;
> >>
> >> @@ -416,7 +416,7 @@ static void drm_fb_helper_dirty_work(struct
> >> work_struct *work)
> >> struct drm_clip_rect *clip = &helper->dirty_clip;
> >> struct drm_clip_rect clip_copy;
> >> unsigned long flags;
> >> -void *vaddr;
> >> +const struct drm_gem_membuf *buf;
> >>
> >> spin_lock_irqsave(&helper->dirty_lock, flags);
> >> clip_copy = *clip;
> >> @@ -429,8 +429,8 @@ static void drm_fb_helper_dirty_work(struct
> >> work_struct *work)
> >>
> >> /* Generic fbdev uses a shadow buffer */
> >> if (helper->buffer) {
> >> -vaddr = drm_client_buffer_vmap(helper->buffer);
> >> -if (IS_ERR(vaddr))
> >> +buf = drm_client_buffer_vmap(helper->buffer);
> >> +if (IS_ERR(buf))
> >> return;
> >> drm_fb_helper_dirty_blit_real(helper, &clip_copy);
> >> }
> >> @@ -2076,7 +2076,7 @@ static int drm_fb_helper_generic_probe(struct
> >> drm_fb_helper *fb_helper,
> >> stru
Re: [PATCH v2 2/4] drm/gem: Update client API to use struct drm_gem_membuf
Hi
Am 13.08.20 um 12:26 schrieb Daniel Vetter:
> On Thu, Aug 06, 2020 at 10:52:37AM +0200, Thomas Zimmermann wrote:
>> GEM's vmap interface now wraps memory pointers in struct drm_gem_membuf.
>> The structure represents a pointer into the framebuffer, which is either
>> in I/O memory or in system memory. The structure contains a flag that
>> distinguishes these cases.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/drm_client.c| 25 -
>> drivers/gpu/drm/drm_fb_helper.c | 18 +-
>> drivers/gpu/drm/drm_gem.c | 19 +++
>> drivers/gpu/drm/drm_internal.h | 5 +++--
>> drivers/gpu/drm/drm_prime.c | 16 ++--
>> include/drm/drm_client.h| 7 ---
>> include/drm/drm_device.h| 26 ++
>> 7 files changed, 75 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index 495f47d23d87..0359b82928c1 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -234,7 +234,7 @@ static void drm_client_buffer_delete(struct
>> drm_client_buffer *buffer)
>> {
>> struct drm_device *dev = buffer->client->dev;
>>
>> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
>> +drm_gem_vunmap(buffer->gem, &buffer->membuf);
>>
>> if (buffer->gem)
>> drm_gem_object_put(buffer->gem);
>> @@ -302,12 +302,13 @@ drm_client_buffer_create(struct drm_client_dev
>> *client, u32 width, u32 height, u
>> * Returns:
>> * The mapped memory's address
>> */
>> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
>> +const struct drm_gem_membuf *
>> +drm_client_buffer_vmap(struct drm_client_buffer *buffer)
>> {
>> -void *vaddr;
>> +int ret;
>>
>> -if (buffer->vaddr)
>> -return buffer->vaddr;
>> +if (buffer->membuf.vaddr)
>> +return &buffer->membuf;
>>
>> /*
>> * FIXME: The dependency on GEM here isn't required, we could
>> @@ -317,13 +318,11 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
>> *buffer)
>> * fd_install step out of the driver backend hooks, to make that
>> * final step optional for internal users.
>> */
>> -vaddr = drm_gem_vmap(buffer->gem);
>> -if (IS_ERR(vaddr))
>> -return vaddr;
>> -
>> -buffer->vaddr = vaddr;
>> +ret = drm_gem_vmap(buffer->gem, &buffer->membuf);
>> +if (ret)
>> +return ERR_PTR(ret);
>>
>> -return vaddr;
>> +return &buffer->membuf;
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vmap);
>>
>> @@ -337,8 +336,8 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
>> */
>> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>> {
>> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
>> -buffer->vaddr = NULL;
>> +drm_gem_vunmap(buffer->gem, &buffer->membuf);
>> +buffer->membuf.vaddr = NULL;
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 8697554ccd41..da24874247e7 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
>> drm_fb_helper *fb_helper,
>> unsigned int cpp = fb->format->cpp[0];
>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
>> void *src = fb_helper->fbdev->screen_buffer + offset;
>> -void *dst = fb_helper->buffer->vaddr + offset;
>> +void *dst = fb_helper->buffer->membuf.vaddr + offset;
>> size_t len = (clip->x2 - clip->x1) * cpp;
>> unsigned int y;
>>
>> @@ -416,7 +416,7 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>> struct drm_clip_rect *clip = &helper->dirty_clip;
>> struct drm_clip_rect clip_copy;
>> unsigned long flags;
>> -void *vaddr;
>> +const struct drm_gem_membuf *buf;
>>
>> spin_lock_irqsave(&helper->dirty_lock, flags);
>> clip_copy = *clip;
>> @@ -429,8 +429,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>>
>> /* Generic fbdev uses a shadow buffer */
>> if (helper->buffer) {
>> -vaddr = drm_client_buffer_vmap(helper->buffer);
>> -if (IS_ERR(vaddr))
>> +buf = drm_client_buffer_vmap(helper->buffer);
>> +if (IS_ERR(buf))
>> return;
>> drm_fb_helper_dirty_blit_real(helper, &clip_copy);
>> }
>> @@ -2076,7 +2076,7 @@ static int drm_fb_helper_generic_probe(struct
>> drm_fb_helper *fb_helper,
>> struct drm_framebuffer *fb;
>> struct fb_info *fbi;
>> u32 format;
>> -void *vaddr;
>> +const struct drm_gem_membuf *membuf;
>>
>> drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
>> sizes->surface_width, sizes->surface_height,
>> @@
Re: [PATCH v2 2/4] drm/gem: Update client API to use struct drm_gem_membuf
On Thu, Aug 06, 2020 at 10:52:37AM +0200, Thomas Zimmermann wrote:
> GEM's vmap interface now wraps memory pointers in struct drm_gem_membuf.
> The structure represents a pointer into the framebuffer, which is either
> in I/O memory or in system memory. The structure contains a flag that
> distinguishes these cases.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_client.c| 25 -
> drivers/gpu/drm/drm_fb_helper.c | 18 +-
> drivers/gpu/drm/drm_gem.c | 19 +++
> drivers/gpu/drm/drm_internal.h | 5 +++--
> drivers/gpu/drm/drm_prime.c | 16 ++--
> include/drm/drm_client.h| 7 ---
> include/drm/drm_device.h| 26 ++
> 7 files changed, 75 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 495f47d23d87..0359b82928c1 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -234,7 +234,7 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap(buffer->gem, buffer->vaddr);
> + drm_gem_vunmap(buffer->gem, &buffer->membuf);
>
> if (buffer->gem)
> drm_gem_object_put(buffer->gem);
> @@ -302,12 +302,13 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> * Returns:
> * The mapped memory's address
> */
> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> +const struct drm_gem_membuf *
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> {
> - void *vaddr;
> + int ret;
>
> - if (buffer->vaddr)
> - return buffer->vaddr;
> + if (buffer->membuf.vaddr)
> + return &buffer->membuf;
>
> /*
>* FIXME: The dependency on GEM here isn't required, we could
> @@ -317,13 +318,11 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
> *buffer)
>* fd_install step out of the driver backend hooks, to make that
>* final step optional for internal users.
>*/
> - vaddr = drm_gem_vmap(buffer->gem);
> - if (IS_ERR(vaddr))
> - return vaddr;
> -
> - buffer->vaddr = vaddr;
> + ret = drm_gem_vmap(buffer->gem, &buffer->membuf);
> + if (ret)
> + return ERR_PTR(ret);
>
> - return vaddr;
> + return &buffer->membuf;
> }
> EXPORT_SYMBOL(drm_client_buffer_vmap);
>
> @@ -337,8 +336,8 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> */
> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> {
> - drm_gem_vunmap(buffer->gem, buffer->vaddr);
> - buffer->vaddr = NULL;
> + drm_gem_vunmap(buffer->gem, &buffer->membuf);
> + buffer->membuf.vaddr = NULL;
> }
> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8697554ccd41..da24874247e7 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> drm_fb_helper *fb_helper,
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->vaddr + offset;
> + void *dst = fb_helper->buffer->membuf.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> @@ -416,7 +416,7 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> struct drm_clip_rect *clip = &helper->dirty_clip;
> struct drm_clip_rect clip_copy;
> unsigned long flags;
> - void *vaddr;
> + const struct drm_gem_membuf *buf;
>
> spin_lock_irqsave(&helper->dirty_lock, flags);
> clip_copy = *clip;
> @@ -429,8 +429,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
>
> /* Generic fbdev uses a shadow buffer */
> if (helper->buffer) {
> - vaddr = drm_client_buffer_vmap(helper->buffer);
> - if (IS_ERR(vaddr))
> + buf = drm_client_buffer_vmap(helper->buffer);
> + if (IS_ERR(buf))
> return;
> drm_fb_helper_dirty_blit_real(helper, &clip_copy);
> }
> @@ -2076,7 +2076,7 @@ static int drm_fb_helper_generic_probe(struct
> drm_fb_helper *fb_helper,
> struct drm_framebuffer *fb;
> struct fb_info *fbi;
> u32 format;
> - void *vaddr;
> + const struct drm_gem_membuf *membuf;
>
> drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
> sizes->surface_width, sizes->surface_height,
> @@ -2112,11 +2112,11 @@ static int drm_fb_helper_generic_probe(struct
> drm_fb_helper *fb_helper,
> fb_de
