Re: [PATCH 5/8] drm/sun4i: Add a driver for the display frontend

2017-12-15 Thread kbuild test robot 
Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on ]

url:
https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-sun4i-Support-the-Display-Engine-frontend/20171216-122702
base:
config: arm-sunxi_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

Note: the 
linux-review/Maxime-Ripard/drm-sun4i-Support-the-Display-Engine-frontend/20171216-122702
 HEAD c38c4ce4b14c4c68a9fde0cc35ead5b1c894776b builds fine.
  It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun4i_backend.c: In function 'sun4i_backend_bind':
>> drivers/gpu/drm/sun4i/sun4i_backend.c:370:22: error: implicit declaration of 
>> function 'sun4i_backend_find_frontend'; did you mean 'sun4i_backend_bind'? 
>> [-Werror=implicit-function-declaration]
 backend->frontend = sun4i_backend_find_frontend(drv, dev->of_node);
 ^~~
 sun4i_backend_bind
>> drivers/gpu/drm/sun4i/sun4i_backend.c:370:20: warning: assignment makes 
>> pointer from integer without a cast [-Wint-conversion]
 backend->frontend = sun4i_backend_find_frontend(drv, dev->of_node);
   ^
   cc1: some warnings being treated as errors

vim +370 drivers/gpu/drm/sun4i/sun4i_backend.c

   346  
   347  static int sun4i_backend_bind(struct device *dev, struct device *master,
   348void *data)
   349  {
   350  struct platform_device *pdev = to_platform_device(dev);
   351  struct drm_device *drm = data;
   352  struct sun4i_drv *drv = drm->dev_private;
   353  struct sun4i_backend *backend;
   354  const struct sun4i_backend_quirks *quirks;
   355  struct resource *res;
   356  void __iomem *regs;
   357  int i, ret;
   358  
   359  backend = devm_kzalloc(dev, sizeof(*backend), GFP_KERNEL);
   360  if (!backend)
   361  return -ENOMEM;
   362  dev_set_drvdata(dev, backend);
   363  
   364  backend->engine.node = dev->of_node;
   365  backend->engine.ops = _backend_engine_ops;
   366  backend->engine.id = sun4i_backend_of_get_id(dev->of_node);
   367  if (backend->engine.id < 0)
   368  return backend->engine.id;
   369  
 > 370  backend->frontend = sun4i_backend_find_frontend(drv, 
 > dev->of_node);
   371  if (IS_ERR(backend->frontend)) {
   372  dev_err(dev, "Couldn't find matching frontend, frontend 
features disabled\n");
   373  }
   374  
   375  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   376  regs = devm_ioremap_resource(dev, res);
   377  if (IS_ERR(regs))
   378  return PTR_ERR(regs);
   379  
   380  backend->reset = devm_reset_control_get(dev, NULL);
   381  if (IS_ERR(backend->reset)) {
   382  dev_err(dev, "Couldn't get our reset line\n");
   383  return PTR_ERR(backend->reset);
   384  }
   385  
   386  ret = reset_control_deassert(backend->reset);
   387  if (ret) {
   388  dev_err(dev, "Couldn't deassert our reset line\n");
   389  return ret;
   390  }
   391  
   392  backend->bus_clk = devm_clk_get(dev, "ahb");
   393  if (IS_ERR(backend->bus_clk)) {
   394  dev_err(dev, "Couldn't get the backend bus clock\n");
   395  ret = PTR_ERR(backend->bus_clk);
   396  goto err_assert_reset;
   397  }
   398  clk_prepare_enable(backend->bus_clk);
   399  
   400  backend->mod_clk = devm_clk_get(dev, "mod");
   401  if (IS_ERR(backend->mod_clk)) {
   402  dev_err(dev, "Couldn't get the backend module clock\n");
   403  ret = PTR_ERR(backend->mod_clk);
   404  goto err_disable_bus_clk;
   405  }
   406  clk_prepare_enable(backend->mod_clk);
   407  
   408  backend->ram_clk = devm_clk_get(dev, "ram");
   409  if (IS_ERR(backend->ram_clk)) {
   410  dev_err(dev, "Couldn't get the backend RAM clock\n");
   411  ret = PTR_ERR(backend->ram_clk);
   412  goto err_disable_mod_clk;
   413  }
   414  clk_prepare_enable(backend->ram_clk);
   415  
   416  if (of_device_is_compatible(dev->of_node,
   417  
"allwinner,sun8i-a33-display-backend")) {
   418  ret = sun4i_backend_init_sat(dev);
   419  if (ret) {
   420

Re: [PATCH 5/8] drm/sun4i: Add a driver for the display frontend

2017-12-13 Thread Neil Armstrong 
On 13/12/2017 16:33, Maxime Ripard wrote:
> The display frontend is an hardware block that can be used to implement
> some more advanced features like hardware scaling or colorspace
> conversions. It can also be used to implement the output format of the VPU.
> 
> Let's create a minimal driver for it that will only enable the hardware
> scaling features.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/sun4i/Makefile |   3 +-
>  drivers/gpu/drm/sun4i/sun4i_backend.c  |   5 +-
>  drivers/gpu/drm/sun4i/sun4i_backend.h  |   1 +-
>  drivers/gpu/drm/sun4i/sun4i_drv.c  |  16 +-
>  drivers/gpu/drm/sun4i/sun4i_drv.h  |   1 +-
>  drivers/gpu/drm/sun4i/sun4i_frontend.c | 377 ++-
>  drivers/gpu/drm/sun4i/sun4i_frontend.h | 102 +++-
>  7 files changed, 500 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.h
> 
> diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> index 0c2f8c7facae..b660d82011f4 100644
> --- a/drivers/gpu/drm/sun4i/Makefile
> +++ b/drivers/gpu/drm/sun4i/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  sun4i-backend-y  += sun4i_backend.o sun4i_layer.o
> +sun4i-frontend-y += sun4i_frontend.o
>  
>  sun4i-drm-y  += sun4i_drv.o
>  sun4i-drm-y  += sun4i_framebuffer.o
> @@ -21,6 +22,6 @@ obj-$(CONFIG_DRM_SUN4I) += sun4i-tcon.o
>  obj-$(CONFIG_DRM_SUN4I)  += sun4i_tv.o
>  obj-$(CONFIG_DRM_SUN4I)  += sun6i_drc.o
>  
> -obj-$(CONFIG_DRM_SUN4I_BACKEND)  += sun4i-backend.o
> +obj-$(CONFIG_DRM_SUN4I_BACKEND)  += sun4i-backend.o sun4i-frontend.o
>  obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o
>  obj-$(CONFIG_DRM_SUN8I_MIXER)+= sun8i-mixer.o
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index f971d3fb5ee4..e83e1fe43823 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -367,6 +367,11 @@ static int sun4i_backend_bind(struct device *dev, struct 
> device *master,
>   if (backend->engine.id < 0)
>   return backend->engine.id;
>  
> + backend->frontend = sun4i_backend_find_frontend(drv, dev->of_node);
> + if (IS_ERR(backend->frontend)) {
> + dev_err(dev, "Couldn't find matching frontend, frontend 
> features disabled\n");

Maybe a dev_warn ?

> + }
> +
>   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   regs = devm_ioremap_resource(dev, res);
>   if (IS_ERR(regs))
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h 
> b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index ac3cc029f5cd..ba1410fd5410 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -145,6 +145,7 @@
>  
>  struct sun4i_backend {
>   struct sunxi_engine engine;
> + struct sun4i_frontend   *frontend;
>  
>   struct reset_control*reset;
>  
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
> b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 75c76cdd82bc..17bf9bfd98ba 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -98,6 +98,7 @@ static int sun4i_drv_bind(struct device *dev)
>   goto free_drm;
>   }
>   drm->dev_private = drv;
> + INIT_LIST_HEAD(>frontend_list);
>   INIT_LIST_HEAD(>engine_list);
>   INIT_LIST_HEAD(>tcon_list);
>  
> @@ -239,9 +240,11 @@ static int sun4i_drv_add_endpoints(struct device *dev,
>   int count = 0;
>  
>   /*
> -  * We don't support the frontend for now, so we will never
> -  * have a device bound. Just skip over it, but we still want
> -  * the rest our pipeline to be added.
> +  * The frontend has been disabled in all of our old device
> +  * trees. If we find a node that is the frontend and is
> +  * disabled, we should just follow through and parse its
> +  * child, but without adding it to the component list.
> +  * Otherwise, we obviously want to add it to the list.
>*/
>   if (!sun4i_drv_node_is_frontend(node) &&
>   !of_device_is_available(node))
> @@ -254,7 +257,12 @@ static int sun4i_drv_add_endpoints(struct device *dev,
>   if (sun4i_drv_node_is_connector(node))
>   return 0;
>  
> - if (!sun4i_drv_node_is_frontend(node)) {
> + /*
> +  * If the device is either just a regular device, or an
> +  * enabled frontend, we add it to our component list.
> +  */
> + if (!sun4i_drv_node_is_frontend(node) ||
> + (sun4i_drv_node_is_frontend(node) && of_device_is_available(node))) 
> {
>   /* Add current component */
>   DRM_DEBUG_DRIVER("Adding component %pOF\n", node);
>   drm_of_component_match_add(dev, match, compare_of, node);
> diff

[PATCH 5/8] drm/sun4i: Add a driver for the display frontend

2017-12-13 Thread Maxime Ripard 
The display frontend is an hardware block that can be used to implement
some more advanced features like hardware scaling or colorspace
conversions. It can also be used to implement the output format of the VPU.

Let's create a minimal driver for it that will only enable the hardware
scaling features.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/Makefile |   3 +-
 drivers/gpu/drm/sun4i/sun4i_backend.c  |   5 +-
 drivers/gpu/drm/sun4i/sun4i_backend.h  |   1 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c  |  16 +-
 drivers/gpu/drm/sun4i/sun4i_drv.h  |   1 +-
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 377 ++-
 drivers/gpu/drm/sun4i/sun4i_frontend.h | 102 +++-
 7 files changed, 500 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.c
 create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.h

diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0c2f8c7facae..b660d82011f4 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 sun4i-backend-y+= sun4i_backend.o sun4i_layer.o
+sun4i-frontend-y   += sun4i_frontend.o
 
 sun4i-drm-y+= sun4i_drv.o
 sun4i-drm-y+= sun4i_framebuffer.o
@@ -21,6 +22,6 @@ obj-$(CONFIG_DRM_SUN4I)   += sun4i-tcon.o
 obj-$(CONFIG_DRM_SUN4I)+= sun4i_tv.o
 obj-$(CONFIG_DRM_SUN4I)+= sun6i_drc.o
 
-obj-$(CONFIG_DRM_SUN4I_BACKEND)+= sun4i-backend.o
+obj-$(CONFIG_DRM_SUN4I_BACKEND)+= sun4i-backend.o sun4i-frontend.o
 obj-$(CONFIG_DRM_SUN4I_HDMI)   += sun4i-drm-hdmi.o
 obj-$(CONFIG_DRM_SUN8I_MIXER)  += sun8i-mixer.o
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index f971d3fb5ee4..e83e1fe43823 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -367,6 +367,11 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
if (backend->engine.id < 0)
return backend->engine.id;
 
+   backend->frontend = sun4i_backend_find_frontend(drv, dev->of_node);
+   if (IS_ERR(backend->frontend)) {
+   dev_err(dev, "Couldn't find matching frontend, frontend 
features disabled\n");
+   }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
regs = devm_ioremap_resource(dev, res);
if (IS_ERR(regs))
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h 
b/drivers/gpu/drm/sun4i/sun4i_backend.h
index ac3cc029f5cd..ba1410fd5410 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -145,6 +145,7 @@
 
 struct sun4i_backend {
struct sunxi_engine engine;
+   struct sun4i_frontend   *frontend;
 
struct reset_control*reset;
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 75c76cdd82bc..17bf9bfd98ba 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -98,6 +98,7 @@ static int sun4i_drv_bind(struct device *dev)
goto free_drm;
}
drm->dev_private = drv;
+   INIT_LIST_HEAD(>frontend_list);
INIT_LIST_HEAD(>engine_list);
INIT_LIST_HEAD(>tcon_list);
 
@@ -239,9 +240,11 @@ static int sun4i_drv_add_endpoints(struct device *dev,
int count = 0;
 
/*
-* We don't support the frontend for now, so we will never
-* have a device bound. Just skip over it, but we still want
-* the rest our pipeline to be added.
+* The frontend has been disabled in all of our old device
+* trees. If we find a node that is the frontend and is
+* disabled, we should just follow through and parse its
+* child, but without adding it to the component list.
+* Otherwise, we obviously want to add it to the list.
 */
if (!sun4i_drv_node_is_frontend(node) &&
!of_device_is_available(node))
@@ -254,7 +257,12 @@ static int sun4i_drv_add_endpoints(struct device *dev,
if (sun4i_drv_node_is_connector(node))
return 0;
 
-   if (!sun4i_drv_node_is_frontend(node)) {
+   /*
+* If the device is either just a regular device, or an
+* enabled frontend, we add it to our component list.
+*/
+   if (!sun4i_drv_node_is_frontend(node) ||
+   (sun4i_drv_node_is_frontend(node) && of_device_is_available(node))) 
{
/* Add current component */
DRM_DEBUG_DRIVER("Adding component %pOF\n", node);
drm_of_component_match_add(dev, match, compare_of, node);
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.h 
b/drivers/gpu/drm/sun4i/sun4i_drv.h
index a960c89270cc..9c26a345f85c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.h
+++