[linux-sunxi] Re: [PATCH v4 06/11] drm/sun4i: add support for Allwinner DE2 mixers

2017-04-21 Thread icenowy


于 2017年4月20日 GMT+08:00 下午4:37:07, Maxime Ripard 
 写到:

On Tue, Apr 18, 2017 at 06:47:56PM +0800, Icenowy Zheng wrote:

>> +  /* Get the physical address of the buffer in memory */
>> +  gem = drm_fb_cma_get_gem_obj(fb, 0);
>> +
>> +  DRM_DEBUG_DRIVER("Using GEM @ %pad\n", &gem->paddr);
>> +
>> +  /* Compute the start of the displayed memory */
>> +  bpp = fb->format->cpp[0];
>> +  paddr = gem->paddr + fb->offsets[0];
>> +  paddr += (state->src_x >> 16) * bpp;
>> +  paddr += (state->src_y >> 16) * fb->pitches[0];
>> +
>> +  DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
>> +
>> +  paddr_u32 = (uint32_t) paddr;
>
>How does that work on 64-bits systems ?

The hardware is not designed to work on 64-bit systems.

Even 64-bit A64/H5 has also 3GiB memory limit.


That's a fragile assumption.


Yes, it's only the basical reason.




The address cell in mixer hardware is also only 32-bit.

So we should just keep the force conversion here. If we then really
met 4GiB-capable AW SoC without changing DE2, I think we should have
other way to limit CMA pool inside 4GiB.


The register name looks like this is only the lower 32 bits that you
can set here, and that there is another register for the upper 32 bits
of that address somewhere.


Maybe... but no one can verify this as their's no currently any user
which has 4GiB+ DRAM.

I think we should keep this until Allwinner really made a 4GiB-capable
hardware.



In that case, please use the lower_32_bits and upper_32_bits helper,
and don't cast it that way.

If it isn't the case, you should set the DMA mask (through
dma_set_mask) so that we only allocate memory that can be accessed by
this device.


How to do it?



Maxime


--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v4 06/11] drm/sun4i: add support for Allwinner DE2 mixers

2017-04-20 Thread Maxime Ripard
On Tue, Apr 18, 2017 at 06:47:56PM +0800, Icenowy Zheng wrote:
> >> +  /* Get the physical address of the buffer in memory */
> >> +  gem = drm_fb_cma_get_gem_obj(fb, 0);
> >> +
> >> +  DRM_DEBUG_DRIVER("Using GEM @ %pad\n", &gem->paddr);
> >> +
> >> +  /* Compute the start of the displayed memory */
> >> +  bpp = fb->format->cpp[0];
> >> +  paddr = gem->paddr + fb->offsets[0];
> >> +  paddr += (state->src_x >> 16) * bpp;
> >> +  paddr += (state->src_y >> 16) * fb->pitches[0];
> >> +
> >> +  DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
> >> +
> >> +  paddr_u32 = (uint32_t) paddr;
> >
> >How does that work on 64-bits systems ?
> 
> The hardware is not designed to work on 64-bit systems.
> 
> Even 64-bit A64/H5 has also 3GiB memory limit.

That's a fragile assumption.

> The address cell in mixer hardware is also only 32-bit.
> 
> So we should just keep the force conversion here. If we then really
> met 4GiB-capable AW SoC without changing DE2, I think we should have
> other way to limit CMA pool inside 4GiB.

The register name looks like this is only the lower 32 bits that you
can set here, and that there is another register for the upper 32 bits
of that address somewhere.

In that case, please use the lower_32_bits and upper_32_bits helper,
and don't cast it that way.

If it isn't the case, you should set the DMA mask (through
dma_set_mask) so that we only allocate memory that can be accessed by
this device.

Maxime

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature


[linux-sunxi] Re: [PATCH v4 06/11] drm/sun4i: add support for Allwinner DE2 mixers

2017-04-18 Thread Icenowy Zheng


于 2017年4月18日 GMT+08:00 下午5:00:47, Maxime Ripard 
 写到:
