From patchwork Thu Jul 6 15:36:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Packard X-Patchwork-Id: 9828535 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 55F7560317 for ; Thu, 6 Jul 2017 15:36:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4504D28653 for ; Thu, 6 Jul 2017 15:36:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 38389286BC; Thu, 6 Jul 2017 15:36:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C9ED228653 for ; Thu, 6 Jul 2017 15:36:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E094E6E62F; Thu, 6 Jul 2017 15:36:03 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from elaine.keithp.com (home.keithp.com [63.227.221.253]) by gabe.freedesktop.org (Postfix) with ESMTP id 78C816E62F for ; Thu, 6 Jul 2017 15:36:02 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by elaine.keithp.com (Postfix) with ESMTP id 5B3FB3F20E2E; Thu, 6 Jul 2017 08:36:01 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at keithp.com Received: from elaine.keithp.com ([127.0.0.1]) by localhost (elaine.keithp.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 4P4GKmqGI71R; Thu, 6 Jul 2017 08:36:00 -0700 (PDT) Received: from hiro.keithp.com (hiro.keithp.com [10.0.0.36]) by elaine.keithp.com (Postfix) with ESMTPSA id DE3E13F20DF3; Thu, 6 Jul 2017 08:35:59 -0700 (PDT) Received: by hiro.keithp.com (Postfix, from userid 1001) id D18E619011CC; Thu, 6 Jul 2017 08:36:00 -0700 (PDT) From: Keith Packard To: Daniel Vetter Subject: Re: [PATCH 2/3] drm: Reorganize drm_pending_event to support future event types In-Reply-To: <20170706073049.mfmjvujx4szf6qji@phenom.ffwll.local> References: <20170705221013.27940-1-keithp@keithp.com> <20170705221013.27940-3-keithp@keithp.com> <20170706073049.mfmjvujx4szf6qji@phenom.ffwll.local> Date: Thu, 06 Jul 2017 08:36:00 -0700 Message-ID: <864lupd1cv.fsf@keithp.com> MIME-Version: 1.0 Cc: Dave Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Daniel Vetter writes: > A few nits below, but looks good otherwise. Thanks. >> static struct drm_pending_vblank_event *create_vblank_event( >> - struct drm_device *dev, uint64_t user_data) >> + struct drm_device *dev, struct drm_crtc *crtc, uint64_t user_data) > > Nit: Please also drop the dev argument, we have crtc->dev easily > available. That fits better into my long-term goal of getting rid of the > (dev, pipe) pairs everywhere in the vblank code and fully switching over > to drm_crtc *. As 'dev' isn't used anyways, this seems like a fine plan. >> + switch (e->event.base.type) { >> + case DRM_EVENT_VBLANK: >> + case DRM_EVENT_FLIP_COMPLETE: >> + if (seq) >> + e->event.vbl.sequence = (u32) seq; >> + if (now) { >> + e->event.vbl.tv_sec = now->tv_sec; >> + e->event.vbl.tv_usec = now->tv_nsec / 1000; >> + } >> + break; >> + } > > Not sure why this change? Also prep for the new, presumably extended > events? Seems at least slightly inconsistent with other paths, where we > still unconditionally fill it in. Yes, this prepares for the new events to make that patch smaller. The places where the data are still unconditionally assigned should know that the event in the struct is either a VBLANK or FLIP_COMPLETE. >> + struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe); > > This'll oops on ums drivers since kms isn't set up. How about this fix? > Or maybe I shouldn't have told you this and seized this opportunity to > break all the old drivers :-) You now know my evil plan :-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 857b7cf011e1..e39b2bd074e4 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -1355,7 +1355,6 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe, union drm_wait_vblank *vblwait, struct drm_file *file_priv) { - struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe); struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; struct drm_pending_vblank_event *e; struct timespec now; @@ -1373,7 +1372,12 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe, e->event.base.type = DRM_EVENT_VBLANK; e->event.base.length = sizeof(e->event.vbl); e->event.vbl.user_data = vblwait->request.signal; - e->event.vbl.crtc_id = crtc ? crtc->base.id : 0; + e->event.vbl.crtc_id = 0; + if (drm_core_check_feature(dev, DRIVER_MODESET)) { + struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe); + if (crtc) + e->event.vbl.crtc_id = crtc->base.id; + } spin_lock_irqsave(&dev->event_lock, flags);