[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5665893.fe4E5jvxvE@avalon>
Date: Thu, 03 Nov 2016 17:12:09 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Peter Ujfalusi <peter.ujfalusi@...com>
Cc: mchehab@....samsung.com, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] media: omap3isp: Use dma_request_chan() to requesting DMA channel
Hi Peter,
On Thursday 03 Nov 2016 15:14:19 Peter Ujfalusi wrote:
> On 11/03/2016 11:23 AM, Peter Ujfalusi wrote:
> > On 11/02/2016 11:19 PM, Laurent Pinchart wrote:
> >>> On Wednesday 02 Nov 2016 14:39:59 Peter Ujfalusi wrote:
> >>>>> With the new dma_request_chan() the client driver does not need to
> >>>>> look for the DMA resource and it does not need to pass filter_fn
> >>>>> anymore. By switching to the new API the driver can now support
> >>>>> deferred probing against DMA.
> >>>
> >>> I believe this breaks the OMAP3 ISP driver.
> >>> dma_request_slave_channel_compat() is a superset of dma_request_chan()
> >>> that will, when called with
> >>> omap_dma_filter_fn, return as a fallback any free channel handled by
> >>> the OMAP DMA engine driver. This feature is actively used by this
> >>> driver and must be preserved.
> >
> > The fallback to use the filter_fn is used only when booted in legacy
> > mode or when requesting a channel for non slave DMA operation.
> > Based on the code in the driver it is handling slave transfers, so it
> > must have DMA request line coming from somewhere. If that is missing the
> > driver should not be able to work as it will not start the transfer.
> >
> > dma_request_chan() is to be used when you want to have slave channel
> > with DMA request.
> >
> > If legacy mode needs to be supported then adding the hist DMA request
> > number to the omap3xxx_sdma_map in arch/arm/mach-omap2/dma.c should be
> > done. The reason the omap3isp is not in the table is that I could not
> > find any place where the DMA resource was set (nor the DMA is specified
> > in DT).
> >
> > I'm unsure how this driver could work w/o DMA request line over a random
> > (and SW triggered) DMA channel with slave operation. For the slave DMA
> > you need to have DMA request line.
>
> I see now.
> The omap3isp never supposed to get valid DMA request line via
> platform_get_resource_byname(), it should never find the "hist" DMA in
> the DT data. If it does, the driver would stop working as there is no
> DMA request associated with the histogram data reading.
That's correct.
> It is a bit misleading that it used dma_request_slave_channel_compat()
> for getting the channel.
>
> I think what would be correct is:
> dma_cap_mask_t mask;
>
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
> hist->dma_ch = dma_request_chan_by_mask(&mask);
>
> We will get any DMA channel capable of slave configuration, but we will
> configure no DMA request number for the channel.
I believe that should work. It could in theory result in a different behaviour
as it could return a DMA channel not handled by the OMAP SDMA engine, but I
don't think that would be an issue.
> and document this in the driver...
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists