Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7b17a825-0f76-3e0f-5e56-0e689b7d7866@amd.com>
Date:   Mon, 17 Apr 2023 07:55:43 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v3] drm/nouveau: fix incorrect conversion to
 dma_resv_wait_timeout()
Content-Language: en-US
To:     John Ogness <john.ogness@linutronix.de>
Cc:     linux-kernel@vger.kernel.org, Ben Skeggs <bskeggs@redhat.com>,
        Karol Herbst <kherbst@redhat.com>,
        Lyude Paul <lyude@redhat.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
        Tanmay Bhushan <007047221b@gmail.com>
References: <20230415012137.615305-1-john.ogness@linutronix.de>
 <87edolaomt.fsf@jogness.linutronix.de>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <87edolaomt.fsf@jogness.linutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZytPalpoQWEyOC9BVWVTRUVMR2QyWU5nM094b285VzEvUUNUaHZVUWFxNUc1?=
 =?utf-8?B?RnBoUFNCamZ5SlRybVNrZWxnSWFiL0VXWUNYWjRZNlR6T3UxeEcxOUxvU2hP?=
 =?utf-8?B?S25pbXlpcXJjaUpMOEFwV2E4UlQrVWRpeWJyWTc1aDhUYWlLQkhWbDlQdXd6?=
 =?utf-8?B?QlVuemFXUW1NenAzUEZnU3ZUalpWNW82WVZKVmhzT25McStUaURhaEgvZzF0?=
 =?utf-8?B?YzROVUhrK1ZKZks5TFVhWjJGU3p1b29hNDFmb1ZwbHdSajBKa2ZCc3l1Wnpk?=
 =?utf-8?B?NGJPbkxrbXRjQ2tGMjJrSVcyaXpVMDRmeHBudW5HOVFHUjFrTFdKSlJ3UjE4?=
 =?utf-8?B?Q1RTTGpTaUNzV095dmtjeXRlQ0lqc0tHSUozMXFlaHZBbWdVM0tIcTVuUTBF?=
 =?utf-8?B?em1Cd1JXK0dscUhJTE4yNnlqdHZVV2plT3B2VUZnbDVhbHplYlUrR0hON0Rv?=
 =?utf-8?B?ajcwNi9oRVMwMTE1K2RtcE1QK0hDK3FjdXBtRFhFWHFOS2Jsd1hpUHF4WnN6?=
 =?utf-8?B?WTJudnlHaVY3WUMrbXNiV3lYdG5Jd3Bad0RoKzlaSUx3b2RQdVBUSWIrcnpv?=
 =?utf-8?B?UlpqSTJXYldqYXhjaHdMQzVGV0pKaVhWVjhvU0REVFp0elc5RXFwNi9vRVZD?=
 =?utf-8?B?V1ZLMHZxekdqOWJjL0FpY3crYkRRdDI5U3BCZDl6cUdMMS84dU9pMktGMmZE?=
 =?utf-8?B?aDZiL0lLSUFYZTloYmh3VkY5eG5heENDWHR5RWJhQnUxcStHeCsxQzR6bnpR?=
 =?utf-8?B?ZVA4WXI3eEZFYi9HeUh6aU1DdXJ3WnN6MGRHbXlPRkxTcFR3SCtsRnB0aHVG?=
 =?utf-8?B?OGl1OWVXeFpJMXhjRTl4SDRwTFFqcnBEb29LR0YrZWprWW5WbWM0ZGZNRjB2?=
 =?utf-8?B?RHArUGs0S1MwWVJYdGYyWGZUQm13NFhiMzhGRzFxRkQ2VlFvTEpKNk5CemIv?=
 =?utf-8?B?eWJqSFhpT0RGVDFKSHRUdDRCamxicHNoN0N4QXpsRUdZRnhIYVBENU00QWd6?=
 =?utf-8?B?VWp3MThvek5RcjlVZVhHbGwydEtrNmZHUEJHckZxV1NteEdKaU9rTlRiQ3l3?=
 =?utf-8?B?ZFpyVktvMHV3djQ4YzlNcVNrdUJ6UGQzNGdNNnpYYkx4d0tlZkM3SUtqRGR3?=
 =?utf-8?B?OTFza2JORTg2cmVYZ0g1UG1ncUFpTjBUNGZzNXFTMTFrT3ltdmFyZ0xia1Zi?=
 =?utf-8?B?RTZBK1RFaWR5Y0lKQU5ITldzcllkU2FpUmlJQ3FWTW9oWlcxNGN0eEtUcXpo?=
 =?utf-8?B?dkRNbXgrT1R0TTZuNkhrRitsRHdTMW9Fb2dvSGtlYmNkdGh2em5XOStyTFRa?=
 =?utf-8?B?QkZMczVaUEtGckg4QTE5RHdSM2JwT1Zoa1VZQmVDTnRWQk1obGM5OXBqM1hV?=
 =?utf-8?B?d0t6N2FFdU9KbkQvRVQyMmRtZ0Jod3RaL1NyUkdsUStVVmZ0SzlqaGVxV1Mr?=
 =?utf-8?B?REkxWDlmcWYrRTc5YmJZT1VSQWc4c2NYNDh1RWtoUW9QZmdOSnJDNHhDclAw?=
 =?utf-8?B?TFIrMmJqNlViUVFvODExdWdLMlRBUEpIcXArY0NBcTRRalJ2VktWN29uYW8y?=
 =?utf-8?B?R0lSUU9DMVVOaU8vREw5cmpqZXFEUE1QNW5KUnJ6SnhGZnBxcUkreUpzR3h1?=
 =?utf-8?B?ZUtOeW9UUWsvYURkZTUrVG1WVVpPNkxoUldnQXF6SHg5eXU3enJRVENpZkpu?=
 =?utf-8?B?dUN3NjUvRlBib2l3ME94ZWVzUXF1U0UyUi9NQWd1UjRhbUM4ZlFpL1ZRd0Zi?=
 =?utf-8?B?eFZOQ25rY295Y0Q4VHhCNEhHdXl4bWI2YlVXek9Uem9uRXJFM0I0V3Uvbk94?=
 =?utf-8?B?NXBjY28vQ2l2djBiV2VzZGx6MlNQenk5RkFEY01uZ0NCQnBMUDdwbFdsbTFk?=
 =?utf-8?B?czdXTjh1eHhmUUFQdUJPczgzZGk1cHNKQ0k0b3Q5cXVJeVU5akt1Y1VRK2lB?=
 =?utf-8?B?WFJYL0lYUHNWOHZXaXNQN1JpOVlDY0RxQjJTMWlsNDF0SUl0eG1DZERXMTFL?=
 =?utf-8?B?aUxVSDlUbWE5a1hReGN5NUN4Z2Vvb1FrMnlMQ3ZPa0pPeVhZbDJtcnBIcnNj?=
 =?utf-8?B?bVJtak82eFdPQURodk1XVHFoV0xsdWlZbk1FTG12R2pXcERWZUhFS0J1Rld5?=
 =?utf-8?Q?cWZ4=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 950c412b-6656-4534-20f2-08db3f0862df
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Apr 2023 05:55:45.8413
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9emN4dt7kgfQKc9RBxk6101T5Gx6jCtVQOhmd8Sj1j6nsxxBMsubIfjKovb19u/t
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB7375
X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        SPF_HELO_PASS,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 15.04.23 um 04:02 schrieb John Ogness:
> Commit 41d351f29528 ("drm/nouveau: stop using ttm_bo_wait")
> converted from ttm_bo_wait_ctx() to dma_resv_wait_timeout().
> However, dma_resv_wait_timeout() returns greater than zero on
> success as opposed to ttm_bo_wait_ctx(). As a result, relocs
> will fail and log errors even when it was a success.
>
> Change the return code handling to match that of
> nouveau_gem_ioctl_cpu_prep(), which was already using
> dma_resv_wait_timeout() correctly.
>
> Fixes: 41d351f29528 ("drm/nouveau: stop using ttm_bo_wait")
> Reported-by: Tanmay Bhushan <007047221b@gmail.com>
> Link: https://lore.kernel.org/lkml/20230119225351.71657-1-007047221b@gmail.com
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   I just realized that the nouveau driver style prefers to scope
>   variables used only in loops.
>
>   v3: Define @lret within the for-loop.
>
>   drivers/gpu/drm/nouveau/nouveau_gem.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index f77e44958037..ab9062e50977 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -645,7 +645,7 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
>   				struct drm_nouveau_gem_pushbuf_reloc *reloc,
>   				struct drm_nouveau_gem_pushbuf_bo *bo)
>   {
> -	long ret = 0;
> +	int ret = 0;
>   	unsigned i;
>   
>   	for (i = 0; i < req->nr_relocs; i++) {
> @@ -653,6 +653,7 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
>   		struct drm_nouveau_gem_pushbuf_bo *b;
>   		struct nouveau_bo *nvbo;
>   		uint32_t data;
> +		long lret;
>   
>   		if (unlikely(r->bo_index >= req->nr_buffers)) {
>   			NV_PRINTK(err, cli, "reloc bo index invalid\n");
> @@ -703,13 +704,18 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
>   				data |= r->vor;
>   		}
>   
> -		ret = dma_resv_wait_timeout(nvbo->bo.base.resv,
> -					    DMA_RESV_USAGE_BOOKKEEP,
> -					    false, 15 * HZ);
> -		if (ret == 0)
> +		lret = dma_resv_wait_timeout(nvbo->bo.base.resv,
> +					     DMA_RESV_USAGE_BOOKKEEP,
> +					     false, 15 * HZ);
> +		if (!lret)
>   			ret = -EBUSY;
> +		else if (lret > 0)
> +			ret = 0;
> +		else
> +			ret = lret;
> +
>   		if (ret) {
> -			NV_PRINTK(err, cli, "reloc wait_idle failed: %ld\n",
> +			NV_PRINTK(err, cli, "reloc wait_idle failed: %d\n",
>   				  ret);
>   			break;
>   		}
>
> base-commit: 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d