>On Sun, Apr 16, 2017 at 08:08:44PM +0800, Icenowy Zheng wrote:
>> Allwinner have a new "Display Engine 2.0" in their new SoCs, which
>comes
>> with mixers to do graphic processing and feed data to TCON, like the
>old
>> backends and frontends.
>> 
>> Add support for the mixer on Allwinner V3s SoC; it's the simplest
>one.
>> 
>> Currently a lot of functions are still missing -- more investigations
>> are needed to gain enough information for them.
>> 
>> Signed-off-by: Icenowy Zheng 
>> ---
>> Changes in v4:
>> - Killed some dead code according to Jernej.
>> 
>>  drivers/gpu/drm/sun4i/Kconfig   |  10 +
>>  drivers/gpu/drm/sun4i/Makefile  |   4 +
>>  drivers/gpu/drm/sun4i/sun8i_layer.c | 142 ++
>>  drivers/gpu/drm/sun4i/sun8i_layer.h |  36 
>>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 381
>
>>  drivers/gpu/drm/sun4i/sun8i_mixer.h | 131 +
>>  6 files changed, 704 insertions(+)
>>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_layer.c
>>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_layer.h
>>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_mixer.c
>>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_mixer.h
>> 
>> diff --git a/drivers/gpu/drm/sun4i/Kconfig
>b/drivers/gpu/drm/sun4i/Kconfig
>> index 5a8227f37cc4..15557484520d 100644
>> --- a/drivers/gpu/drm/sun4i/Kconfig
>> +++ b/drivers/gpu/drm/sun4i/Kconfig
>> @@ -22,3 +22,13 @@ config DRM_SUN4I_BACKEND
>>original Allwinner Display Engine, which has a backend to
>>do some alpha blending and feed graphics to TCON. If M is
>>selected the module will be called sun4i-backend.
>> +
>> +config DRM_SUN4I_SUN8I_MIXER
>> +tristate "Support for Allwinner Display Engine 2.0 Mixer"
>> +depends on DRM_SUN4I
>> +default MACH_SUN8I
>> +help
>> +  Choose this option if you have an Allwinner SoC with the
>> +  Allwinner Display Engine 2.0, which has a mixer to do some
>> +  graphics mixture and feed graphics to TCON, If M is
>> +  selected the module will be called sun8i-mixer.
>> diff --git a/drivers/gpu/drm/sun4i/Makefile
>b/drivers/gpu/drm/sun4i/Makefile
>> index 1db1068b9be1..7625c2dad1bb 100644
>> --- a/drivers/gpu/drm/sun4i/Makefile
>> +++ b/drivers/gpu/drm/sun4i/Makefile
>> @@ -9,7 +9,11 @@ sun4i-tcon-y += sun4i_crtc.o
>>  sun4i-backend-y += sun4i_layer.o
>>  sun4i-backend-y += sun4i_backend.o
>>  
>> +sun8i-mixer-y += sun8i_layer.o
>> +sun8i-mixer-y += sun8i_mixer.o
>> +
>>  obj-$(CONFIG_DRM_SUN4I) += sun4i-drm.o sun4i-tcon.o
>>  obj-$(CONFIG_DRM_SUN4I_BACKEND) += sun4i-backend.o
>> +obj-$(CONFIG_DRM_SUN4I_SUN8I_MIXER) += sun8i-mixer.o
>
>Please align the value using tabs.

Should I re-align existed items?

