Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp2884518imu; Sun, 13 Jan 2019 12:24:49 -0800 (PST) X-Google-Smtp-Source: ALg8bN5bGUmDhHeA0h75U415FcJNWBSlbJr0ZeTnd5XryAYT7sJh6dB55b3JBIQvbicGQhZEETVP X-Received: by 2002:aa7:83c6:: with SMTP id j6mr22720157pfn.91.1547411089246; Sun, 13 Jan 2019 12:24:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1547411089; cv=none; d=google.com; s=arc-20160816; b=IqEQV0fRZ7K5PZA6CUF1BlFmTKrM+dIiOkEjas7CVKIqOPAkvOyl1AxFhBUTqt2Jup dGx0d2yBSG/lSvEw7Z4vE61yiwswC4LOWC0JHP6ceyrha5pJOg4Co75XfzWdNMrnqH0H Iv4c1gqEKPTzWhrVB+u+t+GEmxXf03FWhsDwkCj+6FMOTKl+F8GSWsTGMZ0kdZ+8uDlR CewG/Apa7/qyx9gkDQWgQhlk6lq9I+DYIIFrrjhj9QJkdH0hoA68bgCBvHYB2z5nnGhS iQfnr8Af/zlLYKpbB041L0RZpZFsohmtwhQ0OKs23DI8zsuH4aSS8poduk7bnJCfaC8z lweA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=4XD1SHDzYh2nQxz2nn4PAa18fCDeSDyXBRc9vRQvBAQ=; b=lT/wXs10fq1ia+ufxHlnpWSybrgzpFVJ100oO6jSIm9LtiLqaMMGIYDQPApAqsoHfh g8LiqZsjFL8FIPuN1W0krHa9VWyS5y1le1grcQuoPTfOfy4v9J7zOeKjagJAN1RAHvdl ptaZJ8fRqLlX84c6XnqhM9qf0WWrs/zuwAe9+5Buq526nK50+HMUTgxiQqCQ7QEoIqVV eclPfBJ+R7kofW3Bbj2W/DkVzw72w1blp0ww52vKzb8Ue7AFySjPu3RxSOGWeUhYJchS yoxh9vCcBFuRu083zTL9mLvcQXIRHjCrUJF961TAOcFiRGkOo0TcoGxWveTj5DTFplXd Ju0w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b="k/Lk8Q/8"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 64si53352565pfe.74.2019.01.13.12.24.33; Sun, 13 Jan 2019 12:24:49 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b="k/Lk8Q/8"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726557AbfAMUX3 (ORCPT + 99 others); Sun, 13 Jan 2019 15:23:29 -0500 Received: from mail-qt1-f193.google.com ([209.85.160.193]:46403 "EHLO mail-qt1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726428AbfAMUX3 (ORCPT ); Sun, 13 Jan 2019 15:23:29 -0500 Received: by mail-qt1-f193.google.com with SMTP id y20so24455492qtm.13; Sun, 13 Jan 2019 12:23:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=4XD1SHDzYh2nQxz2nn4PAa18fCDeSDyXBRc9vRQvBAQ=; b=k/Lk8Q/8JjsjH3p8Fxc8tX8RzI50eWKPgDJfIZvVUd129ww8akm47XmBxUmkBCfiwx 8Jjl7g2K61fQhE7QEqXLablu98ypEnVmZlqBa+ln36+fAto33Wxot0GC8+Fmw1uQeTjq 2k0FkL+leynTm4zJH9zCUkMqnkjwH81/DoKzERMB3jaCqeUoMjT0jvPeJY0W1xYKhyjm unYhMsyg9x1ZKW9nK7OJ4OKsBGtKDmf05IlOe0YWeLMJKsS0sLe4DAgrrZUEK361O64Q gVYl1op3gVkscal/dL5x9+hflg7Iye4z4t8NG+XRLR7KXJbJBq2SMFj6pSHDVmWClXW5 PniA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=4XD1SHDzYh2nQxz2nn4PAa18fCDeSDyXBRc9vRQvBAQ=; b=TQBSTZmfG7LeaIo08hTZG2d9ATcyqVySTn/qGOXJOp+X9hr2VU/c7We/NcHW3M2294 YrbMkpPEX+NTAmS+irjFHJgAbYV7saJHD3LA0JVn4NikSoj2xQ7piPBcyhPyWZPhjj6c 3lLvXNquOSKkoj46fvEoB6mx1h9IY1V/d/29lV0XVQIz4e8IicofQN4oiL6l+kRAWW6C EHEwGYu+luXz6AXTCC0wL1n405qQ+5s1ud7T29hkGD9y9BrJyh/t5wVFunLMxtVTNdjs llqrhKopAPetQi9vwIXQM1xPsiRYg0iAk3p8h+9C5EEvb39Nlj3+gj51Qc7fDZShjugs 9kJQ== X-Gm-Message-State: AJcUukdiSo/QzPdtvR42A5t/tRq6KZSoEba0Xq4/uc1lkBqz8++y8zkr w5wt04FuI5qi7Cyn7fdgYBw= X-Received: by 2002:a0c:b517:: with SMTP id d23mr21935257qve.221.1547411007618; Sun, 13 Jan 2019 12:23:27 -0800 (PST) Received: from smtp.gmail.com ([143.107.45.1]) by smtp.gmail.com with ESMTPSA id y68sm15516925qkd.49.2019.01.13.12.23.25 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 13 Jan 2019 12:23:26 -0800 (PST) Date: Sun, 13 Jan 2019 18:23:23 -0200 From: Rodrigo Siqueira To: Daniel Vetter Cc: Gustavo Padovan , Maarten Lankhorst , Sean Paul , Dave Airlie , dri-devel , Linux Kernel Mailing List , kernel-janitors@vger.kernel.org Subject: Re: [PATCH v2] drm/drm_vblank: Change EINVAL by the correct errno Message-ID: <20190113202323.co7upmn5fg32b7gn@smtp.gmail.com> References: <20181015170529.dgzpbm37hbuvqatc@smtp.gmail.com> <20181016123541.GW31561@phenom.ffwll.local> <20181017124315.khhlykapxnug6ej6@smtp.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, I resend this patch for CI via “intel-gfx@lists.freedesktop.org” as Daniel suggested, and I got a feedback that reported an issue as can be seen here: https://patchwork.freedesktop.org/series/51147/ After a careful analysis of what happened, I concluded that the problem is related to the function “igt_wait_for_vblank_count()” in “igt_kms.c”. This function has the following assert: igt_assert(drmWaitVBlank(drm_fd, &wait_vbl) == 0) This function only checks if everything went well with the drmWaitVBlank() operation and does not make any other validation. IMHO the patch is correct, and the problem pointed out by CI is not related to this change. Make sense? Best Regards On 10/17, Daniel Vetter wrote: > On Wed, Oct 17, 2018 at 2:43 PM Rodrigo Siqueira > wrote: > > > > Hi, > > > > First of all, thanks to all for the reviewers and feedbacks. > > > > On 10/16, Daniel Vetter wrote: > > > On Mon, Oct 15, 2018 at 02:05:29PM -0300, Rodrigo Siqueira wrote: > > > > For historical reason, the function drm_wait_vblank_ioctl always return > > > > -EINVAL if something gets wrong. This scenario limits the flexibility > > > > for the userspace make detailed verification of the problem and take > > > > some action. In particular, the validation of “if (!dev->irq_enabled)” > > > > in the drm_wait_vblank_ioctl is responsible for checking if the driver > > > > support vblank or not. If the driver does not support VBlank, the > > > > function drm_wait_vblank_ioctl returns EINVAL which does not represent > > > > the real issue; this patch changes this behavior by return EOPNOTSUPP. > > > > Additionally, some operations are unsupported by this function, and > > > > returns EINVAL; this patch also changes the return value to EOPNOTSUPP > > > > in this case. Lastly, the function drm_wait_vblank_ioctl is invoked by > > > > libdrm, which is used by many compositors; because of this, it is > > > > important to check if this change breaks any compositor. In this sense, > > > > the following projects were examined: > > > > > > > > * Drm-hwcomposer > > > > * Kwin > > > > * Sway > > > > * Wlroots > > > > * Wayland-core > > > > * Weston > > > > * Xorg (67 different drivers) > > > > > > > > For each repository the verification happened in three steps: > > > > > > > > * Update the main branch > > > > * Look for any occurrence "drmWaitVBlank" with the command: > > > > git grep -n "drmWaitVBlank" > > > > * Look in the git history of the project with the command: > > > > git log -SdrmWaitVBlank > > > > > > > > Finally, none of the above projects validate the use of EINVAL which > > > > make safe, at least for these projects, to change the return values. > > > > > > > > Change since V1: > > > > Daniel Vetter and Chris Wilson > > > > - Replace ENOTTY by EOPNOTSUPP > > > > - Return EINVAL if the parameters are wrong > > > > > > > > Signed-off-by: Rodrigo Siqueira > > > > > > Reviewed-by: Daniel Vetter > > > > > > Can you pls also let intel-gfx-ci test this patch? You just need to > > > include intel-gfx@lists.freedesktop.org in your recipient list. > > > > I did know about intel-gfx-ci, really nice:) > > > > Should I CC this mailing list if I send patches to the DRM core, amdgpu, > > i915, vc4, vgem, and virtio-gpu? I suppose that IGT is running on the > > CI, right? > > It's only intel gpus (well and one special amd one) running igt on a > _lot_ of different machines. > > > Another question, do I need to send a V3 with intel-gfx-ci? > > If the patch-set is unchanged people use the "FOR CI" subject prefix > when resending, instead of incrementing the version number. > > > > For merging, since you plan to stick around doing kms stuff a bit: Want > > > commit rights for drm-misc? > > > > > > https://drm.pages.freedesktop.org/maintainer-tools/getting-started.html > > > > Yes, I want :) > > I will need some guidance, in the beginning, to get confident about the > > processes. If you can help me with this, I will be glad to play around > > with 'dim' and the merging tasks. > > Sure, just ping me on irc. First you need a freedesktop.org ssh account: > > https://www.freedesktop.org/wiki/AccountRequests/ > > You need to request access to the drm-misc group. Once you have the > bugzilla, pls ping me so I can ack it. > > Thanks, Daniel > > > > > Best Regards > > > > > Cheers, Daniel > > > > > > > --- > > > > drivers/gpu/drm/drm_vblank.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > > > > index 98e091175921..80f5a3bb427e 100644 > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > @@ -1533,10 +1533,10 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, > > > > unsigned int flags, pipe, high_pipe; > > > > > > > > if (!dev->irq_enabled) > > > > - return -EINVAL; > > > > + return -EOPNOTSUPP; > > > > > > > > if (vblwait->request.type & _DRM_VBLANK_SIGNAL) > > > > - return -EINVAL; > > > > + return -EOPNOTSUPP; > > > > > > > > if (vblwait->request.type & > > > > ~(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK | > > > > -- > > > > 2.19.1 > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > > Rodrigo Siqueira > > https://siqueira.tech > > https://twitter.com/siqueirajordao > > Graduate Student > > Department of Computer Science > > University of São Paulo > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch