Hi Shuah,
On 2017-02-22 19:07, Shuah Khan wrote:
On Mon, Feb 20, 2017 at 6:38 AM, Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
To complete DMA memory configuration for MFC device, allocation of the
firmware buffer is needed, because some parameters are dependant on its base
address. Till now, this has been handled in the s5p_mfc_alloc_firmware()
function. This patch moves that logic to s5p_mfc_configure_dma_memory() to
keep DMA memory related operations in a single place. This way
s5p_mfc_alloc_firmware() is simplified and does what it name says. The
other consequence of this change is moving s5p_mfc_alloc_firmware() call
from the s5p_mfc_probe() function to the s5p_mfc_configure_dma_memory().
Overall looks good. This patch makes subtle change in the dma and firwmare
initialization sequence. Might be okay, but wanted to call out just in case,
Before this change:
vb2_dma_contig_set_max_seg_size() is done for both iommu and non-iommu
case before s5p_mfc_alloc_firmware(). With this change setting
dma_contig max size happens after s5p_mfc_alloc_firmware(). From what
I can tell this might not be an issue.
vb2_dma_contig_clear_max_seg_size() still happens after
s5p_mfc_release_firmware(), so that part hasn't changed.
Do any of the dma_* calls made from s5p_mfc_alloc_firmware() and later
during the dma congiguration sequence depend on dmap_parms being
allocated? Doesn't looks like it from what I can tell, but safe to
ask.
Firmware allocation doesn't depend on dma max segment size at all. The only
calls which might depend on it are dma_map_sg(), which are performed much
later as a part of video buffer allocation/preparation in videobuf2, when
dma-buf or user pointer v4l2 modes are selected.
> [...]
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
