From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() Date: Fri, 8 Aug 2014 20:53:45 +0200 Message-ID: <20140808185345.GZ8727@phenom.ffwll.local> References: <1407523871-15684-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50]) by gabe.freedesktop.org (Postfix) with ESMTP id F17E76E88C for ; Fri, 8 Aug 2014 11:53:33 -0700 (PDT) Received: by mail-wg0-f50.google.com with SMTP id n12so5830032wgh.21 for ; Fri, 08 Aug 2014 11:53:33 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1407523871-15684-1-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Aug 08, 2014 at 09:51:10PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > Move the entire DSPCNTR register setup into the .update_primary_plane() > functions. That's where it belongs anyway and it'll also help 830M which > has the extra problem that plane registers reads will return the value > latched at the last vblank, not the value that was last written. > = > Also move DSPPOS and DSPSIZE setup there. > = > v2: Don't move variable initialization to avoid churn later > = > Reviewed-by: Matt Roper > Signed-off-by: Ville Syrj=E4l=E4 Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 100 +++++++++++------------------= ------ > 1 file changed, 32 insertions(+), 68 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 89e0ac5..4158257 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2394,12 +2394,26 @@ static void i9xx_update_primary_plane(struct drm_= crtc *crtc, > int plane =3D intel_crtc->plane; > unsigned long linear_offset; > u32 dspcntr; > - u32 reg; > + u32 reg =3D DSPCNTR(plane); > + > + dspcntr =3D DISPPLANE_GAMMA_ENABLE; > + > + if (intel_crtc->primary_enabled) > + dspcntr |=3D DISPLAY_PLANE_ENABLE; > + > + if (INTEL_INFO(dev)->gen < 4) { > + if (intel_crtc->pipe =3D=3D PIPE_B) > + dspcntr |=3D DISPPLANE_SEL_PIPE_B; > + > + /* pipesrc and dspsize control the size that is scaled from, > + * which should always be the user's requested size. > + */ > + I915_WRITE(DSPSIZE(plane), > + ((intel_crtc->config.pipe_src_h - 1) << 16) | > + (intel_crtc->config.pipe_src_w - 1)); > + I915_WRITE(DSPPOS(plane), 0); > + } > = > - reg =3D DSPCNTR(plane); > - dspcntr =3D I915_READ(reg); > - /* Mask out pixel format bits in case we change it */ > - dspcntr &=3D ~DISPPLANE_PIXFORMAT_MASK; > switch (fb->pixel_format) { > case DRM_FORMAT_C8: > dspcntr |=3D DISPPLANE_8BPP; > @@ -2431,12 +2445,9 @@ static void i9xx_update_primary_plane(struct drm_c= rtc *crtc, > BUG(); > } > = > - if (INTEL_INFO(dev)->gen >=3D 4) { > - if (obj->tiling_mode !=3D I915_TILING_NONE) > - dspcntr |=3D DISPPLANE_TILED; > - else > - dspcntr &=3D ~DISPPLANE_TILED; > - } > + if (INTEL_INFO(dev)->gen >=3D 4 && > + obj->tiling_mode !=3D I915_TILING_NONE) > + dspcntr |=3D DISPPLANE_TILED; > = > if (IS_G4X(dev)) > dspcntr |=3D DISPPLANE_TRICKLE_FEED_DISABLE; > @@ -2480,12 +2491,16 @@ static void ironlake_update_primary_plane(struct = drm_crtc *crtc, > int plane =3D intel_crtc->plane; > unsigned long linear_offset; > u32 dspcntr; > - u32 reg; > + u32 reg =3D DSPCNTR(plane); > + > + dspcntr =3D DISPPLANE_GAMMA_ENABLE; > + > + if (intel_crtc->primary_enabled) > + dspcntr |=3D DISPLAY_PLANE_ENABLE; > + > + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > + dspcntr |=3D DISPPLANE_PIPE_CSC_ENABLE; > = > - reg =3D DSPCNTR(plane); > - dspcntr =3D I915_READ(reg); > - /* Mask out pixel format bits in case we change it */ > - dspcntr &=3D ~DISPPLANE_PIXFORMAT_MASK; > switch (fb->pixel_format) { > case DRM_FORMAT_C8: > dspcntr |=3D DISPPLANE_8BPP; > @@ -2515,12 +2530,8 @@ static void ironlake_update_primary_plane(struct d= rm_crtc *crtc, > = > if (obj->tiling_mode !=3D I915_TILING_NONE) > dspcntr |=3D DISPPLANE_TILED; > - else > - dspcntr &=3D ~DISPPLANE_TILED; > = > - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > - dspcntr &=3D ~DISPPLANE_TRICKLE_FEED_DISABLE; > - else > + if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) > dspcntr |=3D DISPPLANE_TRICKLE_FEED_DISABLE; > = > I915_WRITE(reg, dspcntr); > @@ -3946,7 +3957,6 @@ static void ironlake_crtc_enable(struct drm_crtc *c= rtc) > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > struct intel_encoder *encoder; > int pipe =3D intel_crtc->pipe; > - enum plane plane =3D intel_crtc->plane; > = > WARN_ON(!crtc->enabled); > = > @@ -3968,10 +3978,6 @@ static void ironlake_crtc_enable(struct drm_crtc *= crtc) > = > ironlake_set_pipeconf(crtc); > = > - /* Set up the display plane register */ > - I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE); > - POSTING_READ(DSPCNTR(plane)); > - > dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, > crtc->x, crtc->y); > = > @@ -4059,7 +4065,6 @@ static void haswell_crtc_enable(struct drm_crtc *cr= tc) > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > struct intel_encoder *encoder; > int pipe =3D intel_crtc->pipe; > - enum plane plane =3D intel_crtc->plane; > = > WARN_ON(!crtc->enabled); > = > @@ -4083,10 +4088,6 @@ static void haswell_crtc_enable(struct drm_crtc *c= rtc) > = > intel_set_pipe_csc(crtc); > = > - /* Set up the display plane register */ > - I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE | DISPPLANE_PIPE_CSC_= ENABLE); > - POSTING_READ(DSPCNTR(plane)); > - > dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, > crtc->x, crtc->y); > = > @@ -4642,9 +4643,7 @@ static void valleyview_crtc_enable(struct drm_crtc = *crtc) > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > struct intel_encoder *encoder; > int pipe =3D intel_crtc->pipe; > - int plane =3D intel_crtc->plane; > bool is_dsi; > - u32 dspcntr; > = > WARN_ON(!crtc->enabled); > = > @@ -4660,27 +4659,13 @@ static void valleyview_crtc_enable(struct drm_crt= c *crtc) > vlv_prepare_pll(intel_crtc); > } > = > - /* Set up the display plane register */ > - dspcntr =3D DISPPLANE_GAMMA_ENABLE; > - > if (intel_crtc->config.has_dp_encoder) > intel_dp_set_m_n(intel_crtc); > = > intel_set_pipe_timings(intel_crtc); > = > - /* pipesrc and dspsize control the size that is scaled from, > - * which should always be the user's requested size. > - */ > - I915_WRITE(DSPSIZE(plane), > - ((intel_crtc->config.pipe_src_h - 1) << 16) | > - (intel_crtc->config.pipe_src_w - 1)); > - I915_WRITE(DSPPOS(plane), 0); > - > i9xx_set_pipeconf(intel_crtc); > = > - I915_WRITE(DSPCNTR(plane), dspcntr); > - POSTING_READ(DSPCNTR(plane)); > - > dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, > crtc->x, crtc->y); > = > @@ -4735,8 +4720,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > struct intel_encoder *encoder; > int pipe =3D intel_crtc->pipe; > - int plane =3D intel_crtc->plane; > - u32 dspcntr; > = > WARN_ON(!crtc->enabled); > = > @@ -4745,32 +4728,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crt= c) > = > i9xx_set_pll_dividers(intel_crtc); > = > - /* Set up the display plane register */ > - dspcntr =3D DISPPLANE_GAMMA_ENABLE; > - > - if (pipe =3D=3D 0) > - dspcntr &=3D ~DISPPLANE_SEL_PIPE_MASK; > - else > - dspcntr |=3D DISPPLANE_SEL_PIPE_B; > - > if (intel_crtc->config.has_dp_encoder) > intel_dp_set_m_n(intel_crtc); > = > intel_set_pipe_timings(intel_crtc); > = > - /* pipesrc and dspsize control the size that is scaled from, > - * which should always be the user's requested size. > - */ > - I915_WRITE(DSPSIZE(plane), > - ((intel_crtc->config.pipe_src_h - 1) << 16) | > - (intel_crtc->config.pipe_src_w - 1)); > - I915_WRITE(DSPPOS(plane), 0); > - > i9xx_set_pipeconf(intel_crtc); > = > - I915_WRITE(DSPCNTR(plane), dspcntr); > - POSTING_READ(DSPCNTR(plane)); > - > dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, > crtc->x, crtc->y); > = > -- = > 1.8.5.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch