From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 6/9] drm/i915: Add eDP support for Valleyview Date: Thu, 27 Sep 2012 15:50:58 +0200 Message-ID: <20120927135058.GE2098@bremse> References: <1348666658-31345-1-git-send-email-vijay.a.purushothaman@intel.com> <1348666658-31345-7-git-send-email-vijay.a.purushothaman@intel.com> <20120926143146.GJ1980@bremse> <20120926144948.GN1980@bremse> <506456E1.5070904@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-bk0-f49.google.com (mail-bk0-f49.google.com [209.85.214.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 9634A9F676 for ; Thu, 27 Sep 2012 06:51:04 -0700 (PDT) Received: by bkwj4 with SMTP id j4so1702502bkw.36 for ; Thu, 27 Sep 2012 06:51:03 -0700 (PDT) Content-Disposition: inline In-Reply-To: <506456E1.5070904@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Vijay Purushothaman Cc: "Nikula, Jani" , Intel Graphics List-Id: intel-gfx@lists.freedesktop.org On Thu, Sep 27, 2012 at 07:08:41PM +0530, Vijay Purushothaman wrote: > On 9/26/2012 8:19 PM, Daniel Vetter wrote: > >On Wed, Sep 26, 2012 at 04:31:46PM +0200, Daniel Vetter wrote: > >>On Wed, Sep 26, 2012 at 07:07:35PM +0530, Vijay Purushothaman wrote: > >>>Eventhough Valleyview display block is derived from Cantiga, VLV > >>>supports eDP. So, added eDP checks in i9xx_crtc_mode_set path. > >>> > >>>v2: use different DPIO_DIVISOR values for VGA, DP and eDP > >>>v3: fix DPIO value calculation to use same values for all display > >>> interfaces > >>> > >>>Signed-off-by: Gajanan Bhat > >>>Signed-off-by: Vijay Purushothaman > >>>Signed-off-by: Ben Widawsky > >>>--- > >>> drivers/gpu/drm/i915/intel_display.c | 6 ++++++ > >>> drivers/gpu/drm/i915/intel_dp.c | 13 ++++++++----- > >>> 2 files changed, 14 insertions(+), 5 deletions(-) > >>> > >>>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>>index a8a81d1..aee6151 100644 > >>>--- a/drivers/gpu/drm/i915/intel_display.c > >>>+++ b/drivers/gpu/drm/i915/intel_display.c > >>>@@ -4405,6 +4405,12 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, > >>> } > >>> } > >>> > >>>+ if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) { > >>>+ pipeconf |= PIPECONF_BPP_6 | > >>>+ PIPECONF_ENABLE | > >>>+ I965_PIPECONF_ACTIVE; > >>>+ } > >> > >>No. > >> > >>Jani Nikula and me just figured out that we have a giant mess with 6bpc > >>dithering on DP outputs, but unconditionally enabling 6bpc on vlv eDP only > >>papers over issues. > > > >Forgotten to put Jani on cc. > >-Daniel > > Thanks for the catch. I've removed this unconditional enabling of > 6bpc for VLV eDP. For long term i believe, eDP handling in > i9xx_crtc_mode_set should be changed along the lines of > ironlake_crtc_mode_set for cleaner code. For now, this should > unblock others with VLV enabling. My Big Plan (tm) for handling bpc, bandwidth, dotclocks and clocks in general (e.g. pll sharing) is to move all this stuff out of the ->mode_set callbacks and into a new preparation step at the beginning of the modeset sequence (i.e. before we start touching the hw). Essentially I want to extend the current ->mode_adjust callbacks and move all these calculations in there. Aims for this are - should allow us to move a lot of the encoder specific code that currently sits in the xxx_crtc_mode_set functions into encoder callbacks - allows us to fail a modeset that won't work (e.g. due to pll sharing limits or fdi link bw limits) _before_ we touch the hw - prepares for the global modeset stuff, where we want to make clever decision about shared resources (e.g. enable 6bpc dithering if there are not enough fdi lanes availbale in 3 pipe configs). So the ironlake bpc code is a bit cleaner, but imho still gets it wrong. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch