From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E59E62F363E; Tue, 30 Sep 2025 10:33:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759228399; cv=none; b=bwsSUE/IEYVZf8/zY4QY6OF0AUdL9xgx8ITyj0KlyQ2JJ0K/gTg8k2FPRSm1T2d3Fykx0TCKpIXK8EfWK9vSO8kBuaeznPL+78qBBiDP2ck+nVXZRYzxFU7Ff5bckgx1R/vCjapWiP29coNukdbc8/YwWtJPbGYGqq7eodBJJtI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759228399; c=relaxed/simple; bh=9kytHiFrpM4nP2m+PtHjF+LmiM13fFvF27ssloeTfnA=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=MiA+Gw0gCtm/DxzPCyqlxOvv3MzmUqo/0ol2jxgt6FACEuKtoqMZxpZOlbh2u3/j+HEfQBOI8M9Qzz/mVSaqjkGlrW1fMNwgmt4Cr4LvmpyBoNBSnY1D1EANZqaCL0tqPJOJo8eUsl+e0/aBaAf+FDUUtSO4qki2ghmfOr3tZNU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HGvpomiq; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HGvpomiq" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 28729C4CEF0; Tue, 30 Sep 2025 10:33:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1759228398; bh=9kytHiFrpM4nP2m+PtHjF+LmiM13fFvF27ssloeTfnA=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=HGvpomiqFwr6mmm3CTLEpr3OrLb899zID3z1pPal1usJfbgNesB2/WJAqEDIDlJsW 160yrqK94HbRLXvGVzgHfzRboKzvTF99D0kfk1j+JVtvby9T2qE+MHjeXYMISdiIHd thgGQrhJo0MW2qdOJgW9PiCBI/dFFoALC8U8DwLWh8HZPqCWxNTbr+vRKoZyvYUN+Q qSU6V1sRE3niWCkNIkpkndDhb/lkDMIZBmxV2nH/9zc6H5TK260ZafJLPt3UrksB6W VK4IkLfCbtP79/qz8dyziPWsbQAlqU5YoBiPdDCrH0dV/ZYq6+6ivJGGavEwNuzGj7 F125oQ8CI2xaw== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 30 Sep 2025 12:33:12 +0200 Message-Id: Subject: Re: [PATCH v2 05/10] gpu: nova-core: gsp: Add GSP command queue handling Cc: "Alexandre Courbot" , , , "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Benno Lossin" , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "David Airlie" , "Simona Vetter" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "John Hubbard" , "Joel Fernandes" , "Timur Tabi" , , To: "Alistair Popple" From: "Danilo Krummrich" References: <20250922113026.3083103-1-apopple@nvidia.com> <20250922113026.3083103-6-apopple@nvidia.com> <7brvf66smc3ltqrw7tm7smth6wnefeqykdm2n4qahi5xnr6d5o@4l5xfdbznlp3> In-Reply-To: On Tue Sep 30, 2025 at 2:36 AM CEST, Alistair Popple wrote: > I feel like this is largely where the confusion/disagreement lies - what = the > `gsp` module provides versus what the `fw` module provides. My thinking i= s that > the types don't leak outside the `gsp` module, the `fw` is really just pr= oviding > raw binding information that the `gsp` module turns into something nice/a= bstract > for the rest of nova-core in the form of functions (or types) to do whate= ver > is needed. The goal is that firmware specific structures (and the majority of their implementation details) don't leak into the business logic of the driver co= de. Of course, due to its nature the `gsp` module can't be entirely independent= from firmware specifics. However, think of it in different layers: If we encapsulate the layout of firmware specific structures in new types (= i.e. wrap firmware structures into new types in the `fw` module), all changes to= the layout of a structure, new features, etc. are local to and can be handled transparently by this abstraction layer. Once we actually introduce a second firmware version, we will also introduc= e the corresponding proc macros to annotate those structures accordingly with `#[versions(GSP)]` and let the proc macro generate the version specific structures, which subsequently allows us to treat the new types in the `fw` module as if they'd be existing only once (and not per firmware version). Hence, all the other code in the GSP module can be left untouched unless a = new firmware version introduces semantical changes (which should be rare). This is not only a major advantage when it comes to maintainability (which = is one of the major motivations for the Nova project to begin with), but also = leads to a much cleaner structure and naturally separates the layout of data from= its semantics. Yes, it does require a bit more code to set it up to begin with, but it avo= ids a horrible mess leaking trivial firmware changes into the semantic layers of = the GSP implementation. But even if that would never happen and the firmware interface would never change (which I think will not and should not happen), the separation of ho= w to lay out data and the implementation of the semantics by itself adds clarity= and helps with maintainability. The feedback Alex gave in this context looks pretty in line with the above. I hope this helps. - Danilo