From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris BREZILLON Subject: Re: [RESEND PATCH v3 05/11] drm: add Atmel HLCDC Display Controller support Date: Tue, 15 Jul 2014 13:26:24 +0200 Message-ID: <20140715132624.00add008@bbrezillon> References: <1404751384-5077-1-git-send-email-boris.brezillon@free-electrons.com> <1404751384-5077-6-git-send-email-boris.brezillon@free-electrons.com> <20140712201654.565472d8@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-pwm-owner@vger.kernel.org To: Rob Clark Cc: Mark Rutland , "devicetree@vger.kernel.org" , Nicolas Ferre , "dri-devel@lists.freedesktop.org" , Alexandre Belloni , Laurent Pinchart , Bo Shen , Lee Jones , Jean-Jacques Hiblot , Samuel Ortiz , Tim Niemeyer , Jean-Christophe Plagniol-Villard , linux-pwm@vger.kernel.org, Pawel Moll , Ian Campbell , Rob Herring , Andrew Victor , "linux-arm-kernel@lists.infradead.org" , Thomas Petazzoni List-Id: devicetree@vger.kernel.org On Sat, 12 Jul 2014 14:37:16 -0400 Rob Clark wrote: > On Sat, Jul 12, 2014 at 2:16 PM, Boris BREZILLON > wrote: > > Hello, > > > > On Mon, 7 Jul 2014 18:42:58 +0200 > > Boris BREZILLON wrote: > > > > > >> +int atmel_hlcdc_layer_disable(struct atmel_hlcdc_layer *layer) > >> +{ > >> + struct atmel_hlcdc_layer_dma_channel *dma = &layer->dma; > >> + unsigned long flags; > >> + int i; > >> + > >> + spin_lock_irqsave(&dma->lock, flags); > >> + for (i = 0; i < layer->max_planes; i++) { > >> + if (!dma->cur[i]) > >> + break; > >> + > >> + dma->cur[i]->ctrl = 0; > >> + } > >> + spin_unlock_irqrestore(&dma->lock, flags); > >> + > >> + return 0; > >> +} > > > > > > I'm trying to simplify the hlcdc_layer code and in order to do that I > > need to know what's expected when a user calls plane_disable (or more > > exactly DRM_IOCTL_MODE_SETPLANE ioctl call with the frame buffer ID set > > to 0). > > > > The HLCDC Display Controller support two types of disable: > > > > 1) The plane is disabled at the end of the current frame (the is the > > solution I'm using) > > > > 2) The plane is disabled right away (I haven't tested it, but I think > > this solution could generate some sort of artifacts for a short period > > of time, because the framebuffer might be partially displayed) > > > > If solution 1 is chosen, should I wait for the plane to be actually > > disabled before returning ? > > for cursor in particular, if you block, it is going to be a massive > slowdown for some apps. I remember at least older gdm would rapidly > flash a spinning cursor. As a result, if you wait for vsync each > time, it would take a couple minutes to login! That makes sense. > > if #2 works, I'd recommend it. Otherwise you may have to do some of > the same hijinks that I have to do in mdp4_crtc for the cursor. I already have a working solution which does not block with #1, I was just trying to simplify my code ;-). I'll try #2 and if it works without any side effects I'll go for it. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: boris.brezillon@free-electrons.com (Boris BREZILLON) Date: Tue, 15 Jul 2014 13:26:24 +0200 Subject: [RESEND PATCH v3 05/11] drm: add Atmel HLCDC Display Controller support In-Reply-To: References: <1404751384-5077-1-git-send-email-boris.brezillon@free-electrons.com> <1404751384-5077-6-git-send-email-boris.brezillon@free-electrons.com> <20140712201654.565472d8@bbrezillon> Message-ID: <20140715132624.00add008@bbrezillon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Sat, 12 Jul 2014 14:37:16 -0400 Rob Clark wrote: > On Sat, Jul 12, 2014 at 2:16 PM, Boris BREZILLON > wrote: > > Hello, > > > > On Mon, 7 Jul 2014 18:42:58 +0200 > > Boris BREZILLON wrote: > > > > > >> +int atmel_hlcdc_layer_disable(struct atmel_hlcdc_layer *layer) > >> +{ > >> + struct atmel_hlcdc_layer_dma_channel *dma = &layer->dma; > >> + unsigned long flags; > >> + int i; > >> + > >> + spin_lock_irqsave(&dma->lock, flags); > >> + for (i = 0; i < layer->max_planes; i++) { > >> + if (!dma->cur[i]) > >> + break; > >> + > >> + dma->cur[i]->ctrl = 0; > >> + } > >> + spin_unlock_irqrestore(&dma->lock, flags); > >> + > >> + return 0; > >> +} > > > > > > I'm trying to simplify the hlcdc_layer code and in order to do that I > > need to know what's expected when a user calls plane_disable (or more > > exactly DRM_IOCTL_MODE_SETPLANE ioctl call with the frame buffer ID set > > to 0). > > > > The HLCDC Display Controller support two types of disable: > > > > 1) The plane is disabled at the end of the current frame (the is the > > solution I'm using) > > > > 2) The plane is disabled right away (I haven't tested it, but I think > > this solution could generate some sort of artifacts for a short period > > of time, because the framebuffer might be partially displayed) > > > > If solution 1 is chosen, should I wait for the plane to be actually > > disabled before returning ? > > for cursor in particular, if you block, it is going to be a massive > slowdown for some apps. I remember at least older gdm would rapidly > flash a spinning cursor. As a result, if you wait for vsync each > time, it would take a couple minutes to login! That makes sense. > > if #2 works, I'd recommend it. Otherwise you may have to do some of > the same hijinks that I have to do in mdp4_crtc for the cursor. I already have a working solution which does not block with #1, I was just trying to simplify my code ;-). I'll try #2 and if it works without any side effects I'll go for it. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com