* [PATCH 1/4] drm/hisilicon/hibmc: Switch to generic fbdev emulation
2019-11-22 8:30 [PATCH 0/4] Replace hibmc code with generic implmentation Thomas Zimmermann
@ 2019-11-22 8:30 ` Thomas Zimmermann
2019-11-25 9:08 ` Daniel Vetter
2019-11-22 8:30 ` [PATCH 2/4] drm/hisilicon/hibmc: Replace struct hibmc_framebuffer with generic code Thomas Zimmermann
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Thomas Zimmermann @ 2019-11-22 8:30 UTC (permalink / raw)
To: daniel, maarten.lankhorst, mripard, airlied, z.liuxinliang,
zourongrong, kong.kongxinwei, puck.chen, sam, kraxel, hslester96,
yuehaibing
Cc: Thomas Zimmermann, dri-devel
There's nothing special about hibmc's fbdev emulation that is not
provided by the generic implementation. Switch over and remove the
driver's code.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
.../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 +-
.../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 11 -
.../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 240 ------------------
4 files changed, 3 insertions(+), 255 deletions(-)
delete mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 0c2d4296bccd..f99132715597 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
+++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
-hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_fbdev.o hibmc_ttm.o
+hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_ttm.o
obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 2fd4ca91a62d..113d27b8a8f1 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -17,6 +17,7 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
#include <drm/drm_gem_vram_helper.h>
#include <drm/drm_irq.h>
#include <drm/drm_print.h>
@@ -247,8 +248,6 @@ static int hibmc_unload(struct drm_device *dev)
{
struct hibmc_drm_private *priv = dev->dev_private;
- hibmc_fbdev_fini(priv);
-
drm_atomic_helper_shutdown(dev);
if (dev->irq_enabled)
@@ -307,7 +306,7 @@ static int hibmc_load(struct drm_device *dev)
/* reset all the states of crtc/plane/encoder/connector */
drm_mode_config_reset(dev);
- ret = hibmc_fbdev_init(priv);
+ ret = drm_fbdev_generic_setup(dev, 16);
if (ret) {
DRM_ERROR("failed to initialize fbdev: %d\n", ret);
goto err;
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index e58ecd7edcf8..b34493ead30b 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -25,12 +25,6 @@ struct hibmc_framebuffer {
struct drm_gem_object *obj;
};
-struct hibmc_fbdev {
- struct drm_fb_helper helper; /* must be first */
- struct hibmc_framebuffer *fb;
- int size;
-};
-
struct hibmc_drm_private {
/* hw */
void __iomem *mmio;
@@ -42,9 +36,6 @@ struct hibmc_drm_private {
/* drm */
struct drm_device *dev;
bool mode_config_initialized;
-
- /* fbdev */
- struct hibmc_fbdev *fbdev;
};
#define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
@@ -56,8 +47,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_fbdev_init(struct hibmc_drm_private *priv);
-void hibmc_fbdev_fini(struct hibmc_drm_private *priv);
int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
struct drm_gem_object **obj);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
deleted file mode 100644
index b4c1cea051e8..000000000000
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
+++ /dev/null
@@ -1,240 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/* Hisilicon Hibmc SoC drm driver
- *
- * Based on the bochs drm driver.
- *
- * Copyright (c) 2016 Huawei Limited.
- *
- * Author:
- * Rongrong Zou <zourongrong@huawei.com>
- * Rongrong Zou <zourongrong@gmail.com>
- * Jianhua Li <lijianhua@huawei.com>
- */
-
-#include <drm/drm_crtc.h>
-#include <drm/drm_fb_helper.h>
-#include <drm/drm_fourcc.h>
-#include <drm/drm_gem_vram_helper.h>
-#include <drm/drm_probe_helper.h>
-
-#include "hibmc_drm_drv.h"
-
-static int hibmcfb_create_object(
- struct hibmc_drm_private *priv,
- const struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_gem_object **gobj_p)
-{
- struct drm_gem_object *gobj;
- struct drm_device *dev = priv->dev;
- u32 size;
- int ret = 0;
-
- size = mode_cmd->pitches[0] * mode_cmd->height;
- ret = hibmc_gem_create(dev, size, true, &gobj);
- if (ret)
- return ret;
-
- *gobj_p = gobj;
- return ret;
-}
-
-static struct fb_ops hibmc_drm_fb_ops = {
- .owner = THIS_MODULE,
- .fb_check_var = drm_fb_helper_check_var,
- .fb_set_par = drm_fb_helper_set_par,
- .fb_fillrect = drm_fb_helper_sys_fillrect,
- .fb_copyarea = drm_fb_helper_sys_copyarea,
- .fb_imageblit = drm_fb_helper_sys_imageblit,
- .fb_pan_display = drm_fb_helper_pan_display,
- .fb_blank = drm_fb_helper_blank,
- .fb_setcmap = drm_fb_helper_setcmap,
-};
-
-static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
- struct drm_fb_helper_surface_size *sizes)
-{
- struct hibmc_fbdev *hi_fbdev =
- container_of(helper, struct hibmc_fbdev, helper);
- struct hibmc_drm_private *priv = helper->dev->dev_private;
- struct fb_info *info;
- struct drm_mode_fb_cmd2 mode_cmd;
- struct drm_gem_object *gobj = NULL;
- int ret = 0;
- size_t size;
- unsigned int bytes_per_pixel;
- struct drm_gem_vram_object *gbo = NULL;
- void *base;
-
- DRM_DEBUG_DRIVER("surface width(%d), height(%d) and bpp(%d)\n",
- sizes->surface_width, sizes->surface_height,
- sizes->surface_bpp);
-
- bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
-
- mode_cmd.width = sizes->surface_width;
- mode_cmd.height = sizes->surface_height;
- mode_cmd.pitches[0] = mode_cmd.width * bytes_per_pixel;
- mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
- sizes->surface_depth);
-
- size = PAGE_ALIGN(mode_cmd.pitches[0] * mode_cmd.height);
-
- ret = hibmcfb_create_object(priv, &mode_cmd, &gobj);
- if (ret) {
- DRM_ERROR("failed to create fbcon backing object: %d\n", ret);
- return -ENOMEM;
- }
-
- gbo = drm_gem_vram_of_gem(gobj);
-
- ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
- if (ret) {
- DRM_ERROR("failed to pin fbcon: %d\n", ret);
- goto out_unref_gem;
- }
-
- base = drm_gem_vram_kmap(gbo, true, NULL);
- if (IS_ERR(base)) {
- ret = PTR_ERR(base);
- DRM_ERROR("failed to kmap fbcon: %d\n", ret);
- goto out_unpin_bo;
- }
-
- info = drm_fb_helper_alloc_fbi(helper);
- if (IS_ERR(info)) {
- ret = PTR_ERR(info);
- DRM_ERROR("failed to allocate fbi: %d\n", ret);
- goto out_release_fbi;
- }
-
- hi_fbdev->fb = hibmc_framebuffer_init(priv->dev, &mode_cmd, gobj);
- if (IS_ERR(hi_fbdev->fb)) {
- ret = PTR_ERR(hi_fbdev->fb);
- hi_fbdev->fb = NULL;
- DRM_ERROR("failed to initialize framebuffer: %d\n", ret);
- goto out_release_fbi;
- }
-
- priv->fbdev->size = size;
- hi_fbdev->helper.fb = &hi_fbdev->fb->fb;
-
- info->fbops = &hibmc_drm_fb_ops;
-
- drm_fb_helper_fill_info(info, &priv->fbdev->helper, sizes);
-
- info->screen_base = base;
- info->screen_size = size;
-
- info->fix.smem_start = gbo->bo.mem.bus.offset + gbo->bo.mem.bus.base;
- info->fix.smem_len = size;
- return 0;
-
-out_release_fbi:
- drm_gem_vram_kunmap(gbo);
-out_unpin_bo:
- drm_gem_vram_unpin(gbo);
-out_unref_gem:
- drm_gem_object_put_unlocked(gobj);
-
- return ret;
-}
-
-static void hibmc_fbdev_destroy(struct hibmc_fbdev *fbdev)
-{
- struct hibmc_framebuffer *gfb = fbdev->fb;
- struct drm_fb_helper *fbh = &fbdev->helper;
-
- drm_fb_helper_unregister_fbi(fbh);
-
- drm_fb_helper_fini(fbh);
-
- if (gfb)
- drm_framebuffer_put(&gfb->fb);
-}
-
-static const struct drm_fb_helper_funcs hibmc_fbdev_helper_funcs = {
- .fb_probe = hibmc_drm_fb_create,
-};
-
-int hibmc_fbdev_init(struct hibmc_drm_private *priv)
-{
- int ret;
- struct fb_var_screeninfo *var;
- struct fb_fix_screeninfo *fix;
- struct hibmc_fbdev *hifbdev;
-
- hifbdev = devm_kzalloc(priv->dev->dev, sizeof(*hifbdev), GFP_KERNEL);
- if (!hifbdev) {
- DRM_ERROR("failed to allocate hibmc_fbdev\n");
- return -ENOMEM;
- }
-
- priv->fbdev = hifbdev;
- drm_fb_helper_prepare(priv->dev, &hifbdev->helper,
- &hibmc_fbdev_helper_funcs);
-
- /* Now just one crtc and one channel */
- ret = drm_fb_helper_init(priv->dev, &hifbdev->helper, 1);
- if (ret) {
- DRM_ERROR("failed to initialize fb helper: %d\n", ret);
- return ret;
- }
-
- ret = drm_fb_helper_single_add_all_connectors(&hifbdev->helper);
- if (ret) {
- DRM_ERROR("failed to add all connectors: %d\n", ret);
- goto fini;
- }
-
- ret = drm_fb_helper_initial_config(&hifbdev->helper, 16);
- if (ret) {
- DRM_ERROR("failed to setup initial conn config: %d\n", ret);
- goto fini;
- }
-
- var = &hifbdev->helper.fbdev->var;
- fix = &hifbdev->helper.fbdev->fix;
-
- DRM_DEBUG_DRIVER("Member of info->var is :\n"
- "xres=%d\n"
- "yres=%d\n"
- "xres_virtual=%d\n"
- "yres_virtual=%d\n"
- "xoffset=%d\n"
- "yoffset=%d\n"
- "bits_per_pixel=%d\n"
- "...\n", var->xres, var->yres, var->xres_virtual,
- var->yres_virtual, var->xoffset, var->yoffset,
- var->bits_per_pixel);
- DRM_DEBUG_DRIVER("Member of info->fix is :\n"
- "smem_start=%lx\n"
- "smem_len=%d\n"
- "type=%d\n"
- "type_aux=%d\n"
- "visual=%d\n"
- "xpanstep=%d\n"
- "ypanstep=%d\n"
- "ywrapstep=%d\n"
- "line_length=%d\n"
- "accel=%d\n"
- "capabilities=%d\n"
- "...\n", fix->smem_start, fix->smem_len, fix->type,
- fix->type_aux, fix->visual, fix->xpanstep,
- fix->ypanstep, fix->ywrapstep, fix->line_length,
- fix->accel, fix->capabilities);
-
- return 0;
-
-fini:
- drm_fb_helper_fini(&hifbdev->helper);
- return ret;
-}
-
-void hibmc_fbdev_fini(struct hibmc_drm_private *priv)
-{
- if (!priv->fbdev)
- return;
-
- hibmc_fbdev_destroy(priv->fbdev);
- priv->fbdev = NULL;
-}
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/4] drm/hisilicon/hibmc: Switch to generic fbdev emulation
2019-11-22 8:30 ` [PATCH 1/4] drm/hisilicon/hibmc: Switch to generic fbdev emulation Thomas Zimmermann
@ 2019-11-25 9:08 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2019-11-25 9:08 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: yuehaibing, airlied, puck.chen, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong, sam
On Fri, Nov 22, 2019 at 09:30:41AM +0100, Thomas Zimmermann wrote:
> There's nothing special about hibmc's fbdev emulation that is not
> provided by the generic implementation. Switch over and remove the
> driver's code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 +-
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 11 -
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 240 ------------------
> 4 files changed, 3 insertions(+), 255 deletions(-)
> delete mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 0c2d4296bccd..f99132715597 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_fbdev.o hibmc_ttm.o
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_ttm.o
>
> obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 2fd4ca91a62d..113d27b8a8f1 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -17,6 +17,7 @@
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_drv.h>
> +#include <drm/drm_fb_helper.h>
> #include <drm/drm_gem_vram_helper.h>
> #include <drm/drm_irq.h>
> #include <drm/drm_print.h>
> @@ -247,8 +248,6 @@ static int hibmc_unload(struct drm_device *dev)
> {
> struct hibmc_drm_private *priv = dev->dev_private;
>
> - hibmc_fbdev_fini(priv);
> -
> drm_atomic_helper_shutdown(dev);
>
> if (dev->irq_enabled)
> @@ -307,7 +306,7 @@ static int hibmc_load(struct drm_device *dev)
> /* reset all the states of crtc/plane/encoder/connector */
> drm_mode_config_reset(dev);
>
> - ret = hibmc_fbdev_init(priv);
> + ret = drm_fbdev_generic_setup(dev, 16);
Nice, +1 line, -lots of lines!
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> if (ret) {
> DRM_ERROR("failed to initialize fbdev: %d\n", ret);
> goto err;
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index e58ecd7edcf8..b34493ead30b 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -25,12 +25,6 @@ struct hibmc_framebuffer {
> struct drm_gem_object *obj;
> };
>
> -struct hibmc_fbdev {
> - struct drm_fb_helper helper; /* must be first */
> - struct hibmc_framebuffer *fb;
> - int size;
> -};
> -
> struct hibmc_drm_private {
> /* hw */
> void __iomem *mmio;
> @@ -42,9 +36,6 @@ struct hibmc_drm_private {
> /* drm */
> struct drm_device *dev;
> bool mode_config_initialized;
> -
> - /* fbdev */
> - struct hibmc_fbdev *fbdev;
> };
>
> #define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
> @@ -56,8 +47,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_fbdev_init(struct hibmc_drm_private *priv);
> -void hibmc_fbdev_fini(struct hibmc_drm_private *priv);
>
> int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
> struct drm_gem_object **obj);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
> deleted file mode 100644
> index b4c1cea051e8..000000000000
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
> +++ /dev/null
> @@ -1,240 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/* Hisilicon Hibmc SoC drm driver
> - *
> - * Based on the bochs drm driver.
> - *
> - * Copyright (c) 2016 Huawei Limited.
> - *
> - * Author:
> - * Rongrong Zou <zourongrong@huawei.com>
> - * Rongrong Zou <zourongrong@gmail.com>
> - * Jianhua Li <lijianhua@huawei.com>
> - */
> -
> -#include <drm/drm_crtc.h>
> -#include <drm/drm_fb_helper.h>
> -#include <drm/drm_fourcc.h>
> -#include <drm/drm_gem_vram_helper.h>
> -#include <drm/drm_probe_helper.h>
> -
> -#include "hibmc_drm_drv.h"
> -
> -static int hibmcfb_create_object(
> - struct hibmc_drm_private *priv,
> - const struct drm_mode_fb_cmd2 *mode_cmd,
> - struct drm_gem_object **gobj_p)
> -{
> - struct drm_gem_object *gobj;
> - struct drm_device *dev = priv->dev;
> - u32 size;
> - int ret = 0;
> -
> - size = mode_cmd->pitches[0] * mode_cmd->height;
> - ret = hibmc_gem_create(dev, size, true, &gobj);
> - if (ret)
> - return ret;
> -
> - *gobj_p = gobj;
> - return ret;
> -}
> -
> -static struct fb_ops hibmc_drm_fb_ops = {
> - .owner = THIS_MODULE,
> - .fb_check_var = drm_fb_helper_check_var,
> - .fb_set_par = drm_fb_helper_set_par,
> - .fb_fillrect = drm_fb_helper_sys_fillrect,
> - .fb_copyarea = drm_fb_helper_sys_copyarea,
> - .fb_imageblit = drm_fb_helper_sys_imageblit,
> - .fb_pan_display = drm_fb_helper_pan_display,
> - .fb_blank = drm_fb_helper_blank,
> - .fb_setcmap = drm_fb_helper_setcmap,
> -};
> -
> -static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
> - struct drm_fb_helper_surface_size *sizes)
> -{
> - struct hibmc_fbdev *hi_fbdev =
> - container_of(helper, struct hibmc_fbdev, helper);
> - struct hibmc_drm_private *priv = helper->dev->dev_private;
> - struct fb_info *info;
> - struct drm_mode_fb_cmd2 mode_cmd;
> - struct drm_gem_object *gobj = NULL;
> - int ret = 0;
> - size_t size;
> - unsigned int bytes_per_pixel;
> - struct drm_gem_vram_object *gbo = NULL;
> - void *base;
> -
> - DRM_DEBUG_DRIVER("surface width(%d), height(%d) and bpp(%d)\n",
> - sizes->surface_width, sizes->surface_height,
> - sizes->surface_bpp);
> -
> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> -
> - mode_cmd.width = sizes->surface_width;
> - mode_cmd.height = sizes->surface_height;
> - mode_cmd.pitches[0] = mode_cmd.width * bytes_per_pixel;
> - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> - sizes->surface_depth);
> -
> - size = PAGE_ALIGN(mode_cmd.pitches[0] * mode_cmd.height);
> -
> - ret = hibmcfb_create_object(priv, &mode_cmd, &gobj);
> - if (ret) {
> - DRM_ERROR("failed to create fbcon backing object: %d\n", ret);
> - return -ENOMEM;
> - }
> -
> - gbo = drm_gem_vram_of_gem(gobj);
> -
> - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> - if (ret) {
> - DRM_ERROR("failed to pin fbcon: %d\n", ret);
> - goto out_unref_gem;
> - }
> -
> - base = drm_gem_vram_kmap(gbo, true, NULL);
> - if (IS_ERR(base)) {
> - ret = PTR_ERR(base);
> - DRM_ERROR("failed to kmap fbcon: %d\n", ret);
> - goto out_unpin_bo;
> - }
> -
> - info = drm_fb_helper_alloc_fbi(helper);
> - if (IS_ERR(info)) {
> - ret = PTR_ERR(info);
> - DRM_ERROR("failed to allocate fbi: %d\n", ret);
> - goto out_release_fbi;
> - }
> -
> - hi_fbdev->fb = hibmc_framebuffer_init(priv->dev, &mode_cmd, gobj);
> - if (IS_ERR(hi_fbdev->fb)) {
> - ret = PTR_ERR(hi_fbdev->fb);
> - hi_fbdev->fb = NULL;
> - DRM_ERROR("failed to initialize framebuffer: %d\n", ret);
> - goto out_release_fbi;
> - }
> -
> - priv->fbdev->size = size;
> - hi_fbdev->helper.fb = &hi_fbdev->fb->fb;
> -
> - info->fbops = &hibmc_drm_fb_ops;
> -
> - drm_fb_helper_fill_info(info, &priv->fbdev->helper, sizes);
> -
> - info->screen_base = base;
> - info->screen_size = size;
> -
> - info->fix.smem_start = gbo->bo.mem.bus.offset + gbo->bo.mem.bus.base;
> - info->fix.smem_len = size;
> - return 0;
> -
> -out_release_fbi:
> - drm_gem_vram_kunmap(gbo);
> -out_unpin_bo:
> - drm_gem_vram_unpin(gbo);
> -out_unref_gem:
> - drm_gem_object_put_unlocked(gobj);
> -
> - return ret;
> -}
> -
> -static void hibmc_fbdev_destroy(struct hibmc_fbdev *fbdev)
> -{
> - struct hibmc_framebuffer *gfb = fbdev->fb;
> - struct drm_fb_helper *fbh = &fbdev->helper;
> -
> - drm_fb_helper_unregister_fbi(fbh);
> -
> - drm_fb_helper_fini(fbh);
> -
> - if (gfb)
> - drm_framebuffer_put(&gfb->fb);
> -}
> -
> -static const struct drm_fb_helper_funcs hibmc_fbdev_helper_funcs = {
> - .fb_probe = hibmc_drm_fb_create,
> -};
> -
> -int hibmc_fbdev_init(struct hibmc_drm_private *priv)
> -{
> - int ret;
> - struct fb_var_screeninfo *var;
> - struct fb_fix_screeninfo *fix;
> - struct hibmc_fbdev *hifbdev;
> -
> - hifbdev = devm_kzalloc(priv->dev->dev, sizeof(*hifbdev), GFP_KERNEL);
> - if (!hifbdev) {
> - DRM_ERROR("failed to allocate hibmc_fbdev\n");
> - return -ENOMEM;
> - }
> -
> - priv->fbdev = hifbdev;
> - drm_fb_helper_prepare(priv->dev, &hifbdev->helper,
> - &hibmc_fbdev_helper_funcs);
> -
> - /* Now just one crtc and one channel */
> - ret = drm_fb_helper_init(priv->dev, &hifbdev->helper, 1);
> - if (ret) {
> - DRM_ERROR("failed to initialize fb helper: %d\n", ret);
> - return ret;
> - }
> -
> - ret = drm_fb_helper_single_add_all_connectors(&hifbdev->helper);
> - if (ret) {
> - DRM_ERROR("failed to add all connectors: %d\n", ret);
> - goto fini;
> - }
> -
> - ret = drm_fb_helper_initial_config(&hifbdev->helper, 16);
> - if (ret) {
> - DRM_ERROR("failed to setup initial conn config: %d\n", ret);
> - goto fini;
> - }
> -
> - var = &hifbdev->helper.fbdev->var;
> - fix = &hifbdev->helper.fbdev->fix;
> -
> - DRM_DEBUG_DRIVER("Member of info->var is :\n"
> - "xres=%d\n"
> - "yres=%d\n"
> - "xres_virtual=%d\n"
> - "yres_virtual=%d\n"
> - "xoffset=%d\n"
> - "yoffset=%d\n"
> - "bits_per_pixel=%d\n"
> - "...\n", var->xres, var->yres, var->xres_virtual,
> - var->yres_virtual, var->xoffset, var->yoffset,
> - var->bits_per_pixel);
> - DRM_DEBUG_DRIVER("Member of info->fix is :\n"
> - "smem_start=%lx\n"
> - "smem_len=%d\n"
> - "type=%d\n"
> - "type_aux=%d\n"
> - "visual=%d\n"
> - "xpanstep=%d\n"
> - "ypanstep=%d\n"
> - "ywrapstep=%d\n"
> - "line_length=%d\n"
> - "accel=%d\n"
> - "capabilities=%d\n"
> - "...\n", fix->smem_start, fix->smem_len, fix->type,
> - fix->type_aux, fix->visual, fix->xpanstep,
> - fix->ypanstep, fix->ywrapstep, fix->line_length,
> - fix->accel, fix->capabilities);
> -
> - return 0;
> -
> -fini:
> - drm_fb_helper_fini(&hifbdev->helper);
> - return ret;
> -}
> -
> -void hibmc_fbdev_fini(struct hibmc_drm_private *priv)
> -{
> - if (!priv->fbdev)
> - return;
> -
> - hibmc_fbdev_destroy(priv->fbdev);
> - priv->fbdev = NULL;
> -}
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/4] drm/hisilicon/hibmc: Replace struct hibmc_framebuffer with generic code
2019-11-22 8:30 [PATCH 0/4] Replace hibmc code with generic implmentation Thomas Zimmermann
2019-11-22 8:30 ` [PATCH 1/4] drm/hisilicon/hibmc: Switch to generic fbdev emulation Thomas Zimmermann
@ 2019-11-22 8:30 ` Thomas Zimmermann
2019-11-25 9:09 ` Daniel Vetter
2019-11-22 8:30 ` [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers Thomas Zimmermann
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Thomas Zimmermann @ 2019-11-22 8:30 UTC (permalink / raw)
To: daniel, maarten.lankhorst, mripard, airlied, z.liuxinliang,
zourongrong, kong.kongxinwei, puck.chen, sam, kraxel, hslester96,
yuehaibing
Cc: Thomas Zimmermann, dri-devel
The hibmc driver's struct hibmc_framebuffer stores a DRM framebuffer
with an associated GEM object. This functionality is also provided by
generic code. Switch hibmc over.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
.../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 4 +-
.../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 11 ---
drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 69 +------------------
3 files changed, 3 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index 6527a97f68a3..7fa7d4933f60 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -99,14 +99,12 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
s64 gpu_addr = 0;
unsigned int line_l;
struct hibmc_drm_private *priv = plane->dev->dev_private;
- struct hibmc_framebuffer *hibmc_fb;
struct drm_gem_vram_object *gbo;
if (!state->fb)
return;
- hibmc_fb = to_hibmc_framebuffer(state->fb);
- gbo = drm_gem_vram_of_gem(hibmc_fb->obj);
+ gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
gpu_addr = drm_gem_vram_offset(gbo);
if (WARN_ON_ONCE(gpu_addr < 0))
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index b34493ead30b..8eb7258b236a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -20,11 +20,6 @@
struct drm_device;
struct drm_gem_object;
-struct hibmc_framebuffer {
- struct drm_framebuffer fb;
- struct drm_gem_object *obj;
-};
-
struct hibmc_drm_private {
/* hw */
void __iomem *mmio;
@@ -38,8 +33,6 @@ struct hibmc_drm_private {
bool mode_config_initialized;
};
-#define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
-
void hibmc_set_power_mode(struct hibmc_drm_private *priv,
unsigned int power_mode);
void hibmc_set_current_gate(struct hibmc_drm_private *priv,
@@ -50,10 +43,6 @@ 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);
-struct hibmc_framebuffer *
-hibmc_framebuffer_init(struct drm_device *dev,
- const struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_gem_object *obj);
int hibmc_mm_init(struct hibmc_drm_private *hibmc);
void hibmc_mm_fini(struct hibmc_drm_private *hibmc);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
index 21b684eab5c9..f6d25b85c209 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
@@ -15,6 +15,7 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_gem.h>
+#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_vram_helper.h>
#include <drm/drm_print.h>
@@ -97,74 +98,8 @@ int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
return 0;
}
-static void hibmc_user_framebuffer_destroy(struct drm_framebuffer *fb)
-{
- struct hibmc_framebuffer *hibmc_fb = to_hibmc_framebuffer(fb);
-
- drm_gem_object_put_unlocked(hibmc_fb->obj);
- drm_framebuffer_cleanup(fb);
- kfree(hibmc_fb);
-}
-
-static const struct drm_framebuffer_funcs hibmc_fb_funcs = {
- .destroy = hibmc_user_framebuffer_destroy,
-};
-
-struct hibmc_framebuffer *
-hibmc_framebuffer_init(struct drm_device *dev,
- const struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_gem_object *obj)
-{
- struct hibmc_framebuffer *hibmc_fb;
- int ret;
-
- hibmc_fb = kzalloc(sizeof(*hibmc_fb), GFP_KERNEL);
- if (!hibmc_fb) {
- DRM_ERROR("failed to allocate hibmc_fb\n");
- return ERR_PTR(-ENOMEM);
- }
-
- drm_helper_mode_fill_fb_struct(dev, &hibmc_fb->fb, mode_cmd);
- hibmc_fb->obj = obj;
- ret = drm_framebuffer_init(dev, &hibmc_fb->fb, &hibmc_fb_funcs);
- if (ret) {
- DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
- kfree(hibmc_fb);
- return ERR_PTR(ret);
- }
-
- return hibmc_fb;
-}
-
-static struct drm_framebuffer *
-hibmc_user_framebuffer_create(struct drm_device *dev,
- struct drm_file *filp,
- const struct drm_mode_fb_cmd2 *mode_cmd)
-{
- struct drm_gem_object *obj;
- struct hibmc_framebuffer *hibmc_fb;
-
- DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",
- mode_cmd->width, mode_cmd->height,
- (mode_cmd->pixel_format) & 0xff,
- (mode_cmd->pixel_format >> 8) & 0xff,
- (mode_cmd->pixel_format >> 16) & 0xff,
- (mode_cmd->pixel_format >> 24) & 0xff);
-
- obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
- if (!obj)
- return ERR_PTR(-ENOENT);
-
- hibmc_fb = hibmc_framebuffer_init(dev, mode_cmd, obj);
- if (IS_ERR(hibmc_fb)) {
- drm_gem_object_put_unlocked(obj);
- return ERR_PTR((long)hibmc_fb);
- }
- return &hibmc_fb->fb;
-}
-
const struct drm_mode_config_funcs hibmc_mode_funcs = {
.atomic_check = drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit,
- .fb_create = hibmc_user_framebuffer_create,
+ .fb_create = drm_gem_fb_create,
};
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/4] drm/hisilicon/hibmc: Replace struct hibmc_framebuffer with generic code
2019-11-22 8:30 ` [PATCH 2/4] drm/hisilicon/hibmc: Replace struct hibmc_framebuffer with generic code Thomas Zimmermann
@ 2019-11-25 9:09 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2019-11-25 9:09 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: yuehaibing, airlied, puck.chen, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong, sam
On Fri, Nov 22, 2019 at 09:30:42AM +0100, Thomas Zimmermann wrote:
> The hibmc driver's struct hibmc_framebuffer stores a DRM framebuffer
> with an associated GEM object. This functionality is also provided by
> generic code. Switch hibmc over.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Aside from the one silly compile fail I managed to do and not spot this
matches what I've done.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 4 +-
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 11 ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 69 +------------------
> 3 files changed, 3 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index 6527a97f68a3..7fa7d4933f60 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -99,14 +99,12 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
> s64 gpu_addr = 0;
> unsigned int line_l;
> struct hibmc_drm_private *priv = plane->dev->dev_private;
> - struct hibmc_framebuffer *hibmc_fb;
> struct drm_gem_vram_object *gbo;
>
> if (!state->fb)
> return;
>
> - hibmc_fb = to_hibmc_framebuffer(state->fb);
> - gbo = drm_gem_vram_of_gem(hibmc_fb->obj);
> + gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
>
> gpu_addr = drm_gem_vram_offset(gbo);
> if (WARN_ON_ONCE(gpu_addr < 0))
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index b34493ead30b..8eb7258b236a 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -20,11 +20,6 @@
> struct drm_device;
> struct drm_gem_object;
>
> -struct hibmc_framebuffer {
> - struct drm_framebuffer fb;
> - struct drm_gem_object *obj;
> -};
> -
> struct hibmc_drm_private {
> /* hw */
> void __iomem *mmio;
> @@ -38,8 +33,6 @@ struct hibmc_drm_private {
> bool mode_config_initialized;
> };
>
> -#define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
> -
> void hibmc_set_power_mode(struct hibmc_drm_private *priv,
> unsigned int power_mode);
> void hibmc_set_current_gate(struct hibmc_drm_private *priv,
> @@ -50,10 +43,6 @@ 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);
> -struct hibmc_framebuffer *
> -hibmc_framebuffer_init(struct drm_device *dev,
> - const struct drm_mode_fb_cmd2 *mode_cmd,
> - struct drm_gem_object *obj);
>
> int hibmc_mm_init(struct hibmc_drm_private *hibmc);
> void hibmc_mm_fini(struct hibmc_drm_private *hibmc);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> index 21b684eab5c9..f6d25b85c209 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> @@ -15,6 +15,7 @@
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_gem.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> #include <drm/drm_gem_vram_helper.h>
> #include <drm/drm_print.h>
>
> @@ -97,74 +98,8 @@ int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
> return 0;
> }
>
> -static void hibmc_user_framebuffer_destroy(struct drm_framebuffer *fb)
> -{
> - struct hibmc_framebuffer *hibmc_fb = to_hibmc_framebuffer(fb);
> -
> - drm_gem_object_put_unlocked(hibmc_fb->obj);
> - drm_framebuffer_cleanup(fb);
> - kfree(hibmc_fb);
> -}
> -
> -static const struct drm_framebuffer_funcs hibmc_fb_funcs = {
> - .destroy = hibmc_user_framebuffer_destroy,
> -};
> -
> -struct hibmc_framebuffer *
> -hibmc_framebuffer_init(struct drm_device *dev,
> - const struct drm_mode_fb_cmd2 *mode_cmd,
> - struct drm_gem_object *obj)
> -{
> - struct hibmc_framebuffer *hibmc_fb;
> - int ret;
> -
> - hibmc_fb = kzalloc(sizeof(*hibmc_fb), GFP_KERNEL);
> - if (!hibmc_fb) {
> - DRM_ERROR("failed to allocate hibmc_fb\n");
> - return ERR_PTR(-ENOMEM);
> - }
> -
> - drm_helper_mode_fill_fb_struct(dev, &hibmc_fb->fb, mode_cmd);
> - hibmc_fb->obj = obj;
> - ret = drm_framebuffer_init(dev, &hibmc_fb->fb, &hibmc_fb_funcs);
> - if (ret) {
> - DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
> - kfree(hibmc_fb);
> - return ERR_PTR(ret);
> - }
> -
> - return hibmc_fb;
> -}
> -
> -static struct drm_framebuffer *
> -hibmc_user_framebuffer_create(struct drm_device *dev,
> - struct drm_file *filp,
> - const struct drm_mode_fb_cmd2 *mode_cmd)
> -{
> - struct drm_gem_object *obj;
> - struct hibmc_framebuffer *hibmc_fb;
> -
> - DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",
> - mode_cmd->width, mode_cmd->height,
> - (mode_cmd->pixel_format) & 0xff,
> - (mode_cmd->pixel_format >> 8) & 0xff,
> - (mode_cmd->pixel_format >> 16) & 0xff,
> - (mode_cmd->pixel_format >> 24) & 0xff);
> -
> - obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
> - if (!obj)
> - return ERR_PTR(-ENOENT);
> -
> - hibmc_fb = hibmc_framebuffer_init(dev, mode_cmd, obj);
> - if (IS_ERR(hibmc_fb)) {
> - drm_gem_object_put_unlocked(obj);
> - return ERR_PTR((long)hibmc_fb);
> - }
> - return &hibmc_fb->fb;
> -}
> -
> const struct drm_mode_config_funcs hibmc_mode_funcs = {
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = drm_atomic_helper_commit,
> - .fb_create = hibmc_user_framebuffer_create,
> + .fb_create = drm_gem_fb_create,
> };
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers
2019-11-22 8:30 [PATCH 0/4] Replace hibmc code with generic implmentation Thomas Zimmermann
2019-11-22 8:30 ` [PATCH 1/4] drm/hisilicon/hibmc: Switch to generic fbdev emulation Thomas Zimmermann
2019-11-22 8:30 ` [PATCH 2/4] drm/hisilicon/hibmc: Replace struct hibmc_framebuffer with generic code Thomas Zimmermann
@ 2019-11-22 8:30 ` Thomas Zimmermann
2019-11-23 8:56 ` Sam Ravnborg
2019-11-25 9:14 ` Daniel Vetter
2019-11-22 8:30 ` [PATCH 4/4] drm/hisilicon/hibmc: Export VRAM MM information to debugfs Thomas Zimmermann
2019-11-23 8:59 ` [PATCH 0/4] Replace hibmc code with generic implmentation Sam Ravnborg
4 siblings, 2 replies; 14+ messages in thread
From: Thomas Zimmermann @ 2019-11-22 8:30 UTC (permalink / raw)
To: daniel, maarten.lankhorst, mripard, airlied, z.liuxinliang,
zourongrong, kong.kongxinwei, puck.chen, sam, kraxel, hslester96,
yuehaibing
Cc: Thomas Zimmermann, dri-devel
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 <tzimmermann@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);
+ 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers
2019-11-22 8:30 ` [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers Thomas Zimmermann
@ 2019-11-23 8:56 ` Sam Ravnborg
2019-11-26 7:41 ` Thomas Zimmermann
2019-11-25 9:14 ` Daniel Vetter
1 sibling, 1 reply; 14+ messages in thread
From: Sam Ravnborg @ 2019-11-23 8:56 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: yuehaibing, airlied, puck.chen, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong
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()
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 <tzimmermann@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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers
2019-11-23 8:56 ` Sam Ravnborg
@ 2019-11-26 7:41 ` Thomas Zimmermann
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Zimmermann @ 2019-11-26 7:41 UTC (permalink / raw)
To: Sam Ravnborg
Cc: airlied, puck.chen, yuehaibing, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong
[-- Attachment #1.1.1: Type: text/plain, Size: 6884 bytes --]
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 <tzimmermann@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
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers
2019-11-22 8:30 ` [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers Thomas Zimmermann
2019-11-23 8:56 ` Sam Ravnborg
@ 2019-11-25 9:14 ` Daniel Vetter
2019-11-26 7:40 ` Thomas Zimmermann
1 sibling, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2019-11-25 9:14 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: yuehaibing, airlied, puck.chen, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong, 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 <tzimmermann@suse.de>
I concur with Sam, the vram change should be split out.
> ---
> 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
I also noticed that no one sets this to true, neither here nor in
drm_gem_vram_create(). Maybe remove that too? Otherwise the argument list
becomes very unwielding. And you're already touching the (few) callers.
> * @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);
> + if (pitch_align)
> + pitch = ALIGN(pitch, pitch_align);
Maybe throw a WARN_IS(is_pot(align)) in here?
Cheers, Daniel
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers
2019-11-25 9:14 ` Daniel Vetter
@ 2019-11-26 7:40 ` Thomas Zimmermann
2019-11-26 8:38 ` Daniel Vetter
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Zimmermann @ 2019-11-26 7:40 UTC (permalink / raw)
To: Daniel Vetter
Cc: airlied, puck.chen, yuehaibing, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong, sam
[-- Attachment #1.1.1: Type: text/plain, Size: 6802 bytes --]
Hi
Am 25.11.19 um 10:14 schrieb Daniel Vetter:
> 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 <tzimmermann@suse.de>
>
> I concur with Sam, the vram change should be split out.
>
>> ---
>> 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
>
> I also noticed that no one sets this to true, neither here nor in
> drm_gem_vram_create(). Maybe remove that too? Otherwise the argument list
> becomes very unwielding. And you're already touching the (few) callers.
OK, I'll add this as a separate patch.
BTW What's the DRM interface's behavior wrt interruption? For example,
can a ioctl call like CREATE_DUMB return EINTR to userspace?
Best regards
Thomas
>
>> * @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);
>> + if (pitch_align)
>> + pitch = ALIGN(pitch, pitch_align);
>
> Maybe throw a WARN_IS(is_pot(align)) in here?
>
> Cheers, Daniel
>
>> 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
>>
>
--
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
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers
2019-11-26 7:40 ` Thomas Zimmermann
@ 2019-11-26 8:38 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2019-11-26 8:38 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied, puck.chen, yuehaibing, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong, sam
On Tue, Nov 26, 2019 at 08:40:27AM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 25.11.19 um 10:14 schrieb Daniel Vetter:
> > 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 <tzimmermann@suse.de>
> >
> > I concur with Sam, the vram change should be split out.
> >
> >> ---
> >> 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
> >
> > I also noticed that no one sets this to true, neither here nor in
> > drm_gem_vram_create(). Maybe remove that too? Otherwise the argument list
> > becomes very unwielding. And you're already touching the (few) callers.
>
> OK, I'll add this as a separate patch.
Yeah makes sense.
> BTW What's the DRM interface's behavior wrt interruption? For example,
> can a ioctl call like CREATE_DUMB return EINTR to userspace?
Yup. Everyone is required to use drmIoctl() for all drm ioctls, which
auto-restarts all syscalls when userspace sees a EINTR. We also generally
test that in igt (but maybe not for all the kms ioctls, at least not for
the dumb ones).
interruptible + igts using igt_while_interruptible is a fairly effective
way to exercise error paths in all kinds of places. Only trouble is that
if we introduce a new interruptible point somewhere we might run into
userspace that gets it wrong (e.g. dumb ioctls I think aren't
interruptible on most x86 drivers right now, so there might be a surprise
and we need to audit userspaces, including plymouth and all those).
-Daniel
>
> Best regards
> Thomas
>
> >
> >> * @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);
> >> + if (pitch_align)
> >> + pitch = ALIGN(pitch, pitch_align);
> >
> > Maybe throw a WARN_IS(is_pot(align)) in here?
> >
> > Cheers, Daniel
> >
> >> 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
> >>
> >
>
> --
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] drm/hisilicon/hibmc: Export VRAM MM information to debugfs
2019-11-22 8:30 [PATCH 0/4] Replace hibmc code with generic implmentation Thomas Zimmermann
` (2 preceding siblings ...)
2019-11-22 8:30 ` [PATCH 3/4] drm/hisilicon/hibmc: Implement hibmc_dumb_create() with generic helpers Thomas Zimmermann
@ 2019-11-22 8:30 ` Thomas Zimmermann
2019-11-25 9:15 ` Daniel Vetter
2019-11-23 8:59 ` [PATCH 0/4] Replace hibmc code with generic implmentation Sam Ravnborg
4 siblings, 1 reply; 14+ messages in thread
From: Thomas Zimmermann @ 2019-11-22 8:30 UTC (permalink / raw)
To: daniel, maarten.lankhorst, mripard, airlied, z.liuxinliang,
zourongrong, kong.kongxinwei, puck.chen, sam, kraxel, hslester96,
yuehaibing
Cc: Thomas Zimmermann, dri-devel
This change makes information about VRAM consumption available on
debugfs. See
/sys/kernel/debug/dri/0/vram-mm
for an overview of how VRAM is being used.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 113d27b8a8f1..11d1b0761c9a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -55,6 +55,7 @@ static struct drm_driver hibmc_driver = {
.desc = "hibmc drm driver",
.major = 1,
.minor = 0,
+ .debugfs_init = drm_vram_mm_debugfs_init,
.dumb_create = hibmc_dumb_create,
.dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset,
.gem_prime_mmap = drm_gem_prime_mmap,
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 4/4] drm/hisilicon/hibmc: Export VRAM MM information to debugfs
2019-11-22 8:30 ` [PATCH 4/4] drm/hisilicon/hibmc: Export VRAM MM information to debugfs Thomas Zimmermann
@ 2019-11-25 9:15 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2019-11-25 9:15 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: yuehaibing, airlied, puck.chen, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong, sam
On Fri, Nov 22, 2019 at 09:30:44AM +0100, Thomas Zimmermann wrote:
> This change makes information about VRAM consumption available on
> debugfs. See
>
> /sys/kernel/debug/dri/0/vram-mm
>
> for an overview of how VRAM is being used.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Wrt merging: I'd wait 2-3 weeks for testing from maintainers, then merge
either way. Either it works, or we've found someone who cares enough to
test patches if something broke in here :-)
-Daniel
> ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 113d27b8a8f1..11d1b0761c9a 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -55,6 +55,7 @@ static struct drm_driver hibmc_driver = {
> .desc = "hibmc drm driver",
> .major = 1,
> .minor = 0,
> + .debugfs_init = drm_vram_mm_debugfs_init,
> .dumb_create = hibmc_dumb_create,
> .dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset,
> .gem_prime_mmap = drm_gem_prime_mmap,
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/4] Replace hibmc code with generic implmentation
2019-11-22 8:30 [PATCH 0/4] Replace hibmc code with generic implmentation Thomas Zimmermann
` (3 preceding siblings ...)
2019-11-22 8:30 ` [PATCH 4/4] drm/hisilicon/hibmc: Export VRAM MM information to debugfs Thomas Zimmermann
@ 2019-11-23 8:59 ` Sam Ravnborg
4 siblings, 0 replies; 14+ messages in thread
From: Sam Ravnborg @ 2019-11-23 8:59 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: yuehaibing, airlied, puck.chen, dri-devel, z.liuxinliang,
kong.kongxinwei, hslester96, kraxel, zourongrong
Hi Thomas.
> The patch set replaces code in hibmc with generic implementation.
>
> Patches 1 to 3 replace fbdev emuation, framebuffer and creation of
> dumb buffers with respective code from DRM helpers. Patch 4 adds an
> additional interface to debugfs that displays the allocated and free
> areas in video memory.
>
> The patches have only been compile-tested. Further testing is
> appreciated.
Changes looks good and diffstat is very nice.
From my quick browsing everything was fine, a small comment on
one of the patches.
You can add my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
to the whole series.
Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread