From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD241C2D0DB for ; Wed, 22 Jan 2020 08:48:02 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 888F724656 for ; Wed, 22 Jan 2020 08:48:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="UKSBiEYw" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 888F724656 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E1BA86F427; Wed, 22 Jan 2020 08:48:01 +0000 (UTC) Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) by gabe.freedesktop.org (Postfix) with ESMTPS id CA6806F42F for ; Wed, 22 Jan 2020 08:48:00 +0000 (UTC) Received: by mail-wm1-x344.google.com with SMTP id f129so6242890wmf.2 for ; Wed, 22 Jan 2020 00:48:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Uq5t2nHKeAbata66c80z73yaauEQidgUJUc1sD9+41c=; b=UKSBiEYw2Ff2u9o8skUQjAGWRCWizC/tt+PKPjMUuBS1MKwkZk7S9ZXRRirzOIGMcL ytEdLy/V5Esjgq08STJdU23PvuzyXx2x1yD+vek8h+lp7NLfYGioMcx0ybEQhz/JLzWz rDRRRwjScx3CMEbSKmp6XPOLGe1npQjEq9sYc= 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:in-reply-to; bh=Uq5t2nHKeAbata66c80z73yaauEQidgUJUc1sD9+41c=; b=chMQvgYx+mV82esUHjT9xTGWNGEl2Je3/h/SZgIIwum3pFh7JsAqvfFQkSpiQhZd/m 3AoaXsa1b9C1w8d+1JxqzHtiARHSY55rybwGPIyz9vP1qARB3IlVUn1jnMhmPt1YkOG2 aqWbbEbMi1ptBecFK8reocAd738xPIvCcgxg0d2YUNpiuW0Kl9v7cfOPw9WSnolQmDsQ CbUJozKOtPLQGJaSgaQQV9Fw7BOHj+7Hnsl028eB5J8wALQdkM76EvjXJ0n8zw6YgN19 3Yq8/KUVAYCtgKWQ66HxRQ6GTf/I7moXTK1mDMzNdbGVuMl1kH6B/GBQNZAPq2lr0tLq dhjQ== X-Gm-Message-State: APjAAAUY0xYxcTc8s+HppSsEl3jwuPyzn+ANH8n98xDZiATKLkKqu3Vs Q4VeGmljBfuVpGS0glpeKyJMTg== X-Google-Smtp-Source: APXvYqwUwlSIIgmpnI5/agyTykFWWTZ7dQHIO2RCE6ucbX6R8SaUXaZpU7ceSLj4x1bRORovWhX96Q== X-Received: by 2002:a7b:c450:: with SMTP id l16mr1801170wmi.31.1579682878837; Wed, 22 Jan 2020 00:47:58 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id y7sm3867654wmd.1.2020.01.22.00.47.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Jan 2020 00:47:58 -0800 (PST) Date: Wed, 22 Jan 2020 09:47:56 +0100 From: Daniel Vetter To: Thomas Zimmermann Subject: Re: [PATCH v3 2/4] drm: Initialize struct drm_crtc_state.no_vblank from device settings Message-ID: <20200122084756.GQ43062@phenom.ffwll.local> References: <20200120122051.25178-1-tzimmermann@suse.de> <20200120122051.25178-3-tzimmermann@suse.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200120122051.25178-3-tzimmermann@suse.de> X-Operating-System: Linux phenom 5.3.0-3-amd64 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: david@lechnology.com, oleksandr_andrushchenko@epam.com, airlied@linux.ie, sam@ravnborg.org, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, hdegoede@redhat.com, kraxel@redhat.com, xen-devel@lists.xenproject.org, emil.velikov@collabora.com, sean@poorly.run, laurent.pinchart@ideasonboard.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Mon, Jan 20, 2020 at 01:20:49PM +0100, Thomas Zimmermann wrote: > At the end of a commit, atomic helpers can generate a VBLANK event > automatically. Originally implemented for writeback connectors, the > functionality can be used by any driver and/or hardware without proper > VBLANK interrupt. > > First of all, the patch updates the documentation to make this behaviour > official: settings struct drm_crtc_state.no_vblank to true enables > automatic VBLANK generation. > > Atomic modesetting helper set the initial value of no_vblank in > drm_atomic_helper_check_modeset(). If vblanking has been initialized > for a CRTC, no_blank is disabled. Otherwise it's enabled. Hence, > atomic helpers will automatically send out VBLANK events with any > driver that did not initialize vblanking. > > As drivers previously send out VBLANK events by themselves, all > affected drivers have to be updated as well. Usually, deleting the > driver's vblanking code is sufficient. Xen implements its own logic > for generating events and therefore needs to override no_vblank > with a value of false. > > v3: > * squash all related changes patches into this patch Hm, since the fall-back only happens when the driver hasn't sent out the even I think it'd be safe to split the driver cleanups into a separate patch. Makes the core/helper changes stand out more properly. Even the xen hunk I think isn't strictly needed, since that pick up the event correctly and clears state->event to NULL. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/arc/arcpgu_crtc.c | 16 -------------- > drivers/gpu/drm/bochs/bochs_kms.c | 9 -------- > drivers/gpu/drm/cirrus/cirrus.c | 8 ------- > drivers/gpu/drm/drm_atomic_helper.c | 10 ++++++++- > drivers/gpu/drm/drm_mipi_dbi.c | 9 -------- > drivers/gpu/drm/drm_vblank.c | 9 ++++++++ > drivers/gpu/drm/qxl/qxl_display.c | 14 ------------ > drivers/gpu/drm/tiny/gm12u320.c | 9 -------- > drivers/gpu/drm/tiny/ili9225.c | 9 -------- > drivers/gpu/drm/tiny/repaper.c | 9 -------- > drivers/gpu/drm/tiny/st7586.c | 9 -------- > drivers/gpu/drm/vboxvideo/vbox_mode.c | 12 ----------- > drivers/gpu/drm/virtio/virtgpu_display.c | 8 ------- > drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 ++++++++++++ > include/drm/drm_crtc.h | 27 ++++++++++++++++++------ > include/drm/drm_simple_kms_helper.h | 7 ++++-- > 16 files changed, 56 insertions(+), 122 deletions(-) > > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c > index 8ae1e1f97a73..be7c29cec318 100644 > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c > @@ -9,7 +9,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -138,24 +137,9 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc, > ~ARCPGU_CTRL_ENABLE_MASK); > } > > -static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc, > - struct drm_crtc_state *state) > -{ > - struct drm_pending_vblank_event *event = crtc->state->event; > - > - if (event) { > - crtc->state->event = NULL; > - > - spin_lock_irq(&crtc->dev->event_lock); > - drm_crtc_send_vblank_event(crtc, event); > - spin_unlock_irq(&crtc->dev->event_lock); > - } > -} > - > static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = { > .mode_valid = arc_pgu_crtc_mode_valid, > .mode_set_nofb = arc_pgu_crtc_mode_set_nofb, > - .atomic_begin = arc_pgu_crtc_atomic_begin, > .atomic_enable = arc_pgu_crtc_atomic_enable, > .atomic_disable = arc_pgu_crtc_atomic_disable, > }; > diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c > index 3f0006c2470d..ff275faee88d 100644 > --- a/drivers/gpu/drm/bochs/bochs_kms.c > +++ b/drivers/gpu/drm/bochs/bochs_kms.c > @@ -7,7 +7,6 @@ > #include > #include > #include > -#include > > #include "bochs.h" > > @@ -57,16 +56,8 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe, > struct drm_plane_state *old_state) > { > struct bochs_device *bochs = pipe->crtc.dev->dev_private; > - struct drm_crtc *crtc = &pipe->crtc; > > bochs_plane_update(bochs, pipe->plane.state); > - > - if (crtc->state->event) { > - spin_lock_irq(&crtc->dev->event_lock); > - drm_crtc_send_vblank_event(crtc, crtc->state->event); > - crtc->state->event = NULL; > - spin_unlock_irq(&crtc->dev->event_lock); > - } > } > > static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = { > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c > index 248c9f765c45..a91fb0d7282c 100644 > --- a/drivers/gpu/drm/cirrus/cirrus.c > +++ b/drivers/gpu/drm/cirrus/cirrus.c > @@ -38,7 +38,6 @@ > #include > #include > #include > -#include > > #define DRIVER_NAME "cirrus" > #define DRIVER_DESC "qemu cirrus vga" > @@ -434,13 +433,6 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, > > if (drm_atomic_helper_damage_merged(old_state, state, &rect)) > cirrus_fb_blit_rect(pipe->plane.state->fb, &rect); > - > - if (crtc->state->event) { > - spin_lock_irq(&crtc->dev->event_lock); > - drm_crtc_send_vblank_event(crtc, crtc->state->event); > - crtc->state->event = NULL; > - spin_unlock_irq(&crtc->dev->event_lock); > - } > } > > static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = { > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 4511c2e07bb9..6e9c730a8919 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -583,6 +583,7 @@ mode_valid(struct drm_atomic_state *state) > * &drm_crtc_state.connectors_changed is set when a connector is added or > * removed from the CRTC. &drm_crtc_state.active_changed is set when > * &drm_crtc_state.active changes, which is used for DPMS. > + * &drm_crtc_state.no_vblank is set from the result of drm_crtc_has_vblank(). > * See also: drm_atomic_crtc_needs_modeset() > * > * IMPORTANT: > @@ -649,6 +650,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > > return -EINVAL; > } > + > + if (drm_crtc_has_vblank(crtc)) > + new_crtc_state->no_vblank = false; > + else > + new_crtc_state->no_vblank = true; Yeah this looks much better than my hack :-) > } > > ret = handle_conflicting_encoders(state, false); > @@ -2215,7 +2221,9 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies); > * when a job is queued, and any change to the pipeline that does not touch the > * connector is leading to timeouts when calling > * drm_atomic_helper_wait_for_vblanks() or > - * drm_atomic_helper_wait_for_flip_done(). > + * drm_atomic_helper_wait_for_flip_done(). In addition to writeback > + * connectors, this function can also fake VBLANK events for CRTCs without > + * VBLANK interrupt. I still think we should reword this entire paragraph to make the "hw has no vblank" the main use-case, with writeback connectors as the "Also used for ..." special case. > * > * This is part of the atomic helper support for nonblocking commits, see > * drm_atomic_helper_setup_commit() for an overview. > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c > index 16bff1be4b8a..13b753cb3f67 100644 > --- a/drivers/gpu/drm/drm_mipi_dbi.c > +++ b/drivers/gpu/drm/drm_mipi_dbi.c > @@ -24,7 +24,6 @@ > #include > #include > #include > -#include > #include