Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 7 Jun 2021 17:04:57 +0530
From:   Vinod Koul <vkoul@kernel.org>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Serge Semin <Sergey.Semin@baikalelectronics.ru>,
        dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
        Viresh Kumar <vireshk@kernel.org>
Subject: Re: [PATCH v1 1/1] dmaengine: dw: Program xBAR hardware for Elkhart
 Lake
Message-ID: <YL4EYb35GOVYxdQO@vkoul-mobl>
References: <20210602085604.21933-1-andriy.shevchenko@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210602085604.21933-1-andriy.shevchenko@linux.intel.com>
Precedence: bulk

On 02-06-21, 11:56, Andy Shevchenko wrote:
> Intel Elkhart Lake PSE DMA implementation is integrated with crossbar IP
> in order to serve more hardware than there are DMA request lines available.
> 
> Due to this, program xBAR hardware to make flexible support of PSE peripheral.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/dma/dw/idma32.c              | 146 ++++++++++++++++++++++++++-
>  drivers/dma/dw/internal.h            |  16 +++
>  drivers/dma/dw/pci.c                 |   6 +-
>  drivers/dma/dw/platform.c            |   6 +-
>  include/linux/platform_data/dma-dw.h |   3 +
>  5 files changed, 168 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma/dw/idma32.c b/drivers/dma/dw/idma32.c
> index 3ce44de25d33..5232fcb1a736 100644
> --- a/drivers/dma/dw/idma32.c
> +++ b/drivers/dma/dw/idma32.c
> @@ -1,15 +1,152 @@
>  // SPDX-License-Identifier: GPL-2.0
> -// Copyright (C) 2013,2018 Intel Corporation
> +// Copyright (C) 2013,2018,2020 Intel Corporation

2021..?

>  
>  #include <linux/bitops.h>
>  #include <linux/dmaengine.h>
>  #include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
>  #include "internal.h"
>  
> -static void idma32_initialize_chan(struct dw_dma_chan *dwc)
> +#define DMA_CTL_CH(x)			(0x1000 + (x) * 4)
> +#define DMA_SRC_ADDR_FILLIN(x)		(0x1100 + (x) * 4)
> +#define DMA_DST_ADDR_FILLIN(x)		(0x1200 + (x) * 4)
> +#define DMA_XBAR_SEL(x)			(0x1300 + (x) * 4)
> +#define DMA_REGACCESS_CHID_CFG		(0x1400)
> +
> +#define CTL_CH_TRANSFER_MODE_MASK	GENMASK(1, 0)
> +#define CTL_CH_TRANSFER_MODE_S2S	0
> +#define CTL_CH_TRANSFER_MODE_S2D	1
> +#define CTL_CH_TRANSFER_MODE_D2S	2
> +#define CTL_CH_TRANSFER_MODE_D2D	3
> +#define CTL_CH_RD_RS_MASK		GENMASK(4, 3)
> +#define CTL_CH_WR_RS_MASK		GENMASK(6, 5)
> +#define CTL_CH_RD_NON_SNOOP_BIT		BIT(8)
> +#define CTL_CH_WR_NON_SNOOP_BIT		BIT(9)
> +
> +#define XBAR_SEL_DEVID_MASK		GENMASK(15, 0)
> +#define XBAR_SEL_RX_TX_BIT		BIT(16)
> +#define XBAR_SEL_RX_TX_SHIFT		16
> +
> +#define REGACCESS_CHID_MASK		GENMASK(2, 0)
> +
> +static unsigned int idma32_get_slave_devid(struct dw_dma_chan *dwc)
> +{
> +	struct device *slave = dwc->chan.slave;
> +
> +	if (!slave || !dev_is_pci(slave))
> +		return 0;
> +
> +	return to_pci_dev(slave)->devfn;

so this return devfn.. maybe rename function to get_slave_devfn() ?

> +}
> +
> +static void idma32_initialize_chan_xbar(struct dw_dma_chan *dwc)
> +{
> +	struct dw_dma *dw = to_dw_dma(dwc->chan.device);
> +	void __iomem *misc = __dw_regs(dw);
> +	u32 cfghi = 0, cfglo = 0;
> +	u8 dst_id, src_id;
> +	u32 value;
> +
> +	/* DMA Channel ID Configuration register must be programmed first */
> +	value = readl(misc + DMA_REGACCESS_CHID_CFG);
> +
> +	value &= ~REGACCESS_CHID_MASK;
> +	value |= dwc->chan.chan_id;
> +
> +	writel(value, misc + DMA_REGACCESS_CHID_CFG);
> +
> +	/* Configure channel attributes */
> +	value = readl(misc + DMA_CTL_CH(dwc->chan.chan_id));
> +
> +	value &= ~(CTL_CH_RD_NON_SNOOP_BIT | CTL_CH_WR_NON_SNOOP_BIT);
> +	value &= ~(CTL_CH_RD_RS_MASK | CTL_CH_WR_RS_MASK);
> +	value &= ~CTL_CH_TRANSFER_MODE_MASK;
> +
> +	switch (dwc->direction) {
> +	case DMA_MEM_TO_MEM:
> +		value |= CTL_CH_TRANSFER_MODE_D2D;
> +		break;
> +	case DMA_MEM_TO_DEV:
> +		value |= CTL_CH_TRANSFER_MODE_D2S;
> +		value |= CTL_CH_WR_NON_SNOOP_BIT;
> +		break;
> +	case DMA_DEV_TO_MEM:
> +		value |= CTL_CH_TRANSFER_MODE_S2D;
> +		value |= CTL_CH_RD_NON_SNOOP_BIT;
> +		break;
> +	case DMA_DEV_TO_DEV:
> +	default:
> +		value |= CTL_CH_WR_NON_SNOOP_BIT | CTL_CH_RD_NON_SNOOP_BIT;
> +		value |= CTL_CH_TRANSFER_MODE_S2S;
> +		break;

aha, how did you test this...

-- 
~Vinod