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 X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1DA8C433DF for ; Wed, 12 Aug 2020 17:03:48 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id ADDC220855 for ; Wed, 12 Aug 2020 17:03:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="ahQlZNd5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ADDC220855 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nvidia.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 263216E081; Wed, 12 Aug 2020 17:03:48 +0000 (UTC) Received: from hqnvemgate26.nvidia.com (hqnvemgate26.nvidia.com [216.228.121.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 15A506E081 for ; Wed, 12 Aug 2020 17:03:47 +0000 (UTC) Received: from hqpgpgate102.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Wed, 12 Aug 2020 10:03:33 -0700 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate102.nvidia.com (PGP Universal service); Wed, 12 Aug 2020 10:03:46 -0700 X-PGP-Universal: processed; by hqpgpgate102.nvidia.com on Wed, 12 Aug 2020 10:03:46 -0700 Received: from [172.20.40.57] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 12 Aug 2020 17:03:46 +0000 Subject: Re: [git pull] drm for 5.8-rc1 To: Ilia Mirkin , Karol Herbst References: <20200701075719.p7h5zypdtlhqxtgv@box> <20200701075902.hhmaskxtjsm4bcx7@box> <77e744b9-b5e2-9e9b-44c1-98584d2ae2f3@nvidia.com> <5ffa32db-4383-80f6-c0cf-a9bb12e729aa@nvidia.com> <261cd7c9-6853-3d5f-3a3e-86b65c9dba71@nvidia.com> From: James Jones X-Nvconfidentiality: public Message-ID: <0e882aa7-d0ea-19b0-a13d-4f7bc0d384aa@nvidia.com> Date: Wed, 12 Aug 2020 10:03:46 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To HQMAIL107.nvidia.com (172.20.187.13) Content-Language: en-US DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1597251813; bh=XBuVSFXKkeoQOWkMj9NbKDZcRHdU04vqvBj59iGLKsk=; h=X-PGP-Universal:Subject:To:CC:References:From:X-Nvconfidentiality: Message-ID:Date:User-Agent:MIME-Version:In-Reply-To: X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=ahQlZNd54oAs4t1GfxgaEZKf/qbBeqCVceu+bIuKGPQRThNlTQpP1O5cyND7BsL7f kumIackH/9TAtMHMu7AzTU2zLgI5DzKSPYP1hPYVP9vwfBDQfvXTNHtR6sbDmc79Um x8Cxq/LmM+Qb+OdfcRAUQorO768i+P7hvF9VcJ8yLlj53pxpcb+uFWMnLEgyXna7JC EKCYH5hfg29sF9yEa1ueUMguC/emp3vsCw7OOMUQnAq73fEQBjEduPd9gbpFupRKar dWVCyEnaWqfXeO7eJxhzBVd3pGhNlOj7upWqx1js1zdkVBSiVsTn831YPm9/VVUdrc oHZKktE5Tbo/A== 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: Thierry Reding , dri-devel Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 8/12/20 5:37 AM, Ilia Mirkin wrote: > On Wed, Aug 12, 2020 at 8:24 AM Karol Herbst wrote: >> >> On Wed, Aug 12, 2020 at 12:43 PM Karol Herbst wrote: >>> >>> On Wed, Aug 12, 2020 at 12:27 PM Karol Herbst wrote: >>>> >>>> On Wed, Aug 12, 2020 at 2:19 AM James Jones wrote: >>>>> >>>>> Sorry for the slow reply here as well. I've been in the process of >>>>> rebasing and reworking the userspace patches. I'm not clear my changes >>>>> will address the Jetson Nano issue, but if you'd like to try them, the >>>>> latest userspace changes are available here: >>>>> >>>>> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724 >>>>> >>>>> And the tegra-drm kernel patches are here: >>>>> >>>>> >>>>> https://patchwork.ozlabs.org/project/linux-tegra/patch/20191217005205.2573-1-jajones@nvidia.com/ >>>>> >>>>> Those + the kernel changes addressed in this thread are everything I had >>>>> outstanding. >>>>> >>>> >>>> I don't know if that's caused by your changes or not, but now the >>>> assert I hit is a different one pointing out that >>>> nvc0_miptree_select_best_modifier fails in a certain case and returns >>>> MOD_INVALID... anyway, it seems like with your patches applied it's >>>> now way easier to debug and figure out what's going wrong, so maybe I >>>> can figure it out now :) >>>> >>> >>> collected some information which might help to track it down. >>> >>> src/gallium/frontends/dri/dri2.c:648 is the assert hit: assert(*zsbuf) >>> >>> templ is {reference = {count = 0}, width0 = 300, height0 = 300, depth0 >>> = 1, array_size = 1, format = PIPE_FORMAT_Z24X8_UNORM, target = >>> PIPE_TEXTURE_2D, last_level = 0, nr_samples = 0, nr_storage_samples = >>> 0, usage = 0, bind = 1, flags = 0, next = 0x0, screen = 0x0} >>> >>> inside tegra_screen_resource_create modifier says >>> DRM_FORMAT_MOD_INVALID as template->bind is 1 >>> >>> and nvc0_miptree_select_best_modifier returns DRM_FORMAT_MOD_INVALID, >>> so the call just returns NULL leading to the assert. >>> >>> Btw, this is on Xorg-1.20.8-1.fc32.aarch64 with glxgears. >>> >> >> So I digged a bit deeper and here is what tripps it of: >> >> when the context gets made current, the normal framebuffer validation >> and render buffer allocation is done, but we end up inside >> tegra_screen_resource_create at some point with PIPE_BIND_SCANOUT set >> in template->bind. Now the tegra driver forces the >> DRM_FORMAT_MOD_LINEAR modifier and calls into >> resource_create_with_modifiers. >> >> If it wouldn't do that, nouveau would allocate a tiled buffer, with >> that it's linear and we at some point end up with an assert about a >> depth_stencil buffer being there even though it shouldn't. If I always >> use DRM_FORMAT_MOD_INVALID in tegra_screen_resource_create, things >> just work. >> >> That's kind of the cause I pinpointed the issue down to. But I have no >> idea what's supposed to happen and what the actual bug is. > > Yeah, the bug with tegra has always been "trying to render to linear > color + tiled depth", which the hardware plain doesn't support. (And > linear depth isn't a thing.) > > Question is whether what it's doing necessary. PIPE_BIND_SCANOUT > (/linear) requirements are needed for DRI2 to work (well, maybe not in > theory, but at least in practice the nouveau ddx expects linear > buffers). However tegra operates on a more DRI3-like basis, so with > "client" allocations, tiled should work OK as long as there's > something in tegra to copy it to linear when necessary? I can confirm the above: Our hardware can't render to linear depth buffers, nor can it mix linear color buffers with block linear depth buffers. I think there's a misunderstanding on expected behavior of resource_create_with_modifiers() here too: tegra_screen_resource_create() is passing DRM_FORMAT_MOD_INVALID as the only modifier in non-scanout cases. Previously, I believe nouveau may have treated that as "no modifiers specified. Fall back to internal layout selection logic", but in my patches I "fixed" it to match other drivers' behavior, in that allocation will fail if that is the only modifier in the list, since it is equivalent to passing in a list containing only unsupported modifiers. To get fallback behavior, tegra_screen_resource_create() should pass in (NULL, 0) for (modifiers, count), or just call resource_create() on the underlying screen instead. Beyond that, I can only offer my thoughts based on analysis of the code referenced here so far: While I've learned from the origins of this thread applications/things external to Mesa in general shouldn't be querying format modifiers of buffers created without format modifiers, tegra is a Mesa internal component that already has some intimate knowledge of how the nouveau driver it sits on top of works. Nouveau will always be able to construct and return a valid format modifier for unorm single sampled color buffers (and hopefully, anything that can scan out going forward), both before and after my patches I believe, regardless of how they were allocated. After my patches, it should even work for things that can't scan out in theory. Hence, looking at this without knowledge of what motivated the original changes, it seems like tegra_screen_resource_create should just naively forward the resource_create() call, relying on nouveau to select a layout and provide a valid modifier when queried for import. As Karol notes, this works fine for at least this simple test case, and it's what nouveau itself would be doing with an equivalent callstack, excepting the modifier query, so I find it hard to believe it breaks some application behavior. It'll also end up being equivalent (in end result, not quite semantically) to what dri3_alloc_render_buffer() was doing prior to the patch mentioned that broke things for Karol, so certainly for the DRI3 usage it's the right behavior. Ilia, what in the nouveau DDX (As in Xfree86 DDX?) assumes linear buffers? It sounds like you don't think it will interact poorly with this path regardless? Thierry, do you recall what motivated the force-linear code here? As to why this works for Thierry and not Karol, that's confusing. Are you both using the same X11 DDX (modesetting I assume?) and X server versions? Could it be a difference in client-side DRI library code somehow? Thanks, -James > -ilia > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel