On 10/12/20 9:23 pm, Alex Deucher wrote: > On Thu, Dec 10, 2020 at 8:03 AM Shashank Sharma <shashank.sha...@amd.com> > wrote: >> >> On 10/12/20 3:58 pm, Christian König wrote: >>> Am 10.12.20 um 05:49 schrieb Shashank Sharma: >>>> On 09/12/20 11:00 pm, Alex Deucher wrote: >>>>> On Fri, Dec 4, 2020 at 3:41 PM Alex Deucher <alexdeuc...@gmail.com> wrote: >>>>>> And drop it when we detach. If the shared buffer is in vram, >>>>>> we need to make sure we don't put the device into runtime >>>>>> suspend. >>>>>> >>>>>> Signed-off-by: Alex Deucher <alexander.deuc...@amd.com> >>>>> Ping? Any thoughts on this? We really only need this for p2p since >>>>> device memory in involved, but I'm not sure of the best way to handle >>>>> that. >>>>> >>>>> Alex >>>>>> --- >>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 16 ++++++++++++++-- >>>>>> 1 file changed, 14 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c >>>>>> index 5b465ab774d1..f63f182f37f9 100644 >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c >>>>>> @@ -40,6 +40,7 @@ >>>>>> #include <linux/dma-buf.h> >>>>>> #include <linux/dma-fence-array.h> >>>>>> #include <linux/pci-p2pdma.h> >>>>>> +#include <linux/pm_runtime.h> >>>>>> >>>>>> /** >>>>>> * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation >>>>>> @@ -187,9 +188,13 @@ static int amdgpu_dma_buf_attach(struct dma_buf >>>>>> *dmabuf, >>>>>> if (attach->dev->driver == adev->dev->driver) >>>>>> return 0; >>>>>> >>>>>> + r = pm_runtime_get_sync(adev_to_drm(adev)->dev); >>>>>> + if (r < 0) >>>>>> + goto out; >>>>>> + >>>> I am a bit skeptical if we should fail the BO reserve if we don't get the >>>> sync ? I was hoping to continue with it, with a warning maybe, so that it >>>> doesn't block the existing functionality ? >>> I'm not sure why pm_runtime_get_sync() could fail, but not attaching the >>> DMA-buf is certainly the best we could do here in that moment. >> Ah, I see. Just curious about, before this patch, when we tried to reserve >> the BO, without the PM sync, how was that doing OK ? > p2p is not widely used at this point, so we never really hit is except > for specific testing of the functionality. > > Alex
Got it. Apart from this, looks fine by me. Acked-by: Shashank Sharma <shashank.sha...@amd.com> > >> - Shashank >> >>> Otherwise we could end up in accessing the PCIe BAR with power disabled >>> and that's indeed kind of bad. >>> >>> Christian. >>> >>>>>> r = amdgpu_bo_reserve(bo, false); >>>>>> if (unlikely(r != 0)) >>>>>> - return r; >>>>>> + goto out; >>>>>> >>>>>> /* >>>>>> * We only create shared fences for internal use, but importers >>>>>> @@ -201,11 +206,15 @@ static int amdgpu_dma_buf_attach(struct dma_buf >>>>>> *dmabuf, >>>>>> */ >>>>>> r = __dma_resv_make_exclusive(bo->tbo.base.resv); >>>>>> if (r) >>>>>> - return r; >>>>>> + goto out; >>>>>> >>>>>> bo->prime_shared_count++; >>>>>> amdgpu_bo_unreserve(bo); >>>>>> return 0; >>>>>> + >>>>>> +out: >>>>>> + pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); >>>> Why not just pm_runtime_put_sync ? Why autosuspend ? >>>> >>>> - Shashank >>>> >>>>>> + return r; >>>>>> } >>>>>> >>>>>> /** >>>>>> @@ -225,6 +234,9 @@ static void amdgpu_dma_buf_detach(struct dma_buf >>>>>> *dmabuf, >>>>>> >>>>>> if (attach->dev->driver != adev->dev->driver && >>>>>> bo->prime_shared_count) >>>>>> bo->prime_shared_count--; >>>>>> + >>>>>> + pm_runtime_mark_last_busy(adev_to_drm(adev)->dev); >>>>>> + pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); >>>>>> } >>>>>> >>>>>> /** >>>>>> -- >>>>>> 2.25.4 >>>>>> >>>>> _______________________________________________ >>>>> amd-gfx mailing list >>>>> amd-gfx@lists.freedesktop.org >>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cshashank.sharma%40amd.com%7C222e41b3837c4d7f3ec908d89d23ca8b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637432124337679194%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bgSvn4XqmoKffyCF1v1ldVptiKMZXfEfkzFnJj3Ca0Y%3D&reserved=0 >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx@lists.freedesktop.org >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cshashank.sharma%40amd.com%7C222e41b3837c4d7f3ec908d89d23ca8b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637432124337679194%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bgSvn4XqmoKffyCF1v1ldVptiKMZXfEfkzFnJj3Ca0Y%3D&reserved=0 >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cshashank.sharma%40amd.com%7C222e41b3837c4d7f3ec908d89d23ca8b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637432124337679194%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bgSvn4XqmoKffyCF1v1ldVptiKMZXfEfkzFnJj3Ca0Y%3D&reserved=0 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
