Messages in this thread |  | | Subject | Re: [v4,1/3] drm/prime: use dma length macro when mapping sg | From | Marek Szyprowski <> | Date | Fri, 27 Mar 2020 10:10:52 +0100 |
| |
Hi Christian,
On 2020-03-27 09:11, Christian König wrote: > Am 27.03.20 um 08:54 schrieb Marek Szyprowski: >> On 2020-03-25 10:07, Shane Francis wrote: >>> As dma_map_sg can reorganize scatter-gather lists in a >>> way that can cause some later segments to be empty we should >>> always use the sg_dma_len macro to fetch the actual length. >>> >>> This could now be 0 and not need to be mapped to a page or >>> address array >>> >>> Signed-off-by: Shane Francis <bigbeeshane@gmail.com> >>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> >> This patch landed in linux-next 20200326 and it causes a kernel panic on >> various Exynos SoC based boards. >>> --- >>> drivers/gpu/drm/drm_prime.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >>> index 86d9b0e45c8c..1de2cde2277c 100644 >>> --- a/drivers/gpu/drm/drm_prime.c >>> +++ b/drivers/gpu/drm/drm_prime.c >>> @@ -967,7 +967,7 @@ int drm_prime_sg_to_page_addr_arrays(struct >>> sg_table *sgt, struct page **pages, >>> index = 0; >>> for_each_sg(sgt->sgl, sg, sgt->nents, count) { >>> - len = sg->length; >>> + len = sg_dma_len(sg); >>> page = sg_page(sg); >>> addr = sg_dma_address(sg); >> Sorry, but this code is wrong :( > > Well it is at least better than before because it makes most drivers > work correctly again.
Well, I'm not sure that a half-broken fix should be considered as a fix ;)
Anyway, I just got the comment from Shane, that my patch is fixing the issues with amdgpu and radeon, while still working fine for exynos, so it is indeed a proper fix.
> See we only fill the pages array because some drivers (like Exynos) > are still buggy and require this.
Exynos driver use this pages array internally.
> > Accessing the pages of an DMA-buf imported sg_table is illegal and > should be fixed in the drivers.
True, but in meantime we should avoid breaking stuff which worked fine for ages.
> >> [SNIP] >> >> I will send a patch in a few minutes with the above fixed code. > > That is certainly a good idea for now, but could we also put on > somebodies todo list an item to fix Exynos?
Yes, I can take a look into removing the use of the internal pages array. It is used mainly for implementing vmap for fbdev emulation, but this can be handled in a different way.
Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
|  |