On 24/02/16 00:14, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Friday 19 February 2016 11:47:47 Tomi Valkeinen wrote:
>> omap_gem_dma_sync() calls dma_map_page() but does not check the possible
>> error with dma_mapping_error(). If DMA-API debugging is enabled, the
>> debug layer will give a warning if dma_mapping_error() has not been
>> used.
>>
>> This patch adds proper error handling to omap_gem_dma_sync().
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>> ---
>> drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
>> b/drivers/gpu/drm/omapdrm/omap_gem.c index 7098190815f1..a60c30a59f7e
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
>> @@ -775,9 +775,18 @@ void omap_gem_dma_sync(struct drm_gem_object *obj,
>>
>> for (i = 0; i < npages; i++) {
>> if (!omap_obj->addrs[i]) {
>> - omap_obj->addrs[i] = dma_map_page(dev->dev, pages[i], 0,
>> + dma_addr_t addr;
>> +
>> + addr = dma_map_page(dev->dev, pages[i], 0,
>> PAGE_SIZE, DMA_BIDIRECTIONAL);
>> +
>> + if (dma_mapping_error(dev->dev, addr)) {
>> + dev_warn(dev->dev, "omap_gem_dma_sync: dma_map_page
> failed\n");
>
> Same comment as for the previous patch.
>
> No need to unmap ?
I don't know... Maybe, maybe not.
The function doesn't return any error, and the mapped pages are stored
in omap_obj->addrs[]. So, if the failure was temporary, next time we
have already mapped the pages that did succeed. If the failure was not
temporary, well, we don't pass any error anyway, so the pages have to be
unmapped somewhere in any case.
So possibly we could unmap, but I don't see us leaking anything even if
the dma_map_page fails.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
