From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 04/12] drm: Add drm_vblank_on() Date: Wed, 21 May 2014 13:32:35 +0200 Message-ID: <20140521113234.GG2014@ulmo> References: <1400093477-3217-1-git-send-email-daniel.vetter@ffwll.ch> <1400093477-3217-5-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0925630739==" Return-path: In-Reply-To: <1400093477-3217-5-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Intel Graphics Development , DRI Development List-Id: dri-devel@lists.freedesktop.org --===============0925630739== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vJguvTgX93MxBIIe" Content-Disposition: inline --vJguvTgX93MxBIIe Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 14, 2014 at 08:51:06PM +0200, Daniel Vetter wrote: > From: Ville Syrj=C3=A4l=C3=A4 >=20 > drm_vblank_off() will turn off vblank interrupts, but as long as the > refcount is elevated drm_vblank_get() will not re-enable them. This > is a problem is someone is holding a vblank reference while a modeset is > happening, and the driver requires vblank interrupt to work during that > time. >=20 > Add drm_vblank_on() as a counterpart to drm_vblank_off() which will > re-enabled vblank interrupts if the refcount is already elevated. This > will allow drivers to choose the specific places in the modeset sequence > at which vblank interrupts get disabled and enabled. >=20 > Testcase: igt/kms_flip/*-vs-suspend > Signed-off-by: Ville Syrj=C3=A4l=C3=A4 > [danvet: Add Testcase tag for the igt I've written.] > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_irq.c | 72 ++++++++++++++++++++++++++----= ------ > drivers/gpu/drm/i915/intel_display.c | 8 ++++ > include/drm/drmP.h | 1 + > 3 files changed, 62 insertions(+), 19 deletions(-) >=20 > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 13d671ed3421..dd786d84daab 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -840,6 +840,41 @@ static void drm_update_vblank_count(struct drm_devic= e *dev, int crtc) > } > =20 > /** > + * drm_vblank_enable - enable the vblank interrupt on a CRTC > + * @dev: DRM device > + * @crtc: CRTC in question > + */ Perhaps the kernel-doc here should contain some of what's described in the commit message? Also a "Return:" section would be useful here to specify what's an error and what isn't. > +static int drm_vblank_enable(struct drm_device *dev, int crtc) On second thought, since this is a local function, my comments above apply to drm_vblank_on() below rather than drm_vblank_enable(). > +{ > + int ret =3D 0; > + > + assert_spin_locked(&dev->vbl_lock); > + > + spin_lock(&dev->vblank_time_lock); > + > + if (!dev->vblank[crtc].enabled) { > + /* Enable vblank irqs under vblank_time_lock protection. > + * All vblank count & timestamp updates are held off > + * until we are done reinitializing master counter and > + * timestamps. Filtercode in drm_handle_vblank() will > + * prevent double-accounting of same vblank interval. > + */ Coding style: /* * Enable... */ ? > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c Perhaps split off the i915 changes into a separate patch? Thierry --vJguvTgX93MxBIIe Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTfI7SAAoJEN0jrNd/PrOh85MP/jW6cs4UA2PwlXkHXD0bSm0l hssVQUbLsCviO2PMxdfZY6wjmQnjNUY38HmZ6EPai4euesYPRGO/dtqWTJ+tQWcM V459sWKlRjse4CMJH+ihkSSaEhgY9ASffhQ89yNXl/rJ6GalfpNCjSU5lc91SRTn pt/MXUoTOlMPgU1MfAk2rs+zMcI0vItf1Ez4kge4G5NOiQ5PO+d0DPsjy09os64M FZNSyxdqM82WKnPewBJhsU5rm9ELYZBwILQ89zKNj+9IxJx0zjSItjLqVgFXZNIL uOBNnIDfz738IkB/cRBS7oDRqJAahAim7GWlohGAlgXkLaFLfu75pOdzLjhmEPxo RHwEsNn/zBybyqtpgl0GqlO43ahZGuV6VkeFYMpHS0g0+56JsIQKSKL3+lh7/teI L3TYOna39UdNIU7euSo1rYOQRNBqOXK5HKQeTAhcDbA20bUbivV0wrfSjY7d93uH YqKyE3kxsKD5JhkptpjYK5vugZoLspUTQrednlBStp0BLvnUfydbgD4sxKu1kmZt 3pE6YFS6x/opQvq2oYU/lVDLTxb4QHd0IQQjyBNJjVS0YfttfImuhPQWMDg2UfhX dGbsydpjha5aLcVKjCnzGyQjs77vstzpRRrZEHKJIesNU2PRAk4okPLtyOvYaVlM xKyyKdu8nHmuylfzEc1U =aPh4 -----END PGP SIGNATURE----- --vJguvTgX93MxBIIe-- --===============0925630739== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0925630739==--