Re: [Nouveau] [PATCH 07/17] 53c700: improve non-coherent DMA handling

2020-09-15 Thread James Bottomley
On Tue, 2020-09-15 at 08:27 +0200, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 08:20:18AM -0700, James Bottomley wrote:
> > If you're going to change the macros from taking a device to taking
> > a hostdata structure then the descriptive argument name needs to
> > change ... it can't be dev anymore.  I'm happy with it simply
> > becoming 'h' if hostdata is too long.
> > 
> > I already asked for this on the first go around:
> 
> And I did rename them, those hunks just accidentally slipped into
> patch 12 instead of this one.  Fixed for the next versions.

Ah, yes, found it ... thanks for doing that!

James

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH 07/17] 53c700: improve non-coherent DMA handling

2020-09-15 Thread Christoph Hellwig
On Mon, Sep 14, 2020 at 08:20:18AM -0700, James Bottomley wrote:
> If you're going to change the macros from taking a device to taking a
> hostdata structure then the descriptive argument name needs to change
> ... it can't be dev anymore.  I'm happy with it simply becoming 'h' if
> hostdata is too long.
> 
> I already asked for this on the first go around:

And I did rename them, those hunks just accidentally slipped into patch
12 instead of this one.  Fixed for the next versions.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH 07/17] 53c700: improve non-coherent DMA handling

2020-09-14 Thread Christoph Hellwig
Switch the 53c700 driver to only use non-coherent descriptor memory if it
really has to because dma_alloc_coherent fails.  This doesn't matter for
any of the platforms it runs on currently, but that will change soon.

To help with this two new helpers to transfer ownership to and from the
device are added that abstract the syncing of the non-coherent memory.
The two current bidirectional cases are mapped to transfers to the
device, as that appears to what they are used for.  Note that for parisc,
which is the only architecture this driver needs to use non-coherent
memory on, the direction argument of dma_cache_sync is ignored, so this
will not change behavior in any way.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/53c700.c | 113 +++---
 drivers/scsi/53c700.h |   9 ++--
 2 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 84b57a8f86bfa9..c59226d7e2f6b5 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -269,6 +269,20 @@ NCR_700_get_SXFER(struct scsi_device *SDp)
  spi_period(SDp->sdev_target));
 }
 
+static inline void dma_sync_to_dev(struct NCR_700_Host_Parameters *h,
+   void *addr, size_t size)
+{
+   if (h->noncoherent)
+   dma_cache_sync(h->dev, addr, size, DMA_TO_DEVICE);
+}
+
+static inline void dma_sync_from_dev(struct NCR_700_Host_Parameters *h,
+   void *addr, size_t size)
+{
+   if (h->noncoherent)
+   dma_cache_sync(h->dev, addr, size, DMA_FROM_DEVICE);
+}
+
 struct Scsi_Host *
 NCR_700_detect(struct scsi_host_template *tpnt,
   struct NCR_700_Host_Parameters *hostdata, struct device *dev)
