Re: [PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Den 23.11.2022 09.39, skrev Thomas Zimmermann:
> (cc: danvet)
> 
> Hi
> 
> Am 22.11.22 um 21:58 schrieb Noralf Trønnes via B4 Submission Endpoint:
>> From: Noralf Trønnes <noralf@xxxxxxxxxxx>
>>
>> Complete the shadow fb access functions by also preparing imported
>> buffers
>> for CPU access. Update the affected drivers that currently use
>> drm_gem_fb_begin_cpu_access().
>>
>> Through this change the following SHMEM drivers will now also make sure
>> their imported buffers are prepared for CPU access: cirrus, hyperv,
>> mgag200, vkms
> 
> I had a similar patch recently, but Daniel shot it down. AFAIR
> begin_cpu_access *somehow* interferes with *something* and that can
> leads to *problems.*  Sorry that's the best I remember. Daniel should
> know. :D
> 

I loved this explanation, it gave me a good laugh :)

Yeah, I wondered why you hadn't done it, but assumed you just hadn't
gotten atround to it yet and if there were other reasons I would be told :)

Noralf.

> Best regards
> Thomas
> 
>>
>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
>> Cc: Dave Airlie <airlied@xxxxxxxxxx>
>> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
>> ---
>>   drivers/gpu/drm/drm_gem_atomic_helper.c | 13 ++++++++++++-
>>   drivers/gpu/drm/solomon/ssd130x.c       | 10 +---------
>>   drivers/gpu/drm/tiny/gm12u320.c         | 10 +---------
>>   drivers/gpu/drm/tiny/ofdrm.c            | 10 ++--------
>>   drivers/gpu/drm/tiny/simpledrm.c        | 10 ++--------
>>   drivers/gpu/drm/udl/udl_modeset.c       | 11 ++---------
>>   6 files changed, 20 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c
>> b/drivers/gpu/drm/drm_gem_atomic_helper.c
>> index e42800718f51..0eef4bb30d25 100644
>> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
>> @@ -368,6 +368,7 @@ EXPORT_SYMBOL(drm_gem_reset_shadow_plane);
>>    * maps all buffer objects of the plane's framebuffer into kernel
>> address
>>    * space and stores them in struct &drm_shadow_plane_state.map. The
>> first data
>>    * bytes are available in struct &drm_shadow_plane_state.data.
>> + * It also prepares imported buffers for CPU access.
>>    *
>>    * See drm_gem_end_shadow_fb_access() for cleanup.
>>    *
>> @@ -378,11 +379,20 @@ int drm_gem_begin_shadow_fb_access(struct
>> drm_plane *plane, struct drm_plane_sta
>>   {
>>       struct drm_shadow_plane_state *shadow_plane_state =
>> to_drm_shadow_plane_state(plane_state);
>>       struct drm_framebuffer *fb = plane_state->fb;
>> +    int ret;
>>         if (!fb)
>>           return 0;
>>   -    return drm_gem_fb_vmap(fb, shadow_plane_state->map,
>> shadow_plane_state->data);
>> +    ret = drm_gem_fb_vmap(fb, shadow_plane_state->map,
>> shadow_plane_state->data);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>> +    if (ret)
>> +        drm_gem_fb_vunmap(fb, shadow_plane_state->map);
>> +
>> +    return ret;
>>   }
>>   EXPORT_SYMBOL(drm_gem_begin_shadow_fb_access);
>>   @@ -404,6 +414,7 @@ void drm_gem_end_shadow_fb_access(struct
>> drm_plane *plane, struct drm_plane_stat
>>       if (!fb)
>>           return;
>>   +    drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>>       drm_gem_fb_vunmap(fb, shadow_plane_state->map);
>>   }
>>   EXPORT_SYMBOL(drm_gem_end_shadow_fb_access);
>> diff --git a/drivers/gpu/drm/solomon/ssd130x.c
>> b/drivers/gpu/drm/solomon/ssd130x.c
>> index 53464afc2b9a..58a2f0113f24 100644
>> --- a/drivers/gpu/drm/solomon/ssd130x.c
>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
>> @@ -544,7 +544,6 @@ static int ssd130x_fb_blit_rect(struct
>> drm_framebuffer *fb, const struct iosys_m
>>       struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
>>       struct iosys_map dst;
>>       unsigned int dst_pitch;
>> -    int ret = 0;
>>       u8 *buf = NULL;
>>         /* Align y to display page boundaries */
>> @@ -556,21 +555,14 @@ static int ssd130x_fb_blit_rect(struct
>> drm_framebuffer *fb, const struct iosys_m
>>       if (!buf)
>>           return -ENOMEM;
>>   -    ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>> -    if (ret)
>> -        goto out_free;
>> -
>>       iosys_map_set_vaddr(&dst, buf);
>>       drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
>>   -    drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>> -
>>       ssd130x_update_rect(ssd130x, buf, rect);
>>   -out_free:
>>       kfree(buf);
>>   -    return ret;
>> +    return 0;
>>   }
>>     static void ssd130x_primary_plane_helper_atomic_update(struct
>> drm_plane *plane,
>> diff --git a/drivers/gpu/drm/tiny/gm12u320.c
>> b/drivers/gpu/drm/tiny/gm12u320.c
>> index 130fd07a967d..59aad4b468cc 100644
>> --- a/drivers/gpu/drm/tiny/gm12u320.c
>> +++ b/drivers/gpu/drm/tiny/gm12u320.c
>> @@ -252,7 +252,7 @@ static void gm12u320_32bpp_to_24bpp_packed(u8
>> *dst, u8 *src, int len)
>>     static void gm12u320_copy_fb_to_blocks(struct gm12u320_device
>> *gm12u320)
>>   {
>> -    int block, dst_offset, len, remain, ret, x1, x2, y1, y2;
>> +    int block, dst_offset, len, remain, x1, x2, y1, y2;
>>       struct drm_framebuffer *fb;
>>       void *vaddr;
>>       u8 *src;
>> @@ -269,12 +269,6 @@ static void gm12u320_copy_fb_to_blocks(struct
>> gm12u320_device *gm12u320)
>>       y2 = gm12u320->fb_update.rect.y2;
>>       vaddr = gm12u320->fb_update.src_map.vaddr; /* TODO: Use mapping
>> abstraction properly */
>>   -    ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>> -    if (ret) {
>> -        GM12U320_ERR("drm_gem_fb_begin_cpu_access err: %d\n", ret);
>> -        goto put_fb;
>> -    }
>> -
>>       src = vaddr + y1 * fb->pitches[0] + x1 * 4;
>>         x1 += (GM12U320_REAL_WIDTH - GM12U320_USER_WIDTH) / 2;
>> @@ -309,8 +303,6 @@ static void gm12u320_copy_fb_to_blocks(struct
>> gm12u320_device *gm12u320)
>>           src += fb->pitches[0];
>>       }
>>   -    drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>> -put_fb:
>>       drm_framebuffer_put(fb);
>>       gm12u320->fb_update.fb = NULL;
>>   unlock:
>> diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
>> index dc9e4d71b12a..ed3072563db9 100644
>> --- a/drivers/gpu/drm/tiny/ofdrm.c
>> +++ b/drivers/gpu/drm/tiny/ofdrm.c
>> @@ -820,14 +820,10 @@ static void
>> ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>       const struct drm_format_info *dst_format = odev->format;
>>       struct drm_atomic_helper_damage_iter iter;
>>       struct drm_rect damage;
>> -    int ret, idx;
>> -
>> -    ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>> -    if (ret)
>> -        return;
>> +    int idx;
>>         if (!drm_dev_enter(dev, &idx))
>> -        goto out_drm_gem_fb_end_cpu_access;
>> +        return;
>>         drm_atomic_helper_damage_iter_init(&iter, old_plane_state,
>> plane_state);
>>       drm_atomic_for_each_plane_damage(&iter, &damage) {
>> @@ -843,8 +839,6 @@ static void
>> ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>       }
>>         drm_dev_exit(idx);
>> -out_drm_gem_fb_end_cpu_access:
>> -    drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>>   }
>>     static void ofdrm_primary_plane_helper_atomic_disable(struct
>> drm_plane *plane,
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c
>> b/drivers/gpu/drm/tiny/simpledrm.c
>> index 162eb44dcba8..1c0d9e277dc3 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -481,14 +481,10 @@ static void
>> simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>>       struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
>>       struct drm_atomic_helper_damage_iter iter;
>>       struct drm_rect damage;
>> -    int ret, idx;
>> -
>> -    ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>> -    if (ret)
>> -        return;
>> +    int idx;
>>         if (!drm_dev_enter(dev, &idx))
>> -        goto out_drm_gem_fb_end_cpu_access;
>> +        return;
>>         drm_atomic_helper_damage_iter_init(&iter, old_plane_state,
>> plane_state);
>>       drm_atomic_for_each_plane_damage(&iter, &damage) {
>> @@ -504,8 +500,6 @@ static void
>> simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>>       }
>>         drm_dev_exit(idx);
>> -out_drm_gem_fb_end_cpu_access:
>> -    drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>>   }
>>     static void simpledrm_primary_plane_helper_atomic_disable(struct
>> drm_plane *plane,
>> diff --git a/drivers/gpu/drm/udl/udl_modeset.c
>> b/drivers/gpu/drm/udl/udl_modeset.c
>> index 4b79d44752c9..022b18aa3f48 100644
>> --- a/drivers/gpu/drm/udl/udl_modeset.c
>> +++ b/drivers/gpu/drm/udl/udl_modeset.c
>> @@ -271,17 +271,13 @@ static void
>> udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>       struct drm_plane_state *old_plane_state =
>> drm_atomic_get_old_plane_state(state, plane);
>>       struct drm_atomic_helper_damage_iter iter;
>>       struct drm_rect damage;
>> -    int ret, idx;
>> +    int idx;
>>         if (!fb)
>>           return; /* no framebuffer; plane is disabled */
>>   -    ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>> -    if (ret)
>> -        return;
>> -
>>       if (!drm_dev_enter(dev, &idx))
>> -        goto out_drm_gem_fb_end_cpu_access;
>> +        return;
>>         drm_atomic_helper_damage_iter_init(&iter, old_plane_state,
>> plane_state);
>>       drm_atomic_for_each_plane_damage(&iter, &damage) {
>> @@ -289,9 +285,6 @@ static void
>> udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>       }
>>         drm_dev_exit(idx);
>> -
>> -out_drm_gem_fb_end_cpu_access:
>> -    drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>>   }
>>     static const struct drm_plane_helper_funcs
>> udl_primary_plane_helper_funcs = {
>>
> 



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux