From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 07/36] drm/i915: move pipe bpp computation to pipe_config Date: Thu, 21 Feb 2013 16:49:19 +0200 Message-ID: <20130221144919.GF4469@intel.com> References: <1361407828-2419-1-git-send-email-daniel.vetter@ffwll.ch> <1361407828-2419-8-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id EF5E7E6957 for ; Thu, 21 Feb 2013 06:49:22 -0800 (PST) Content-Disposition: inline In-Reply-To: <1361407828-2419-8-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Thu, Feb 21, 2013 at 01:49:59AM +0100, Daniel Vetter wrote: > The procedure has now 3 steps: > = > 1. Compute the bpp that the plane will output, this is done in > pipe_config_set_bpp and stored into pipe_config->pipe_bpp. Also, > this function clamps the pipe_bpp to whatever limit the EDID of any > connected output specifies. > 2. Adjust the pipe_bpp in the encoder and crtc functions, according to > whatever constraints there are. > 3. Decide whether to use dither by comparing the stored plane bpp with > computed pipe_bpp. > = > There are a few slight functional changes in this patch: > - LVDS connector are now also going through the EDID clamping. But in > a 2nd change we now unconditionally force the lvds bpc value - this > shouldn't matter in reality when the panel setup is consistent, but > better safe than sorry. > - HDMI now forces the pipe_bpp to the selected value - I think that's > what we actually want, since otherwise at least the pixelclock > computations are wrong (I'm not sure whether the port would accept > e.g. 10 bpc when in 12bpc mode). Contrary to the old code, we pick > the next higher bpc value, since otherwise there's no way to make > use of the 12 bpc mode (since the next patch will remove the 12bpc > plane format, it doesn't exist). > = > Both of these changes are due to the removal of the > = > pipe_bpp =3D min(display_bpp, plane_bpp); > = > statement. > = > Another slight change is the reworking of the dp bpc code: > - For the mode_valid callback it's sufficient to only check whether > the mode would fit at the lowest bpc. > - The bandwidth computation code is a bit restructured: It now walks > all available bpp values in an outer loop and the codeblock the > compute derived values (once a good configuration is found) has been > moved out of the for loop maze. This is prep work to allow us to > successively fall back on bpc values, and also correctly support bpc > values !=3D 8 or 6. > = > If we ever get around to adding additional dithering (e.g. to squeeze > a mode into 2 fdi lanes), we need to add a flag that tells the crtc > configuration adjusting code that the bpp value selected by the output > can't be lowered. Since we don't have such logic for now, let it be. > = > v2: Rebased on top of Paulo Zanoni's little refactoring to use more > drm dp helper functions. > = > v3: Rebased on top of Jani's eDP bpp fix and Ville's limited color > range work. > = > v4: Remove the INTEL_MODE_DP_FORCE_6BPC #define, no longer needed. > = > v5: Remove intel_crtc->bpp, too, and fix up the 12bpc check in the > hdmi code. Also fixup the bpp check in intel_dp.c, it'll get reworked > in a later patch though again. > = > v6: Fix spelling in a comment. > = > v7: Debug output improvements for the bpp computation. > = > v8: Fixup 6bpc lvds check - dual-link and 8bpc mode are different > things! > = > v9: Reinstate the fix to properly ignore the firmware edp bpp ... this > was lost in a rebase. > = > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_ddi.c | 11 +- > drivers/gpu/drm/i915/intel_display.c | 223 ++++++++++++-----------------= ------ > drivers/gpu/drm/i915/intel_dp.c | 105 ++++++++--------- > drivers/gpu/drm/i915/intel_drv.h | 9 +- > drivers/gpu/drm/i915/intel_hdmi.c | 17 ++- > drivers/gpu/drm/i915/intel_lvds.c | 12 ++ > 6 files changed, 157 insertions(+), 220 deletions(-) > = > @@ -7392,14 +7255,72 @@ static void intel_modeset_commit_output_state(str= uct drm_device *dev) > } > } > = > +static int > +pipe_config_set_bpp(struct drm_crtc *crtc, > + struct drm_framebuffer *fb, > + struct intel_crtc_config *pipe_config) > +{ > + struct drm_device *dev =3D crtc->dev; > + struct drm_connector *connector; > + int bpp; > + > + switch (fb->depth) { > + case 8: > + bpp =3D 8*3; /* since we go through a colormap */ > + break; > + case 15: > + case 16: > + bpp =3D 6*3; /* min is 18bpp */ > + break; > + case 24: > + bpp =3D 8*3; > + break; > + case 30: > + bpp =3D 10*3; > + break; > + case 48: > + bpp =3D 12*3; > + break; > + default: > + DRM_DEBUG_KMS("unsupported depth\n"); > + return -EINVAL; > + } > + > + if (fb->depth > 24 && !HAS_PCH_SPLIT(dev)) { > + DRM_DEBUG_KMS("high depth not supported on gmch platforms\n"); > + return -EINVAL; > + } AFAIK Gen4 does support 2:10:10:10 formats. This would fail there, no? -- = Ville Syrj=E4l=E4 Intel OTC