From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F3AFE2820AC for ; Wed, 25 Feb 2026 16:21:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772036500; cv=none; b=OTWtUtO75/f5p83tlxsDdBzA7VobDcSStwrwSZeajv7DVQq/Ss/AtCXwEVIsgs0NWyUKQ/h4yh3yU+G1gcN/UrkWDaJbIaPgKQOlPzmvKGvEFX6BnBl+JkSLc1STGC2gmOSgW3KF7ZFhT3DnJ4N0lIsi94PlEkZgEEDOo2f92v4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772036500; c=relaxed/simple; bh=oFHE6XeL8OuoBiNq2xCcdd6JYQPIHcEa0B7rzSkZJ3A=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=g3jif7eAC5YaZtVK9nerHuuFWlql0LqvLEhLG3u56SWzH7jpEozuV70XGxrJSGthYxwodW/gUtps8t7MlvXl35Z2XAS193/N8VWfgOEviRX/2W80K9XIlSMmdkUzmRz9AynLkBhkAPtw/RnW6SPpwPbvlnxNcYNEvEmflxfsWRk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ursulin.net; spf=pass smtp.mailfrom=ursulin.net; dkim=pass (2048-bit key) header.d=ursulin.net header.i=@ursulin.net header.b=yYcio7m+; arc=none smtp.client-ip=209.85.128.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ursulin.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ursulin.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ursulin.net header.i=@ursulin.net header.b="yYcio7m+" Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-4833115090dso67289345e9.3 for ; Wed, 25 Feb 2026 08:21:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ursulin.net; s=google; t=1772036496; x=1772641296; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=OO3CJh02/qdvHrL++jNNJeGHaSQf/MMi0XFi9TCjoGc=; b=yYcio7m+BJuBYgD2np+V0zzhK+ec44Fx7pYt5CweOGERnVhUfJ4cYgnj44MWUeUvVl c04HMONg4VDniolVXuTBjJowfv36X8FybyHbWt08S8V4K+DYYI4/UqJIY79ub2FxixIV JIvEOSmgtIOTa3jR1Qnh83dfFANyNhucvY1xSgXTPpYghiv86I5mChPNwJx0uD4l7REk mvrZwLAXzK/wqSZMeRQqYj4eS4/YjyZV5ekbl7WerSwJntG4/ZQujUeJ31hy/0qWvJTW gs54bTPiNfvazhSTWheEg1hu+b9Bcjv3j7AIe5FZ9Iqsk6vtFwrFIvIQCFfWjBf4iSxu VOYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772036496; x=1772641296; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OO3CJh02/qdvHrL++jNNJeGHaSQf/MMi0XFi9TCjoGc=; b=B3fKpJ6fS1m11beDKo1KPHbzBcQhAOt+q3R/Za8SvraoM9Tx8RpmaSyu+nPeOfhssl rnKdw8X8qVCixPFNNkx1qsmAvV6Gi2GLvc2wiiSAdhVCwWNzjSm+96gvArQBpCf3xTEe f8YbtfeqBnzzgWzerlbW1RCK9lbdmgH+rWGRwKhIGvk7BsOYXOQLD1liUdqnIVgrjjAL RMNB7k4Oi83ZC1Dw5wNuEMj/SM6rP0+zL4lUj2LuWupP0fhLUSCejOBFspEShLyOhDfB zN3hB8IZdGzT0qMALx+0kbw2VwuT5LZbaPfMV7B/oYEjbcwkRLa8+NzJXwAjEZmi0aQO cWcw== X-Forwarded-Encrypted: i=1; AJvYcCVgZqSkYo7z6TdO7Mk9j18a4Lev6bK1wX2kFXUxS07BybDyy266eWY7sOooqxNYA2MOPKHnH1Y2pX349r4=@vger.kernel.org X-Gm-Message-State: AOJu0YyDxuFyj8EN7FJq90YyLT77IJisQ5cTk4zuqU265G8YKFK3zcf2 m5SJyYQTCzl+UV+PQu5OxSlsUJ5lBRShLhSJ8kogd5IRByyqOZes0VZlWMjxl5Em3rQ= X-Gm-Gg: ATEYQzz3StwIld1Hq/zt7+FGcY/RfDAx4J1Gx0sKuvRzdNmWkCbA2udBxicq7t2reGn XCmFyUQqkMhlDmAkMaqB7rLkCuFmF5dVCUOHdKEP7D4W2Yos0OMzTUfaoVf77ZmvLmMRfDpJ/ir cdWPWe5GcNotCpQjLTHWYr+HuSJQql+WldCWkvrr31wX35hi1QyS0/bCdSozp2go5kuijQh2x4/ yktnYbY6gCOi6DJro90PmeKHbPO0MHP1p6MR8Kcz7YC86m0pE0sbnRfZNLg5aUeoolWvghzFBNa WMgD1wt8x90VtMlED2h1ZNzOvPMQJ0NYgay2aPfWjD38rZhiZYt+M7kC9tvAnLq+dnOkrA0/xek xV92MuTWn6Yehko2wYCVnRIfYZRb0exLTp1kvZCI2o8jjKYQZN5th16OIbFJAzCc7OjF1w4QWJ1 XpUP4YB530cOouGKX2z1229Yw37Kex6UCYs1AOIgw838dD X-Received: by 2002:a05:600d:8450:20b0:483:b505:9db4 with SMTP id 5b1f17b1804b1-483b5059e2dmr134758965e9.31.1772036495912; Wed, 25 Feb 2026 08:21:35 -0800 (PST) Received: from [192.168.0.101] ([90.240.106.137]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-483bfabb84esm54103155e9.0.2026.02.25.08.21.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Feb 2026 08:21:34 -0800 (PST) Message-ID: Date: Wed, 25 Feb 2026 16:21:34 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v3 1/3] drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors To: Yicong Hui , christian.koenig@amd.com, michel.daenzer@mailbox.org Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, david.hunter.linux@gmail.com References: <20260225124609.968505-1-yiconghui@gmail.com> <20260225124609.968505-2-yiconghui@gmail.com> Content-Language: en-GB From: Tvrtko Ursulin In-Reply-To: <20260225124609.968505-2-yiconghui@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 25/02/2026 12:46, Yicong Hui wrote: > Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to make the > DRM_IOCTL_SYNCOBJ_QUERY ioctl fill out the handles array with the > error code of the first fence found per syncobj and 0 if one is not > found and maintain the normal return value in points. > > Suggested-by: Christian König > Suggested-by: Michel Dänzer > Signed-off-by: Yicong Hui > --- > Changes in v3: > * Fixed inline comments by converting to multi-line comments in > accordance to kernel style guidelines. > * No longer using a separate superfluous function to walk the fence > chain, and instead queries the last signaled fence in in the chain for > its error code > * Fixed types for error and handles array. > > > drivers/gpu/drm/drm_syncobj.c | 22 ++++++++++++++++++++-- > include/uapi/drm/drm.h | 5 +++++ > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index 2d4ab745fdad..b74e491f9d8b 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -1654,14 +1654,17 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > { > struct drm_syncobj_timeline_array *args = data; > struct drm_syncobj **syncobjs; > + unsigned int valid_flags = DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED | > + DRM_SYNCOBJ_QUERY_FLAGS_ERROR; > uint64_t __user *points = u64_to_user_ptr(args->points); > + uint32_t __user *handles = u64_to_user_ptr(args->handles); > uint32_t i; > - int ret; > + int ret, error; > > if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) > return -EOPNOTSUPP; > > - if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) > + if (args->flags & ~valid_flags) > return -EINVAL; > > if (args->count_handles == 0) > @@ -1681,6 +1684,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > > fence = drm_syncobj_fence_get(syncobjs[i]); > chain = to_dma_fence_chain(fence); > + Random whitespace changes should be avoided. > if (chain) { > struct dma_fence *iter, *last_signaled = > dma_fence_get(fence); > @@ -1688,6 +1692,8 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > if (args->flags & > DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) { > point = fence->seqno; > + error = dma_fence_get_status(fence); > + Ditto. > } else { > dma_fence_chain_for_each(iter, fence) { > if (iter->context != fence->context) { > @@ -1702,16 +1708,28 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > point = dma_fence_is_signaled(last_signaled) ? > last_signaled->seqno : > to_dma_fence_chain(last_signaled)->prev_seqno; > + > + error = dma_fence_get_status(last_signaled); > } > dma_fence_put(last_signaled); > } else { > point = 0; > + error = fence ? dma_fence_get_status(fence) : 0; > } > dma_fence_put(fence); > + More of the same. Although in this case I think it is an improvement so you may keep it. > ret = copy_to_user(&points[i], &point, sizeof(uint64_t)); > ret = ret ? -EFAULT : 0; > if (ret) > break; > + > + if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) { > + ret = copy_to_user(&handles[i], &error, sizeof(*handles)); > + This blank line is not inserted between the existing code but still please remove it - it is not separating any logical blocks so it is not improving readability. Apart from nitpicks, the implementation looks correct to me. But userspace folks need to bless it and use it, as other people have already commented. And uapi is fine since fence status is already UABI courtesy of sync_file. So it is not promoting anything kernel internal to UABI. > + ret = ret ? -EFAULT : 0; > + if (ret) > + break; > + } > } > drm_syncobj_array_free(syncobjs, args->count_handles); > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 27cc159c1d27..213b4dc9b612 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -1044,6 +1044,11 @@ struct drm_syncobj_array { > }; > > #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */ > +/* > + * Copy the status of the fence as output into the handles array. > + * The handles array is overwritten by that. The documentation could be improved though. Make it clear that one status per handle is returned (use more plural) and we need an explanation of what is the status, or a link to something existing. For example sync_file uapi header documents it like this: * @status: status of the fence 0:active 1:signaled <0:error See if you can come up with something clear and to the point for this comment block? Regards, Tvrtko > + */ > +#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1) > struct drm_syncobj_timeline_array { > __u64 handles; > __u64 points;