@@ -283,9 +297,13 @@ NCR_700_detect(struct scsi_host_template *tpnt,
if(tpnt->sdev_attrs == NULL)
tpnt->sdev_attrs = NCR_700_dev_attrs;
 
-   memory = dma_alloc_attrs(dev, TOTAL_MEM_SIZE, &pScript,
-GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
-   if(memory == NULL) {
+   memory = dma_alloc_coherent(dev, TOTAL_MEM_SIZE, &pScript, GFP_KERNEL);
+   if (!memory) {
+   hostdata->noncoherent = 1;
+   memory = dma_alloc_attrs(dev, TOTAL_MEM_SIZE, &pScript,
+GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
+   }
+   if (!memory) {
printk(KERN_ERR "53c700: Failed to allocate memory for driver, 
detaching\n");
return NULL;
}
@@ -339,11 +357,11 @@ NCR_700_detect(struct scsi_host_template *tpnt,
for (j = 0; j < PATCHES; j++)
script[LABELPATCHES[j]] = bS_to_host(pScript + 
SCRIPT[LABELPATCHES[j]]);
/* now patch up fixed addresses. */
-   script_patch_32(hostdata->dev, script, MessageLocation,
+   script_patch_32(hostdata, script, MessageLocation,
pScript + MSGOUT_OFFSET);
-   script_patch_32(hostdata->dev, script, StatusAddress,
+   script_patch_32(hostdata, script, StatusAddress,
pScript + STATUS_OFFSET);
-   script_patch_32(hostdata->dev, script, ReceiveMsgAddress,
+   script_patch_32(hostdata, script, ReceiveMsgAddress,
pScript + MSGIN_OFFSET);
 
hostdata->script = script;
@@ -395,8 +413,12 @@ NCR_700_release(struct Scsi_Host *host)
struct NCR_700_Host_Parameters *hostdata = 
(struct NCR_700_Host_Parameters *)host->hostdata[0];
 
-   dma_free_attrs(hostdata->dev, TOTAL_MEM_SIZE, hostdata->script,
-  hostdata->pScript, DMA_ATTR_NON_CONSISTENT);
+   if (hostdata->noncoherent)
+   dma_free_attrs(hostdata->dev, TOTAL_MEM_SIZE, hostdata->script,
+  hostdata->pScript, DMA_ATTR_NON_CONSISTENT);
+   else
+   dma_free_coherent(hostdata->dev, TOTAL_MEM_SIZE,
+ hostdata->script, hostdata->pScript);
return 1;
 }
 
@@ -804,8 +826,8 @@ process_extended_message(struct Scsi_Host *host,
shost_printk(KERN_WARNING, host,
"Unexpected SDTR msg\n");
hostdata->msgout[0] = A_REJECT_MSG;
-   dma_cache_sync(hostdata->dev, hostdata->msgout, 1, 
DMA_TO_DEVICE);
-   script_patch_16(hostdata->dev, hostdata->script,
+   dma_sync_to_dev(hostdata, hostdata->msgout, 1);
+   script_patch_16(hostdata, hostdata->script,
MessageCount, 1);
/* SendMsgOut returns, so set up the return
 * address */
@@ -817,9 +839,8 @@ process_extended_message(struct Scsi_Host *host,
printk(KERN_INFO "scsi%d: (%d:%d), Unsolicited WDTR after CMD, 
Rejecting\n",
   host->host_no, pun, lun);
hostdata->msgout[0] 

Re: [Nouveau] [PATCH 07/17] 53c700: improve non-coherent DMA handling

2020-09-14 Thread James Bottomley
On Mon, 2020-09-14 at 16:44 +0200, Christoph Hellwig wrote:
> @@ -429,7 +430,7 @@ struct NCR_700_Host_Parameters {
>   for(i=0; i< (sizeof(A_##symbol##_used) / sizeof(__u32));
> i++) { \
>   __u32 val =
> bS_to_cpu((script)[A_##symbol##_used[i]]) + da; \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching %s at %d to %pad\n", \
>  #symbol, A_##symbol##_used[i], &da)); \
>   } \
> @@ -441,7 +442,7 @@ struct NCR_700_Host_Parameters {
>   dma_addr_t da = value; \
>   for(i=0; i< (sizeof(A_##symbol##_used) / sizeof(__u32));
> i++) { \
>   (script)[A_##symbol##_used[i]] = bS_to_host(da); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching %s at %d to %pad\n", \
>  #symbol, A_##symbol##_used[i], &da)); \
>   } \
> @@ -456,7 +457,7 @@ struct NCR_700_Host_Parameters {
>   val &= 0xff00; \
>   val |= ((value) & 0xff) << 16; \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching ID field %s at %d to
> 0x%x\n", \
>  #symbol, A_##symbol##_used[i], val)); \
>   } \
> @@ -470,7 +471,7 @@ struct NCR_700_Host_Parameters {
>   val &= 0x; \
>   val |= ((value) & 0x); \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching short field %s at %d to
> 0x%x\n", \
>  #symbol, A_##symbol##_used[i], val)); \
>   } \

If you're going to change the macros from taking a device to taking a
hostdata structure then the descriptive argument name needs to change
... it can't be dev anymore.  I'm happy with it simply becoming 'h' if
hostdata is too long.

I already asked for this on the first go around:

https://lore.kernel.org/alsa-devel/1598971960.4238.5.ca...@hansenpartnership.com/

James

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau