mbox series

[v4,0/7] sunxi: Add DT representation for the MBUS controller

Message ID cover.221220f2f28e14ef07cce84e753ac662085d84ff.1552595146.git-series.maxime.ripard@bootlin.com
Headers show
Series sunxi: Add DT representation for the MBUS controller | expand

Message

Maxime Ripard March 14, 2019, 8:25 p.m. UTC
Hi,

We've had for quite some time to hack around in our drivers to take into
account the fact that our DMA accesses are not done through the parent
node, but through another bus with a different mapping than the CPU for the
RAM (0 instead of 0x40000000 for most SoCs).

After some discussion after the submission of a camera device suffering of
the same hacks, I've decided to put together a serie that introduce a
special interconnect name called "dma" that that allows to express the DMA
relationship between a master and its bus, even if they are not direct
parents in the DT.

Let me know what you think,
Maxime

Changes from v3:
  - Rebased on top of current next
  - Use a function pointer instead of the parent device node to fix the
    recursive cases
  - Fix the usage of the device_node index in __of_get_dma_parent
  - Refer to the DT Specification for dma-ranges in the MBUS binding
  - Used dma-mem as the property instead of dma

Changes from v2:
  - Rewrite commit logs still mentionning dma-parent
  - Removed dma-parent-cells left over in the binding example
  - Removed dma-parent still being mentionned in comments

Changes from v1:
  - Change to use the now merged interconnect bindings
  - Move the DMA parent retrieval logic to its own function
  - Rebase on top of 5.0

Maxime Ripard (7):
  dt-bindings: interconnect: Add a dma interconnect name
  dt-bindings: bus: Add binding for the Allwinner MBUS controller
  of: address: Add parent pointer to the __of_translate_address args
  of: address: Add support for the parent DMA bus
  drm/sun4i: Rely on dma interconnect for our RAM offset
  clk: sunxi-ng: sun5i: Export the MBUS clock
  ARM: dts: sun5i: Add the MBUS controller

 Documentation/devicetree/bindings/arm/sunxi/sunxi-mbus.txt      | 36 ++++++-
 Documentation/devicetree/bindings/interconnect/interconnect.txt |  4 +-
 arch/arm/boot/dts/sun5i.dtsi                                    | 13 ++-
 drivers/clk/sunxi-ng/ccu-sun5i.h                                |  4 +-
 drivers/gpu/drm/sun4i/sun4i_backend.c                           | 28 +++--
 drivers/of/address.c                                            | 42 +++++--
 include/dt-bindings/clock/sun5i-ccu.h                           |  2 +-
 7 files changed, 110 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/sunxi/sunxi-mbus.txt

base-commit: cf08baa29613dd899954089e7cc7dba1d478b365

Comments

Robin Murphy March 27, 2019, 12:50 p.m. UTC | #1
On 14/03/2019 20:26, Maxime Ripard wrote:
> Now that we can express our DMA topology, rely on those property instead of
> hardcoding an offset from the dma_addr_t which wasn't really great.
> 
> We still need to add some code to deal with the old DT that would lack that
> property, but we move the offset to the DRM device dma_pfn_offset to be
> able to rely on just the dma_addr_t associated to the GEM object.

As the least-worst option,

Acked-by: Robin Murphy <robin.murphy@arm.com>

> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>   drivers/gpu/drm/sun4i/sun4i_backend.c | 28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index 4c0d51f73237..93f3cacc3e74 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -361,13 +361,6 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
>   	paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
>   	DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
>   
> -	/*
> -	 * backend DMA accesses DRAM directly, bypassing the system
> -	 * bus. As such, the address range is different and the buffer
> -	 * address needs to be corrected.
> -	 */
> -	paddr -= PHYS_OFFSET;
> -
>   	if (fb->format->is_yuv)
>   		return sun4i_backend_update_yuv_buffer(backend, fb, paddr);
>   
> @@ -814,6 +807,27 @@ static int sun4i_backend_bind(struct device *dev, struct device *master,
>   	dev_set_drvdata(dev, backend);
>   	spin_lock_init(&backend->frontend_lock);
>   
> +	if (of_find_property(dev->of_node, "interconnects", NULL)) {
> +		/*
> +		 * This assume we have the same DMA constraints for all our the
> +		 * devices in our pipeline (all the backends, but also the
> +		 * frontends). This sounds bad, but it has always been the case
> +		 * for us, and DRM doesn't do per-device allocation either, so
> +		 * we would need to fix DRM first...
> +		 */
> +		ret = of_dma_configure(drm->dev, dev->of_node, true);
> +		if (ret)
> +			return ret;
> +	} else {
> +		/*
> +		 * If we don't have the interconnect property, most likely
> +		 * because of an old DT, we need to set the DMA offset by hand
> +		 * on our device since the RAM mapping is at 0 for the DMA bus,
> +		 * unlike the CPU.
> +		 */
> +		drm->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
> +	}
> +
>   	backend->engine.node = dev->of_node;
>   	backend->engine.ops = &sun4i_backend_engine_ops;
>   	backend->engine.id = sun4i_backend_of_get_id(dev->of_node);
>