From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 768D4FA373D for ; Fri, 28 Oct 2022 01:11:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235081AbiJ1BLs (ORCPT ); Thu, 27 Oct 2022 21:11:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47026 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234492AbiJ1BLr (ORCPT ); Thu, 27 Oct 2022 21:11:47 -0400 Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 75F61A3F4F for ; Thu, 27 Oct 2022 18:11:46 -0700 (PDT) Received: by mail-oi1-x22f.google.com with SMTP id r187so4659240oia.8 for ; Thu, 27 Oct 2022 18:11:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=5EA/rRsqesB5knK6U60b9qGTqZ/vIiOct5OFzn794j8=; b=qjm6RD/RmBJahmvvVeY6o5kMWk/aRac6DF01OEVkSagU9ZipwnIjs8nfU7VZkcm2VY 8+830Hy6A+I4LhTVia6tejtfM50lm/SIRDBJBSS2d09WfaoXuQQwptw2839hi1Oj5C0Y wmlkeuT+o9nWmposOEhB7ronNJt0pnA3b/Qfo6WNR4xb9lqUwYGfD7TxKvWiP1QFZxSz W88DQ14iQwoTNwiOuaTSYE7XwQNdiogY0ySbppLTd2CNAssVlJQF7/2LXD0S69e/JHUw zVAVjEgPAAjiVLOOheAzz0hW4SQ+yQYAHFAfiYJ7tTh+QMLxSQ3Xf3+KfFKtpa17GxiT A8Sg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5EA/rRsqesB5knK6U60b9qGTqZ/vIiOct5OFzn794j8=; b=LENXlzQ2gkrt/VGwHF30DGYkjTFQg9z8HZuzisfHU/JZwx8JNzW6ZxSRZ0X5VvIJSG UPNwKLYEzb9MfwWyRO5QaRvwArHp9VpWcwZOlKQ+bIN1dEtGAJ8EHPioE3MPK8W/Xvga jjto/kK8vsd1zNrYQmSDyvDLSuY5dVoFXcUyxftUw0yrRTb8VFZsH6Et0aFc1zBy8/fi ktqMbM2gNIz2aRqQzxVfib16suchnlxE+TZSsX0rtfKONrRoo55i8EumnyzUUg9RPcQD ONY6rtBwudsJO4qHH7J8b6cnX12UExLA/k7yMYTdmEeD16gcaFWR9jZBDDMFDc2WVvi4 iO3g== X-Gm-Message-State: ACrzQf0hnHn7jBI6vxhpZeECFziqMq9ZrH4Npot+yjEPmLKjsQLIVg8n YJGcoWTcI2/DZHrCfZS+BKAgEJBcTAfOCjjYOyM= X-Google-Smtp-Source: AMsMyM69+M9JkB7n7pEjm9pG4X2UrOlWxyLLPQP8olANKeo9vGbNd7nNGot4Jggqx2IeD4VJZouh8j2E5wD/aymEsNA= X-Received: by 2002:a05:6808:8f4:b0:354:946e:8dc5 with SMTP id d20-20020a05680808f400b00354946e8dc5mr6340635oic.183.1666919505708; Thu, 27 Oct 2022 18:11:45 -0700 (PDT) MIME-Version: 1.0 References: <20221020121316.3946-1-christian.koenig@amd.com> <20221020121316.3946-3-christian.koenig@amd.com> In-Reply-To: From: Rob Clark Date: Thu, 27 Oct 2022 18:11:56 -0700 Message-ID: Subject: Re: [PATCH 2/3] drm/prime: set the dma_coherent flag for export To: =?UTF-8?Q?Christian_K=C3=B6nig?= Cc: =?UTF-8?Q?Christian_K=C3=B6nig?= , l.stach@pengutronix.de, nicolas@ndufresne.ca, ppaalanen@gmail.com, sumit.semwal@linaro.org, daniel@ffwll.ch, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Thu, Oct 20, 2022 at 7:57 AM Christian K=C3=B6nig wrote: > > Am 20.10.22 um 16:43 schrieb Rob Clark: > > On Thu, Oct 20, 2022 at 5:13 AM Christian K=C3=B6nig > > wrote: > >> When a device driver is snooping the CPU cache during access we assume > >> that all importers need to be able to snoop the CPU cache as well. > >> > >> Signed-off-by: Christian K=C3=B6nig > >> --- > >> drivers/gpu/drm/drm_prime.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > >> index 20e109a802ae..d5c70b6fe8a4 100644 > >> --- a/drivers/gpu/drm/drm_prime.c > >> +++ b/drivers/gpu/drm/drm_prime.c > >> @@ -28,6 +28,7 @@ > >> > >> #include > >> #include > >> +#include > >> #include > >> #include > >> > >> @@ -889,6 +890,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_ge= m_object *obj, > >> .size =3D obj->size, > >> .flags =3D flags, > >> .priv =3D obj, > >> + .coherent =3D dev_is_dma_coherent(dev->dev), > > To set the coherent flag correctly, I think I'd need a way to override > > on a per buffer basis, since coherency is a property of the gpu > > pgtables (which in the msm case is an immutable property of the gem > > object). We also have some awkwardness that drm->dev isn't actually > > the GPU, thanks to the kernels device model seeing a collection of > > other small devices shoehorned into a single drm device to fit > > userspace's view of the world. So relying on drm->dev isn't really > > going to give sensible results. > > Yeah, I've the same problem for amdgpu where some buffers are snooped > while others aren't. > > But this should be unproblematic since the flag can always be cleared by > the driver later on (it just can't be set). > > Additional to that I've just noted that armada, i915, omap and tegra use > their own DMA-buf export function. MSM could do the same as well if the > device itself is marked as not coherent while some buffers are mapped > cache coherent. yeah, I guess that would work.. it would be a bit unfortunate to need to use our own export function, but I guess it is a small price to pay and I like the overall idea, so a-b for the series For the VMM case, it would be nice to expose this to userspace, but I've sent a patch to do this in an msm specific way, and I guess at least solving that problem for one driver and better than the current state of "iff driver =3D=3D "i915" { it's mmap'd cached } else { it's writecombine }" in crosvm Admittedly the VMM case is a rather special case compared to your average userspace dealing with dmabuf's, but it would be nice to get out of the current situation where it is having to make assumptions which are quite possibly wrong, so giving the VMM some information even if it is "the cachability isn't static, you should bail now if your arch can't cope" would be an improvement. (For background, this case is also a bit specific for android/gralloc.. for driver allocated buffers in a VM, with the native usermode driver (UMD) in guest, you still have some control within the UMD) BR, -R > Regards, > Christian. > > > I guess msm could just bury our heads in the sand and continue to do > > things the way we have been (buffers that are mapped cached-coherent > > are only self-shared) but would be nice to catch if userspace tried to > > import one into (for ex) v4l2.. > > > > BR, > > -R > > > >> .resv =3D obj->resv, > >> }; > >> > >> -- > >> 2.25.1 > >> > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 79AEFECAAA1 for ; Fri, 28 Oct 2022 01:11:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 66C1710E04E; Fri, 28 Oct 2022 01:11:50 +0000 (UTC) Received: from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com [IPv6:2607:f8b0:4864:20::22d]) by gabe.freedesktop.org (Postfix) with ESMTPS id AF9C310E04E for ; Fri, 28 Oct 2022 01:11:46 +0000 (UTC) Received: by mail-oi1-x22d.google.com with SMTP id n83so4648827oif.11 for ; Thu, 27 Oct 2022 18:11:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=5EA/rRsqesB5knK6U60b9qGTqZ/vIiOct5OFzn794j8=; b=qjm6RD/RmBJahmvvVeY6o5kMWk/aRac6DF01OEVkSagU9ZipwnIjs8nfU7VZkcm2VY 8+830Hy6A+I4LhTVia6tejtfM50lm/SIRDBJBSS2d09WfaoXuQQwptw2839hi1Oj5C0Y wmlkeuT+o9nWmposOEhB7ronNJt0pnA3b/Qfo6WNR4xb9lqUwYGfD7TxKvWiP1QFZxSz W88DQ14iQwoTNwiOuaTSYE7XwQNdiogY0ySbppLTd2CNAssVlJQF7/2LXD0S69e/JHUw zVAVjEgPAAjiVLOOheAzz0hW4SQ+yQYAHFAfiYJ7tTh+QMLxSQ3Xf3+KfFKtpa17GxiT A8Sg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5EA/rRsqesB5knK6U60b9qGTqZ/vIiOct5OFzn794j8=; b=W2EDuqPEMufLpevIPkzYJNYKiPOuQR1TnMTggmoKX97tiiFDeWByG6SafWnkimf9GS VJt94jke/QK1zMn76wQF3HdTmvaqgJi3mpUztnXCQQB1ESxr9u1LbzKK8U2in3SyPfIH AgJfPBMfQebjNciwgbIWSTs9FKm5C381nHMoeCEFDwROd/Mp7uQj9xPSwfV4LJ/bTNR9 mWeHue4WeWy31Qd5a9gGl8PZQJmrQHT/qcyxLniTvDUK0QFYomdEsoBFoIQ1L3ZZTaNZ XmkaiLm7ybVhSmOoQznHBqQYalc8XPpsi/xIK0YMpXgsAiLce1lWVTXjbVP8TOXfzhtF AFfA== X-Gm-Message-State: ACrzQf2PMriMR8pyQabQU/+2IlA1L47MT1PkvOrKnW8eiP9634r4OUPY lYYDiWnu2s+DQsE+IGgMRVL+RE+FBmnAobJC8DE= X-Google-Smtp-Source: AMsMyM69+M9JkB7n7pEjm9pG4X2UrOlWxyLLPQP8olANKeo9vGbNd7nNGot4Jggqx2IeD4VJZouh8j2E5wD/aymEsNA= X-Received: by 2002:a05:6808:8f4:b0:354:946e:8dc5 with SMTP id d20-20020a05680808f400b00354946e8dc5mr6340635oic.183.1666919505708; Thu, 27 Oct 2022 18:11:45 -0700 (PDT) MIME-Version: 1.0 References: <20221020121316.3946-1-christian.koenig@amd.com> <20221020121316.3946-3-christian.koenig@amd.com> In-Reply-To: From: Rob Clark Date: Thu, 27 Oct 2022 18:11:56 -0700 Message-ID: Subject: Re: [PATCH 2/3] drm/prime: set the dma_coherent flag for export To: =?UTF-8?Q?Christian_K=C3=B6nig?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?UTF-8?Q?Christian_K=C3=B6nig?= , dri-devel@lists.freedesktop.org, nicolas@ndufresne.ca, linaro-mm-sig@lists.linaro.org, ppaalanen@gmail.com, linux-media@vger.kernel.org, sumit.semwal@linaro.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Thu, Oct 20, 2022 at 7:57 AM Christian K=C3=B6nig wrote: > > Am 20.10.22 um 16:43 schrieb Rob Clark: > > On Thu, Oct 20, 2022 at 5:13 AM Christian K=C3=B6nig > > wrote: > >> When a device driver is snooping the CPU cache during access we assume > >> that all importers need to be able to snoop the CPU cache as well. > >> > >> Signed-off-by: Christian K=C3=B6nig > >> --- > >> drivers/gpu/drm/drm_prime.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > >> index 20e109a802ae..d5c70b6fe8a4 100644 > >> --- a/drivers/gpu/drm/drm_prime.c > >> +++ b/drivers/gpu/drm/drm_prime.c > >> @@ -28,6 +28,7 @@ > >> > >> #include > >> #include > >> +#include > >> #include > >> #include > >> > >> @@ -889,6 +890,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_ge= m_object *obj, > >> .size =3D obj->size, > >> .flags =3D flags, > >> .priv =3D obj, > >> + .coherent =3D dev_is_dma_coherent(dev->dev), > > To set the coherent flag correctly, I think I'd need a way to override > > on a per buffer basis, since coherency is a property of the gpu > > pgtables (which in the msm case is an immutable property of the gem > > object). We also have some awkwardness that drm->dev isn't actually > > the GPU, thanks to the kernels device model seeing a collection of > > other small devices shoehorned into a single drm device to fit > > userspace's view of the world. So relying on drm->dev isn't really > > going to give sensible results. > > Yeah, I've the same problem for amdgpu where some buffers are snooped > while others aren't. > > But this should be unproblematic since the flag can always be cleared by > the driver later on (it just can't be set). > > Additional to that I've just noted that armada, i915, omap and tegra use > their own DMA-buf export function. MSM could do the same as well if the > device itself is marked as not coherent while some buffers are mapped > cache coherent. yeah, I guess that would work.. it would be a bit unfortunate to need to use our own export function, but I guess it is a small price to pay and I like the overall idea, so a-b for the series For the VMM case, it would be nice to expose this to userspace, but I've sent a patch to do this in an msm specific way, and I guess at least solving that problem for one driver and better than the current state of "iff driver =3D=3D "i915" { it's mmap'd cached } else { it's writecombine }" in crosvm Admittedly the VMM case is a rather special case compared to your average userspace dealing with dmabuf's, but it would be nice to get out of the current situation where it is having to make assumptions which are quite possibly wrong, so giving the VMM some information even if it is "the cachability isn't static, you should bail now if your arch can't cope" would be an improvement. (For background, this case is also a bit specific for android/gralloc.. for driver allocated buffers in a VM, with the native usermode driver (UMD) in guest, you still have some control within the UMD) BR, -R > Regards, > Christian. > > > I guess msm could just bury our heads in the sand and continue to do > > things the way we have been (buffers that are mapped cached-coherent > > are only self-shared) but would be nice to catch if userspace tried to > > import one into (for ex) v4l2.. > > > > BR, > > -R > > > >> .resv =3D obj->resv, > >> }; > >> > >> -- > >> 2.25.1 > >> >