Hi thanks for the review.
Am 23.11.19 um 09:56 schrieb Sam Ravnborg:
> Hi Thomas.
>
> Change looks good. If you spin this could you move the
> changes to generic drm code to a separate patch?
> As it is now it is hidden for most.
> But then there are also no users of drm_gem_vram_fill_create_dumb()
I just posted a patchset for mgag200 that uses this function. Maybe
it'll have to stay.
Best regards
Thomas
>
> One small nit below that you can safely ignore :-)
>
>
> Sam
>
>
> On Fri, Nov 22, 2019 at 09:30:43AM +0100, Thomas Zimmermann wrote:
>> The hibmc driver aligns scanlines to 16 bytes. Adding the pitch alignment
>> as an argument to drm_gem_vram_fill_create_dumb() allows to use the generic
>> implementation with hibmc. A value of 0 disables scanline pitches.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
>> ---
>> drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++--
>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 4 --
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 48 +------------------
>> include/drm/drm_gem_vram_helper.h | 1 +
>> 4 files changed, 10 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 666cb4c22bb9..f098784e7dfd 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -485,6 +485,7 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap);
>> * @dev: the DRM device
>> * @bdev: the TTM BO device managing the buffer object
>> * @pg_align: the buffer's alignment in multiples of the page
>> size
>> + * @pitch_align: the scanline's alignment in powers of 2
>> * @interruptible: sleep interruptible if waiting for memory
>> * @args: the arguments as provided to \
>> &struct drm_driver.dumb_create
>> @@ -502,6 +503,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
>> struct drm_device *dev,
>> struct ttm_bo_device *bdev,
>> unsigned long pg_align,
>> + unsigned long pitch_align,
>> bool interruptible,
>> struct drm_mode_create_dumb *args)
>> {
>> @@ -510,7 +512,9 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
>> int ret;
>> u32 handle;
>>
>> - pitch = args->width * ((args->bpp + 7) / 8);
>> + pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> Semi related change...
>
>> + if (pitch_align)
>> + pitch = ALIGN(pitch, pitch_align);
>> size = pitch * args->height;
>>
>> size = roundup(size, PAGE_SIZE);
>> @@ -612,8 +616,8 @@ int drm_gem_vram_driver_dumb_create(struct drm_file
>> *file,
>> if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized"))
>> return -EINVAL;
>>
>> - return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, 0,
>> - false, args);
>> + return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev,
>> + 0, 0, false, args);
>> }
>> EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create);
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index 8eb7258b236a..50a0c1f9d211 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -18,7 +18,6 @@
>> #include <drm/drm_framebuffer.h>
>>
>> struct drm_device;
>> -struct drm_gem_object;
>>
>> struct hibmc_drm_private {
>> /* hw */
>> @@ -41,9 +40,6 @@ void hibmc_set_current_gate(struct hibmc_drm_private *priv,
>> int hibmc_de_init(struct hibmc_drm_private *priv);
>> int hibmc_vdac_init(struct hibmc_drm_private *priv);
>>
>> -int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>> - struct drm_gem_object **obj);
>> -
>> int hibmc_mm_init(struct hibmc_drm_private *hibmc);
>> void hibmc_mm_fini(struct hibmc_drm_private *hibmc);
>> int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> index f6d25b85c209..0af5d966a480 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>> @@ -47,55 +47,11 @@ void hibmc_mm_fini(struct hibmc_drm_private *hibmc)
>> drm_vram_helper_release_mm(hibmc->dev);
>> }
>>
>> -int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>> - struct drm_gem_object **obj)
>> -{
>> - struct drm_gem_vram_object *gbo;
>> - int ret;
>> -
>> - *obj = NULL;
>> -
>> - size = roundup(size, PAGE_SIZE);
>> - if (size == 0)
>> - return -EINVAL;
>> -
>> - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0, false);
>> - if (IS_ERR(gbo)) {
>> - ret = PTR_ERR(gbo);
>> - if (ret != -ERESTARTSYS)
>> - DRM_ERROR("failed to allocate GEM object: %d\n", ret);
>> - return ret;
>> - }
>> - *obj = &gbo->bo.base;
>> - return 0;
>> -}
>> -
>> int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
>> struct drm_mode_create_dumb *args)
>> {
>> - struct drm_gem_object *gobj;
>> - u32 handle;
>> - int ret;
>> -
>> - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 16);
>> - args->size = args->pitch * args->height;
>> -
>> - ret = hibmc_gem_create(dev, args->size, false,
>> - &gobj);
>> - if (ret) {
>> - DRM_ERROR("failed to create GEM object: %d\n", ret);
>> - return ret;
>> - }
>> -
>> - ret = drm_gem_handle_create(file, gobj, &handle);
>> - drm_gem_object_put_unlocked(gobj);
>> - if (ret) {
>> - DRM_ERROR("failed to unreference GEM object: %d\n", ret);
>> - return ret;
>> - }
>> -
>> - args->handle = handle;
>> - return 0;
>> + return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev,
>> + 0, 16, false, args);
>> }
>>
>> const struct drm_mode_config_funcs hibmc_mode_funcs = {
>> diff --git a/include/drm/drm_gem_vram_helper.h
>> b/include/drm/drm_gem_vram_helper.h
>> index e040541a105f..c642b4cb6600 100644
>> --- a/include/drm/drm_gem_vram_helper.h
>> +++ b/include/drm/drm_gem_vram_helper.h
>> @@ -113,6 +113,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
>> struct drm_device *dev,
>> struct ttm_bo_device *bdev,
>> unsigned long pg_align,
>> + unsigned long pitch_align,
>> bool interruptible,
>> struct drm_mode_create_dumb *args);
>>
>> --
>> 2.23.0
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
