Hi

Am 11.06.19 um 22:29 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Tue, Jun 11, 2019 at 03:03:40PM +0200, Thomas Zimmermann wrote:
>> Another explicit lock operation of a GEM VRAM BO is located in AST's
>> framebuffer update code. Instead of locking the BO, we pin it to wherever
>> it is.
>>
>> v2:
>>      * update with pin flag of 0
>>
>> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
>> ---
>>  drivers/gpu/drm/ast/ast_fb.c | 33 ++++++++++++++++-----------------
>>  1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
>> index 05f45222b702..b404b51c2c55 100644
>> --- a/drivers/gpu/drm/ast/ast_fb.c
>> +++ b/drivers/gpu/drm/ast/ast_fb.c
>> @@ -48,32 +48,31 @@ static void ast_dirty_update(struct ast_fbdev *afbdev,
>>                           int x, int y, int width, int height)
>>  {
>>      int i;
>> -    struct drm_gem_object *obj;
>>      struct drm_gem_vram_object *gbo;
>>      int src_offset, dst_offset;
>>      int bpp = afbdev->afb.base.format->cpp[0];
>> -    int ret = -EBUSY;
>> +    int ret;
>>      u8 *dst;
>>      bool unmap = false;
>>      bool store_for_later = false;
>>      int x2, y2;
>>      unsigned long flags;
>>  
>> -    obj = afbdev->afb.obj;
>> -    gbo = drm_gem_vram_of_gem(obj);
>> -
>> -    /* Try to lock the BO. If we fail with -EBUSY then
>> -     * the BO is being moved and we should store up the
>> -     * damage until later.
>> -     */
>> -    if (drm_can_sleep())
>> -            ret = drm_gem_vram_lock(gbo, true);
>> -    if (ret) {
>> -            if (ret != -EBUSY)
>> -                    return;
>> -
>> +    gbo = drm_gem_vram_of_gem(afbdev->afb.obj);
>> +
>> +    if (drm_can_sleep()) {
> 
> While touching this code, anyway we could get rid of drm_can_sleep()?
> I only ask because Daniel V. said that we should not use it.
> This was some months ago during some ehader file clean-up, so nothing
> in particular related to the ast driver.

I'm aware of what is commented in the header and the todo file. Do you
have a link to this discussion?

In any case, I'd prefer to fix this in a separate patch set. I have
patches that replace the ast and mgag200 fbdev consoles with generic
code. That might be the right place.

Best regards
Thomas

> 
>       Sam
> 
>> +            /* We pin the BO so it won't be moved during the
>> +             * update. The actual location, video RAM or system
>> +             * memory, is not important.
>> +             */
>> +            ret = drm_gem_vram_pin(gbo, 0);
>> +            if (ret) {
>> +                    if (ret != -EBUSY)
>> +                            return;
>> +                    store_for_later = true;
>> +            }
>> +    } else
>>              store_for_later = true;
>> -    }
>>  
>>      x2 = x + width - 1;
>>      y2 = y + height - 1;
>> @@ -126,7 +125,7 @@ static void ast_dirty_update(struct ast_fbdev *afbdev,
>>              drm_gem_vram_kunmap(gbo);
>>  
>>  out:
>> -    drm_gem_vram_unlock(gbo);
>> +    drm_gem_vram_unpin(gbo);
>>  }
>>  
>>  static void ast_fillrect(struct fb_info *info,
>> -- 
>> 2.21.0

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

Attachment: signature.asc
Description: OpenPGP digital signature

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

Reply via email to