Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Emil Velikov <emil.l.velikov@gmail.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	david@lechnology.com, oleksandr_andrushchenko@epam.com,
	Dave Airlie <airlied@linux.ie>, Sean Paul <sean@poorly.run>,
	ML dri-devel <dri-devel@lists.freedesktop.org>,
	"open list:VIRTIO GPU DRIVER"
	<virtualization@lists.linux-foundation.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	xen-devel@lists.xenproject.org, Sam Ravnborg <sam@ravnborg.org>,
	Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [Xen-devel] [PATCH v2 4/4] drm/simple-kms: Let DRM core send VBLANK events by default
Date: Thu, 16 Jan 2020 17:22:34 +0000
Message-ID: <CACvgo52gwC6U5HjnsQSUUDgE7Gp_EDb-QqCY8VDFjAX7cE0Lxg@mail.gmail.com> (raw)
In-Reply-To: <33fdd33f-ce8d-70d3-544e-fac727d2686b@suse.de>

Hi all,

On Thu, 16 Jan 2020 at 07:37, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 7cf3cf936547..23d2f51fc1d4 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -149,6 +149,11 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> >       /* Self refresh should be canceled when a new update is available */
> >       state->active = drm_atomic_crtc_effectively_active(state);
> >       state->self_refresh_active = false;
> > +
> > +     if (drm_dev_has_vblank(crtc->dev))
> > +             state->no_vblank = true;
> > +     else
> > +             state->no_vblank = false;
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
>
> I think the if/else branches are in the wrong order.
>
> But generally speaking, is it really that easy? The xen driver already
> has to work around simple-kms's auto-enabling of no_vblank (see patch
> 4). Maybe this settings interferes with other drivers as well. At least
> the calls for sending fake vblanks should be removed from all affected
> drivers.
>

I'm not sure if setting no_vblank based on dev->num_crtcs is the correct thing.
From the original commit and associated description for no_vblank:

In some cases CRTCs are active but are not able to generating events, at
least not at every frame at it's expected to.
This is typically the case when the CRTC is feeding a writeback connector...

Reflects the ability of a CRTC to send VBLANK events....


The proposed handling of no_vblank feels a little dirty, although
nothing better comes to mind.
Nevertheless code seems perfectly reasonable, so if it were me I'd merge it.

HTH
Emil

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15 12:52 [Xen-devel] [PATCH v2 0/4] Use no_vblank property for drivers without VBLANK Thomas Zimmermann
2020-01-15 12:52 ` [Xen-devel] [PATCH v2 1/4] drm: Document struct drm_crtc_state.no_vblank for faking VBLANK events Thomas Zimmermann
2020-01-15 12:52 ` [Xen-devel] [PATCH v2 2/4] drm/ast: Set struct drm_crtc_state.no_vblank in atomic_check() Thomas Zimmermann
2020-01-15 12:52 ` [Xen-devel] [PATCH v2 3/4] drm/cirrus: Let DRM core send VBLANK events Thomas Zimmermann
2020-01-15 12:52 ` [Xen-devel] [PATCH v2 4/4] drm/simple-kms: Let DRM core send VBLANK events by default Thomas Zimmermann
2020-01-16  6:41   ` Daniel Vetter
2020-01-16  7:37     ` Thomas Zimmermann
2020-01-16 17:22       ` Emil Velikov [this message]
2020-01-16 23:59         ` Daniel Vetter
2020-01-17  7:17           ` Thomas Zimmermann
2020-01-15 13:04 ` [Xen-devel] [PATCH v2 0/4] Use no_vblank property for drivers without VBLANK Hans de Goede
  -- strict thread matches above, loose matches on Subject: below --
2020-01-13 10:38 Thomas Zimmermann
2020-01-13 10:38 ` [Xen-devel] [PATCH v2 4/4] drm/simple-kms: Let DRM core send VBLANK events by default Thomas Zimmermann

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACvgo52gwC6U5HjnsQSUUDgE7Gp_EDb-QqCY8VDFjAX7cE0Lxg@mail.gmail.com \
    --to=emil.l.velikov@gmail.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=david@lechnology.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.velikov@collabora.com \
    --cc=hdegoede@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=oleksandr_andrushchenko@epam.com \
    --cc=sam@ravnborg.org \
    --cc=sean@poorly.run \
    --cc=tzimmermann@suse.de \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git