>
>>  obj-$(CONFIG_DRM_SUN4I) += sun6i_drc.o
>>  obj-$(CONFIG_DRM_SUN4I) += sun4i_tv.o
>> diff --git a/drivers/gpu/drm/sun4i/sun8i_layer.c
>b/drivers/gpu/drm/sun4i/sun8i_layer.c
>> new file mode 100644
>> index ..d70a90d963b0
>> --- /dev/null
>> +++ b/drivers/gpu/drm/sun4i/sun8i_layer.c
>> @@ -0,0 +1,142 @@
>> +/*
>> + * Copyright (C) Icenowy Zheng 
>> + *
>> + * Based on sun4i_layer.h, which is:
>> + *   Copyright (C) 2015 Free Electrons
>> + *   Copyright (C) 2015 NextThing Co
>> + *
>> + *   Maxime Ripard 
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "sun4i_crtc.h"
>> +#include "sun8i_layer.h"
>> +#include "sun8i_mixer.h"
>> +
>> +struct sun8i_plane_desc {
>> +   enum drm_plane_type type;
>> +   const uint32_t  *formats;
>> +   uint32_tnformats;
>> +};
>> +
>> +static int sun8i_mixer_layer_atomic_check(struct drm_plane *plane,
>> +struct drm_plane_state *state)
>> +{
>> +return 0;
>> +}
>> +
>> +static void sun8i_mixer_layer_atomic_disable(struct drm_plane
>*plane,
>> +   struct drm_plane_state 
>> *old_state)
>> +{
>> +struct sun8i_layer *layer = plane_to_sun8i_layer(plane);
>> +struct sun8i_mixer *mixer = layer->mixer;
>> +
>> +sun8i_mixer_layer_enable(mixer, layer->id, false);
>> +}
>> +
>> +static void sun8i_mixer_layer_atomic_update(struct drm_plane *plane,
>> +  struct drm_plane_state *old_state)
>> +{
>> +struct sun8i_layer *layer = plane_to_sun8i_layer(plane);
>> +struct sun8i_mixer *mixer = layer->mixer;
>> +
>> +sun8i_mixer_update_layer_coord(mixer, layer->id, plane);
>> +sun8i_mixer_update_layer_formats(mixer, layer->id, plane);
>> +sun8i_mixer_update_layer_buffer(mixer, layer->id, plane);
>> +sun8i_mi

[linux-sunxi] Re: [PATCH v4 06/11] drm/sun4i: add support for Allwinner DE2 mixers

2017-04-18 Thread Maxime Ripard
On Sun, Apr 16, 2017 at 08:08:44PM +0800, Icenowy Zheng wrote:
> Allwinner have a new "Display Engine 2.0" in their new SoCs, which comes
> with mixers to do graphic processing and feed data to TCON, like the old
> backends and frontends.
> 
> Add support for the mixer on Allwinner V3s SoC; it's the simplest one.
> 
> Currently a lot of functions are still missing -- more investigations
> are needed to gain enough information for them.
> 
> Signed-off-by: Icenowy Zheng 
> ---
> Changes in v4:
> - Killed some dead code according to Jernej.
> 
>  drivers/gpu/drm/sun4i/Kconfig   |  10 +
>  drivers/gpu/drm/sun4i/Makefile  |   4 +
>  drivers/gpu/drm/sun4i/sun8i_layer.c | 142 ++
>  drivers/gpu/drm/sun4i/sun8i_layer.h |  36 
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 381 
> 
>  drivers/gpu/drm/sun4i/sun8i_mixer.h | 131 +
>  6 files changed, 704 insertions(+)
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_layer.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_layer.h
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_mixer.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_mixer.h
> 
> diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
> index 5a8227f37cc4..15557484520d 100644
> --- a/drivers/gpu/drm/sun4i/Kconfig
> +++ b/drivers/gpu/drm/sun4i/Kconfig
> @@ -22,3 +22,13 @@ config DRM_SUN4I_BACKEND
> original Allwinner Display Engine, which has a backend to
> do some alpha blending and feed graphics to TCON. If M is
> selected the module will be called sun4i-backend.
> +
> +config DRM_SUN4I_SUN8I_MIXER
> + tristate "Support for Allwinner Display Engine 2.0 Mixer"
> + depends on DRM_SUN4I
> + default MACH_SUN8I
> + help
> +   Choose this option if you have an Allwinner SoC with the
> +   Allwinner Display Engine 2.0, which has a mixer to do some
> +   graphics mixture and feed graphics to TCON, If M is
> +   selected the module will be called sun8i-mixer.
> diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> index 1db1068b9be1..7625c2dad1bb 100644
> --- a/drivers/gpu/drm/sun4i/Makefile
> +++ b/drivers/gpu/drm/sun4i/Makefile
> @@ -9,7 +9,11 @@ sun4i-tcon-y += sun4i_crtc.o
>  sun4i-backend-y += sun4i_layer.o
>  sun4i-backend-y += sun4i_backend.o
>  
> +sun8i-mixer-y += sun8i_layer.o
> +sun8i-mixer-y += sun8i_mixer.o
> +
>  obj-$(CONFIG_DRM_SUN4I)  += sun4i-drm.o sun4i-tcon.o
>  obj-$(CONFIG_DRM_SUN4I_BACKEND)  += sun4i-backend.o
> +obj-$(CONFIG_DRM_SUN4I_SUN8I_MIXER) += sun8i-mixer.o

Please align the value using tabs.

>  obj-$(CONFIG_DRM_SUN4I)  += sun6i_drc.o
>  obj-$(CONFIG_DRM_SUN4I)  += sun4i_tv.o
> diff --git a/drivers/gpu/drm/sun4i/sun8i_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_layer.c
> new file mode 100644
> index ..d70a90d963b0
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun8i_layer.c
> @@ -0,0 +1,142 @@
> +/*
> + * Copyright (C) Icenowy Zheng 
> + *
> + * Based on sun4i_layer.h, which is:
> + *   Copyright (C) 2015 Free Electrons
> + *   Copyright (C) 2015 NextThing Co
> + *
> + *   Maxime Ripard 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "sun4i_crtc.h"
> +#include "sun8i_layer.h"
> +#include "sun8i_mixer.h"
> +
> +struct sun8i_plane_desc {
> +enum drm_plane_type type;
> +const uint32_t  *formats;
> +uint32_tnformats;
> +};
> +
> +static int sun8i_mixer_layer_atomic_check(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + return 0;
> +}
> +
> +static void sun8i_mixer_layer_atomic_disable(struct drm_plane *plane,
> +struct drm_plane_state 
> *old_state)
> +{
> + struct sun8i_layer *layer = plane_to_sun8i_layer(plane);
> + struct sun8i_mixer *mixer = layer->mixer;
> +
> + sun8i_mixer_layer_enable(mixer, layer->id, false);
> +}
> +
> +static void sun8i_mixer_layer_atomic_update(struct drm_plane *plane,
> +   struct drm_plane_state *old_state)
> +{
> + struct sun8i_layer *layer = plane_to_sun8i_layer(plane);
> + struct sun8i_mixer *mixer = layer->mixer;
> +
> + sun8i_mixer_update_layer_coord(mixer, layer->id, plane);
> + sun8i_mixer_update_layer_formats(mixer, layer->id, plane);
> + sun8i_mixer_update_layer_buffer(mixer, layer->id, plane);
> + sun8i_mixer_layer_enable(mixer, layer->id, true);
> +}
> +
> +static struct drm_plane_helper_funcs sun8i_mixer_layer_helper_funcs = {
> + .atomic_check   = sun8i_mixer_layer_atomic_check,
> +