Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1210145 > unrolled thread

Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC x100 status descriptiors

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC  x100 status descriptiors Vinod Koul <vinod.koul@intel.com> - 2015-08-20 07:20 +0200
    Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC  x100 status descriptiors Sudeep Dutt <sudeep.dutt@intel.com> - 2015-08-20 19:40 +0200
      Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC  x100 status descriptiors Vinod Koul <vinod.koul@intel.com> - 2015-08-21 10:30 +0200
        Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC  x100 status descriptiors Sudeep Dutt <sudeep.dutt@intel.com> - 2015-08-21 18:20 +0200

#1210145 — Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC x100 status descriptiors

On Mon, Jul 27, 2015 at 04:58:17PM -0700, Ashutosh Dixit wrote:
> +/* Program a status descriptor with phys as address and value to be written */
> +static int mic_dma_do_status_update(struct mic_dma_chan *ch, dma_addr_t phys,
> +				    u64 value)
> +{
> +	int ret = mic_dma_avail_desc_ring_space(ch, 4);
> +
> +	if (ret < 0)
> +		return ret;
> +	ret = 0;
?
> +	mic_dma_prep_status_desc(&ch->desc_ring[ch->head],
> +				 value, phys, false);
> +	mic_dma_hw_ring_inc_head(ch);
> +	return ret;

return 0 then?

> +}
> +
>  static inline void mic_dma_issue_pending(struct dma_chan *ch)
>  {
>  	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> @@ -287,9 +310,28 @@ mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
>  		return NULL;
>  
>  	spin_lock(&mic_ch->prep_lock);
> +	if (len == 8) {
magic number?

> +		/*
> +		 * This is a hack to program status descriptor since
> +		 * DMA engine API doesn't have support for this.
> +		 */
what do you mean by programming status descriptor, what do you need to
program?

-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1210643

On Thu, 2015-08-20 at 10:50 +0530, Vinod Koul wrote:
> On Mon, Jul 27, 2015 at 04:58:17PM -0700, Ashutosh Dixit wrote:
> > +/* Program a status descriptor with phys as address and value to be written */
> > +static int mic_dma_do_status_update(struct mic_dma_chan *ch, dma_addr_t phys,
> > +				    u64 value)
> > +{
> > +	int ret = mic_dma_avail_desc_ring_space(ch, 4);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +	ret = 0;
> ?
> > +	mic_dma_prep_status_desc(&ch->desc_ring[ch->head],
> > +				 value, phys, false);
> > +	mic_dma_hw_ring_inc_head(ch);
> > +	return ret;
> 
> return 0 then?
> 

Sounds good. We will change this in the next revision of the patch
series which will be posted after the 4.3 merge window closes.

> > +}
> > +
> >  static inline void mic_dma_issue_pending(struct dma_chan *ch)
> >  {
> >  	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > @@ -287,9 +310,28 @@ mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
> >  		return NULL;
> >  
> >  	spin_lock(&mic_ch->prep_lock);
> > +	if (len == 8) {
> magic number?
> 

The MIC X100 DMA engine requires cache line (64 bytes) aligned
src/dst/len. We use the length of 8 bytes as a special case for
programming the status descriptor since it writes an 8 byte value. We
will use a macro in the next revision of the patch series.

> > +		/*
> > +		 * This is a hack to program status descriptor since
> > +		 * DMA engine API doesn't have support for this.
> > +		 */
> what do you mean by programming status descriptor, what do you need to
> program?
> 

The MIC X100 DMA engine has a special status descriptor which writes an
8 byte immediate data value to a destination address. It is used to
signal completion of all DMA descriptors prior to the status descriptor.
The DMA engine API does not allow drivers to pass a 8 byte value. We are
allowed to pass a source physical address but we cannot determine the
source value within the DMA driver using that information specifically
with the IOMMU enabled. We have added this workaround so that we can
program this special status update descriptor without making any changes
to the DMA engine API.

Thanks for the review!

Sudeep Dutt

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211010

On Thu, Aug 20, 2015 at 10:30:50AM -0700, Sudeep Dutt wrote:
> On Thu, 2015-08-20 at 10:50 +0530, Vinod Koul wrote:
> > On Mon, Jul 27, 2015 at 04:58:17PM -0700, Ashutosh Dixit wrote:

> > > +		/*
> > > +		 * This is a hack to program status descriptor since
> > > +		 * DMA engine API doesn't have support for this.
> > > +		 */
> > what do you mean by programming status descriptor, what do you need to
> > program?
> > 
> 
> The MIC X100 DMA engine has a special status descriptor which writes an
> 8 byte immediate data value to a destination address. It is used to
> signal completion of all DMA descriptors prior to the status descriptor.
> The DMA engine API does not allow drivers to pass a 8 byte value. We are
> allowed to pass a source physical address but we cannot determine the
> source value within the DMA driver using that information specifically
> with the IOMMU enabled. We have added this workaround so that we can
> program this special status update descriptor without making any changes
> to the DMA engine API.
I think this is a special case so you should export an API and let client
program using that API.

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211232

On Fri, 2015-08-21 at 13:59 +0530, Vinod Koul wrote:
> On Thu, Aug 20, 2015 at 10:30:50AM -0700, Sudeep Dutt wrote:
> > On Thu, 2015-08-20 at 10:50 +0530, Vinod Koul wrote:
> > > On Mon, Jul 27, 2015 at 04:58:17PM -0700, Ashutosh Dixit wrote:
> 
> > > > +		/*
> > > > +		 * This is a hack to program status descriptor since
> > > > +		 * DMA engine API doesn't have support for this.
> > > > +		 */
> > > what do you mean by programming status descriptor, what do you need to
> > > program?
> > > 
> > 
> > The MIC X100 DMA engine has a special status descriptor which writes an
> > 8 byte immediate data value to a destination address. It is used to
> > signal completion of all DMA descriptors prior to the status descriptor.
> > The DMA engine API does not allow drivers to pass a 8 byte value. We are
> > allowed to pass a source physical address but we cannot determine the
> > source value within the DMA driver using that information specifically
> > with the IOMMU enabled. We have added this workaround so that we can
> > program this special status update descriptor without making any changes
> > to the DMA engine API.
> I think this is a special case so you should export an API and let client
> program using that API.
> 

Sounds good. We will export a new API from the MIC X100 DMA driver in
the next revision of this patch series.

Thanks for the review!

Sudeep Dutt

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web