From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shobhit Kumar Subject: Re: [PATCH 4/4] drm/i915: Add support for Generic MIPI panel driver Date: Fri, 16 May 2014 14:53:11 +0530 Message-ID: <5375D8FF.30708@intel.com> References: <1397454507-10273-1-git-send-email-shobhit.kumar@intel.com> <1397454507-10273-5-git-send-email-shobhit.kumar@intel.com> <20140515164857.GJ27103@strange.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 96F246EF90 for ; Fri, 16 May 2014 02:23:19 -0700 (PDT) In-Reply-To: <20140515164857.GJ27103@strange.amr.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Damien Lespiau Cc: Jani Nikula , Daniel Vetter , intel-gfx , arjan.van.de.ven@intel.com List-Id: intel-gfx@lists.freedesktop.org Thanks Damien for your review On Thursday 15 May 2014 10:18 PM, Damien Lespiau wrote: > On Mon, Apr 14, 2014 at 11:18:27AM +0530, Shobhit Kumar wrote: >> >This driver makes use of the generic panel information from the VBT. >> >Panel information is classified into two - panel configuration and panel >> >power sequence which is unique to each panel. The generic driver uses the >> >panel configuration and sequence parsed from VBT block #52 and #53 >> > >> >v2: Address review comments by Jani >> > - Move all of the things in driver c file from header >> > - Make all functions static >> > - Make use of video/mipi_display.c instead of redefining >> > - Null checks during sequence execution >> > >> >Signed-off-by: Shobhit Kumar > I've done a first past on this. Overall looks reasonable. I'm missing > some documentation to double check the various LP->HS, HS->LP count and > other magic around the clocks (send you a mail about it) before I can > add my r-b tag. > > I've added a few tiny comments as well along the road. All look okay to me and Will push updated patch asap. There is one issue which I am struggling for now. If we have all these patches in, then disable sequence pipe off does not work and wait_for_pipe_off gives a warn dump but everything works. Its not this patch issue but DSI patches that are already merged. I know the fix is to actually disable port after disabling pipe and plane but doing that does not succeed enable in first attempt. Subsequent disable/enable works. Looking into that and should have a fix by next week on that. Regards Shobhit