Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-17 Thread Chen-Yu Tsai 
On Thu, Jan 18, 2018 at 3:22 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
>> > 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 supported by the driver, we add it to our
>> > +* component list.
>> > +*/
>> > +   if (!sun4i_drv_node_is_frontend(node) ||
>> > +   (sun4i_drv_node_is_supported_frontend(node) &&
>> > +of_device_is_available(node))) {
>>
>> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
>> sun4i_drv_node_is_frontend, so of_device_is_available should always
>> be true at this point.
>
> That's not really the condition though :)
>
> It's if the device is *not* a frontend or if it is a supported
> frontend that is available, add it to the endpoints list.

Right. I got confused by the inverted logic. Sorry.

>
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
>> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
>> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
>>
>> Seems that you also need to set the "ALPHA_EN" bit for ARGB.
>
> I have not seen that bit documented anywhere. Where is it coming from?

The A31's user manual. I just checked all the datasheets, and only
the A31 and the A80 have this bit (bit 7) defined. It says if the
bit is cleared, alpha is replaced with 0xff. I assume it works either
way on the A33, or you wouldn't be suprised. Leave it out for now then.
(Or maybe a TODO note now that we know about it.)

ChenYu

>
>> > +   frontend->reset = devm_reset_control_get(dev, NULL);
>> > +   if (IS_ERR(frontend->reset)) {
>> > +   dev_err(dev, "Couldn't get our reset line\n");
>> > +   return PTR_ERR(frontend->reset);
>> > +   }
>> > +   reset_control_reset(frontend->reset);
>>
>> reset_control_reset leaves the reset control deasserted. At this
>> point the clock might not be running, which might mean the internal
>> state is not completely wiped out. (Though this really depends on
>> the design of the internal logic.)
>>
>> Maybe just assert it? It gets deasserted in the runtime PM callback
>> later. And just to be safe, I would move it close to the end of the
>> probe path, past all possible errors, so the hardware doesn't get
>> touched until everything is ready. Or don't touch it anywhere in
>> the probe path, and have the runtime PM resume function do a reset.
>
> That seems like the best solution yes.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-17 Thread Chen-Yu Tsai 
On Thu, Jan 18, 2018 at 3:22 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
>> > 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 supported by the driver, we add it to our
>> > +* component list.
>> > +*/
>> > +   if (!sun4i_drv_node_is_frontend(node) ||
>> > +   (sun4i_drv_node_is_supported_frontend(node) &&
>> > +of_device_is_available(node))) {
>>
>> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
>> sun4i_drv_node_is_frontend, so of_device_is_available should always
>> be true at this point.
>
> That's not really the condition though :)
>
> It's if the device is *not* a frontend or if it is a supported
> frontend that is available, add it to the endpoints list.

Right. I got confused by the inverted logic. Sorry.

>
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
>> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
>> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
>>
>> Seems that you also need to set the "ALPHA_EN" bit for ARGB.
>
> I have not seen that bit documented anywhere. Where is it coming from?

The A31's user manual. I just checked all the datasheets, and only
the A31 and the A80 have this bit (bit 7) defined. It says if the
bit is cleared, alpha is replaced with 0xff. I assume it works either
way on the A33, or you wouldn't be suprised. Leave it out for now then.
(Or maybe a TODO note now that we know about it.)

ChenYu

>
>> > +   frontend->reset = devm_reset_control_get(dev, NULL);
>> > +   if (IS_ERR(frontend->reset)) {
>> > +   dev_err(dev, "Couldn't get our reset line\n");
>> > +   return PTR_ERR(frontend->reset);
>> > +   }
>> > +   reset_control_reset(frontend->reset);
>>
>> reset_control_reset leaves the reset control deasserted. At this
>> point the clock might not be running, which might mean the internal
>> state is not completely wiped out. (Though this really depends on
>> the design of the internal logic.)
>>
>> Maybe just assert it? It gets deasserted in the runtime PM callback
>> later. And just to be safe, I would move it close to the end of the
>> probe path, past all possible errors, so the hardware doesn't get
>> touched until everything is ready. Or don't touch it anywhere in
>> the probe path, and have the runtime PM resume function do a reset.
>
> That seems like the best solution yes.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-17 Thread Maxime Ripard 
Hi,

On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
> > 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 supported by the driver, we add it to our
> > +* component list.
> > +*/
> > +   if (!sun4i_drv_node_is_frontend(node) ||
> > +   (sun4i_drv_node_is_supported_frontend(node) &&
> > +of_device_is_available(node))) {
> 
> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
> sun4i_drv_node_is_frontend, so of_device_is_available should always
> be true at this point.

That's not really the condition though :)

It's if the device is *not* a frontend or if it is a supported
frontend that is available, add it to the endpoints list.

> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
> 
> Seems that you also need to set the "ALPHA_EN" bit for ARGB.

I have not seen that bit documented anywhere. Where is it coming from?

> > +   frontend->reset = devm_reset_control_get(dev, NULL);
> > +   if (IS_ERR(frontend->reset)) {
> > +   dev_err(dev, "Couldn't get our reset line\n");
> > +   return PTR_ERR(frontend->reset);
> > +   }
> > +   reset_control_reset(frontend->reset);
> 
> reset_control_reset leaves the reset control deasserted. At this
> point the clock might not be running, which might mean the internal
> state is not completely wiped out. (Though this really depends on
> the design of the internal logic.)
> 
> Maybe just assert it? It gets deasserted in the runtime PM callback
> later. And just to be safe, I would move it close to the end of the
> probe path, past all possible errors, so the hardware doesn't get
> touched until everything is ready. Or don't touch it anywhere in
> the probe path, and have the runtime PM resume function do a reset.

That seems like the best solution yes.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-17 Thread Maxime Ripard 
Hi,

On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
> > 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 supported by the driver, we add it to our
> > +* component list.
> > +*/
> > +   if (!sun4i_drv_node_is_frontend(node) ||
> > +   (sun4i_drv_node_is_supported_frontend(node) &&
> > +of_device_is_available(node))) {
> 
> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
> sun4i_drv_node_is_frontend, so of_device_is_available should always
> be true at this point.

That's not really the condition though :)

It's if the device is *not* a frontend or if it is a supported
frontend that is available, add it to the endpoints list.

> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
> 
> Seems that you also need to set the "ALPHA_EN" bit for ARGB.

I have not seen that bit documented anywhere. Where is it coming from?

> > +   frontend->reset = devm_reset_control_get(dev, NULL);
> > +   if (IS_ERR(frontend->reset)) {
> > +   dev_err(dev, "Couldn't get our reset line\n");
> > +   return PTR_ERR(frontend->reset);
> > +   }
> > +   reset_control_reset(frontend->reset);
> 
> reset_control_reset leaves the reset control deasserted. At this
> point the clock might not be running, which might mean the internal
> state is not completely wiped out. (Though this really depends on
> the design of the internal logic.)
> 
> Maybe just assert it? It gets deasserted in the runtime PM callback
> later. And just to be safe, I would move it close to the end of the
> probe path, past all possible errors, so the hardware doesn't get
> touched until everything is ready. Or don't touch it anywhere in
> the probe path, and have the runtime PM resume function do a reset.

That seems like the best solution yes.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-17 Thread Chen-Yu Tsai 
On Tue, Jan 9, 2018 at 6:09 PM, 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_drv.c  |  27 +-
>  drivers/gpu/drm/sun4i/sun4i_drv.h  |   1 +-
>  drivers/gpu/drm/sun4i/sun4i_frontend.c | 384 ++-
>  drivers/gpu/drm/sun4i/sun4i_frontend.h |  99 +++-
>  5 files changed, 509 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_drv.c 
> b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 75c76cdd82bc..42e68cf3a2e8 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -23,6 +23,7 @@
>  #include 
>
>  #include "sun4i_drv.h"
> +#include "sun4i_frontend.h"
>  #include "sun4i_framebuffer.h"
>  #include "sun4i_tcon.h"
>
> @@ -98,6 +99,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);
>
> @@ -185,6 +187,14 @@ static bool sun4i_drv_node_is_frontend(struct 
> device_node *node)
> of_device_is_compatible(node, 
> "allwinner,sun8i-a33-display-frontend");
>  }
>
> +static bool sun4i_drv_node_is_supported_frontend(struct device_node *node)
> +{
> +   if (IS_ENABLED(CONFIG_DRM_SUN4I_BACKEND))
> +   return !!of_match_node(sun4i_frontend_of_table, node);
> +
> +   return false;
> +}
> +
>  static bool sun4i_drv_node_is_tcon(struct device_node *node)
>  {
> return of_device_is_compatible(node, "allwinner,sun4i-a10-tcon") ||
> @@ -239,9 +249,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 some 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 +266,14 @@ 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 supported by the driver, we add it to our
> +* component list.
> +*/
> +   if (!sun4i_drv_node_is_frontend(node) ||
> +   (sun4i_drv_node_is_supported_frontend(node) &&
> +of_device_is_available(node))) {

Nit: sun4i_drv_node_is_supported_frontend should be a subset of
sun4i_drv_node_is_frontend, so of_device_is_available should always
be true at this point.

> /* 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
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h
> @@ -19,6 +19,7 @@
>
>  struct sun4i_drv

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-17 Thread Chen-Yu Tsai 
On Tue, Jan 9, 2018 at 6:09 PM, 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_drv.c  |  27 +-
>  drivers/gpu/drm/sun4i/sun4i_drv.h  |   1 +-
>  drivers/gpu/drm/sun4i/sun4i_frontend.c | 384 ++-
>  drivers/gpu/drm/sun4i/sun4i_frontend.h |  99 +++-
>  5 files changed, 509 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_drv.c 
> b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 75c76cdd82bc..42e68cf3a2e8 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -23,6 +23,7 @@
>  #include 
>
>  #include "sun4i_drv.h"
> +#include "sun4i_frontend.h"
>  #include "sun4i_framebuffer.h"
>  #include "sun4i_tcon.h"
>
> @@ -98,6 +99,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);
>
> @@ -185,6 +187,14 @@ static bool sun4i_drv_node_is_frontend(struct 
> device_node *node)
> of_device_is_compatible(node, 
> "allwinner,sun8i-a33-display-frontend");
>  }
>
> +static bool sun4i_drv_node_is_supported_frontend(struct device_node *node)
> +{
> +   if (IS_ENABLED(CONFIG_DRM_SUN4I_BACKEND))
> +   return !!of_match_node(sun4i_frontend_of_table, node);
> +
> +   return false;
> +}
> +
>  static bool sun4i_drv_node_is_tcon(struct device_node *node)
>  {
> return of_device_is_compatible(node, "allwinner,sun4i-a10-tcon") ||
> @@ -239,9 +249,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 some 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 +266,14 @@ 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 supported by the driver, we add it to our
> +* component list.
> +*/
> +   if (!sun4i_drv_node_is_frontend(node) ||
> +   (sun4i_drv_node_is_supported_frontend(node) &&
> +of_device_is_available(node))) {

Nit: sun4i_drv_node_is_supported_frontend should be a subset of
sun4i_drv_node_is_frontend, so of_device_is_available should always
be true at this point.

> /* 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
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h
> @@ -19,6 +19,7 @@
>
>  struct sun4i_drv {
> struct list_headengine_list;
> +   struct

[PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-09 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_drv.c  |  27 +-
 drivers/gpu/drm/sun4i/sun4i_drv.h  |   1 +-
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 384 ++-
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  99 +++-
 5 files changed, 509 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_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 75c76cdd82bc..42e68cf3a2e8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -23,6 +23,7 @@
 #include 
 
 #include "sun4i_drv.h"
+#include "sun4i_frontend.h"
 #include "sun4i_framebuffer.h"
 #include "sun4i_tcon.h"
 
@@ -98,6 +99,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);
 
@@ -185,6 +187,14 @@ static bool sun4i_drv_node_is_frontend(struct device_node 
*node)
of_device_is_compatible(node, 
"allwinner,sun8i-a33-display-frontend");
 }
 
+static bool sun4i_drv_node_is_supported_frontend(struct device_node *node)
+{
+   if (IS_ENABLED(CONFIG_DRM_SUN4I_BACKEND))
+   return !!of_match_node(sun4i_frontend_of_table, node);
+
+   return false;
+}
+
 static bool sun4i_drv_node_is_tcon(struct device_node *node)
 {
return of_device_is_compatible(node, "allwinner,sun4i-a10-tcon") ||
@@ -239,9 +249,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 some 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 +266,14 @@ 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 supported by the driver, we add it to our
+* component list.
+*/
+   if (!sun4i_drv_node_is_frontend(node) ||
+   (sun4i_drv_node_is_supported_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
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.h
@@ -19,6 +19,7 @@
 
 struct sun4i_drv {
struct list_headengine_list;
+   struct list_headfrontend_list;
struct list_headtcon_list;
 
struct drm_fbdev_cma*fbdev;
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c 
b/drivers/gpu/drm/sun4i/sun4i_frontend.c
new file mode 100644
index ..c19238cec1b7
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *

[PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

2018-01-09 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_drv.c  |  27 +-
 drivers/gpu/drm/sun4i/sun4i_drv.h  |   1 +-
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 384 ++-
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  99 +++-
 5 files changed, 509 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_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 75c76cdd82bc..42e68cf3a2e8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -23,6 +23,7 @@
 #include 
 
 #include "sun4i_drv.h"
+#include "sun4i_frontend.h"
 #include "sun4i_framebuffer.h"
 #include "sun4i_tcon.h"
 
@@ -98,6 +99,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);
 
@@ -185,6 +187,14 @@ static bool sun4i_drv_node_is_frontend(struct device_node 
*node)
of_device_is_compatible(node, 
"allwinner,sun8i-a33-display-frontend");
 }
 
+static bool sun4i_drv_node_is_supported_frontend(struct device_node *node)
+{
+   if (IS_ENABLED(CONFIG_DRM_SUN4I_BACKEND))
+   return !!of_match_node(sun4i_frontend_of_table, node);
+
+   return false;
+}
+
 static bool sun4i_drv_node_is_tcon(struct device_node *node)
 {
return of_device_is_compatible(node, "allwinner,sun4i-a10-tcon") ||
@@ -239,9 +249,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 some 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 +266,14 @@ 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 supported by the driver, we add it to our
+* component list.
+*/
+   if (!sun4i_drv_node_is_frontend(node) ||
+   (sun4i_drv_node_is_supported_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
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.h
@@ -19,6 +19,7 @@
 
 struct sun4i_drv {
struct list_headengine_list;
+   struct list_headfrontend_list;
struct list_headtcon_list;
 
struct drm_fbdev_cma*fbdev;
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c 
b/drivers/gpu/drm/sun4i/sun4i_frontend.c
new file mode 100644
index ..c19238cec1b7
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2017 Free Electrons
+