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 7AEBC23AB9D for ; Fri, 13 Feb 2026 22:22:19 +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=1771021339; cv=none; b=i5zdUu34sME6efB2shI/pwSCHvumN8mi/yu2fbeHXP+eE7fUGi4Zy29W85oqqmw9KV3Fj1Tghq5FJ3QgrS368q0tMfwEuDtSlMQG6p6U4RlcyC5u6EFkEaW/Qi8Xff1aV78x/BNVbzZ4AzizoUxzS2zie7Wuki9lW8Kv2xGgXa4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771021339; c=relaxed/simple; bh=JZG9zpO3v26S4FfIoUMCHmNqp0Ksq9OU7wxZoT298/g=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:To:From:Subject: References:In-Reply-To; b=KGM6XWQ/SWWnTnRCEBJ7Jnlw2eIzTnIGxWKQg3wgs7UxmIbgNzR65nPIHDIHKsLcuixqkzgPdGiHZ9IjPKOmd1qNuifT0MaeE8CvReSYgJQ6EbZZBehEFLzN7PhPqOSfn4UA1Qfi/Hdc/aax/PNeemAgQCBjc9kQEaA28o12Uz4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=JWFVI9Ff; 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="JWFVI9Ff" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0B957C116C6; Fri, 13 Feb 2026 22:22:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1771021339; bh=JZG9zpO3v26S4FfIoUMCHmNqp0Ksq9OU7wxZoT298/g=; h=Date:Cc:To:From:Subject:References:In-Reply-To:From; b=JWFVI9FfTpCRFluJKQFdWPC0eQ8LKFMbZkCTuWMLXd8u8taybBVw59hcDTR/NKju8 d2iCaDIrlDcc5iN8tNy8KRlXUkArJpVfICH8XY9ENMkLNTniEHVjP8vYZ333vGD4Xk FaUBJgnq/iGGYAclZspPyB70Smyh8A3uEPnwkG9C+KV5QxDzIQal5/zxRR8yg9rwqB Ew9AYbLgpD6doQp8Hx4lVBl4drJrHNaAUetdPJ5iyjhsQXaOORdJ9DpM6+H5DTnQu+ OBEmddjmnjhSc+U+Mb4c1B0nJsGJfuthVTJ+QfdXu2FhEqC/m1CFZ7/nQuVrg42idq fy1ivTUTXAMgw== Precedence: bulk X-Mailing-List: linux-kernel@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: Fri, 13 Feb 2026 23:22:15 +0100 Message-Id: Cc: "Lyude Paul" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Mary Guillemard" , , , To: "M Henning" From: "Danilo Krummrich" Subject: Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device References: <20260205-zcull3-v2-0-ac572f38cc7b@darkrefraction.com> <20260205-zcull3-v2-1-ac572f38cc7b@darkrefraction.com> In-Reply-To: On Fri Feb 13, 2026 at 10:48 PM CET, M Henning wrote: > On Fri, Feb 13, 2026 at 12:12=E2=80=AFPM Danilo Krummrich wrote: >> >> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote: >> > This information will be exposed to userspace in the following commit. >> > >> > Signed-off-by: Mel Henning >> >> For someone looking at this commit, this commit message is not very usef= ul. >> >> Please add at least a brief explanation of what the patch does and - eve= n more >> important - why it does it. See also [1]. >> >> [1] https://docs.kernel.org/process/submitting-patches.html#describe-you= r-changes > > What I'm struggling with is that I don't know how to do this without > repeating myself. If you want, I can copy-paste my explanation of > zcull here too and then it will appear three times, once in each > commit and once in the cover letter. But that kind of repetition > doesn't seem very helpful to me. Again, the commit message should explain what the commit does and why. For instance, I asked you why you did combine those two callbacks below. The commit message could mention this, e.g. it could be something along the= lines of: "Add struct nvkm_gr_zcull_info, which serves as abstraction layer between t= he corresponding uAPI (added in a subsequent patch) and the firmware (version specific) structure. This is needed in order to not leak the uAPI layer into nvkm. Also note tha= t we are bypassing the nvif layer, since ... Also note that we reuse the get_ctxbufs_info() callback, since ..." I.e. make it obvious to maintainers what's going on and what's the motivati= on for the patch and it's implementation details. > Because of this, I decided that it was simplest to combine them in a > single call, which avoids repeated rpc calls to the gpu without the > complexity of handling partially complete states. Ok, that seems reasonable. >> > + if (WARN_ON(IS_ERR(zcull_info))) >> >> What justifies this WARN_ON()? To me this seems like normal error handli= ng, i.e. >> it is not a violation of some API invariant, etc. Also, this is in the d= river's >> probe() path. > > I was just copying the error handling that already exists in this functio= n. > > I do think these are weird error cases though - they mean that the gpu > was partially but not fully initialized which shouldn't happen during > normal usage. The only cases I can think of that would trigger this > warning are a kernel bug or an intermittent PCI link, which I think > are both reasonable to warn on. It could also be that the firmware is buggy, etc. In any case, I don't see = that a WARN_ON() is justified. Please use dev_err() instead.