Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-09 Thread Ville Syrjälä 
On Wed, Sep 09, 2015 at 02:51:58AM +0300, Konduru, Chandra wrote:
> > > > > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > > > > + int pipe, int enable)
> > > > > +{
> > > > > + if (pipe == PIPE_A || pipe == PIPE_B) {
> > > > > + if (enable)
> > > > > + I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > > > + DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> > > > > + else
> > > > > + I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > > > + I915_READ(CLKGATE_DIS_PSL(pipe) &
> > > > > + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
> > > > > + }
> > > > > +}
> > > > > +
> > > > >  static void haswell_crtc_enable(struct drm_crtc *crtc)
> > > > >  {
> > > > >   struct drm_device *dev = crtc->dev;
> > > > > @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc
> > > > *crtc)
> > > > >   intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > > > >   intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > > > >   }
> > > > > +
> > > > > + /* workaround for NV12 */
> > > > > + skl_wa_clkgate(dev_priv, pipe, 1);
> > > >
> > > > I wonder what's the cost of having this
> > > > a) always enabled
> > > > b) enabled when the pipe is enabled
> > > > c) enabled only when NV12 is used
> > > > ?
> > >
> > > Initially optimized to enable only when nv12 is used,
> > > but there are some corner cases when planes switch to and
> > > from nv12 to non-nv12 and SV recommendation is to enable
> > > always; and SV evaluated cost, and it isn't a big concern.
> > 
> > So, based on that we could just stuff it into init_clock_gating and
> > forget about it.
> 
> Couldn't include into init_clock_gating because this requires
> a pipe based check.

init_clock_gating()
{
...
enable for pipe A
enable for pipe B
...
}

or

for_each_pipe(pipe)
if (pipe != C)
enable w/a


> 
> By the way, so far 4 patches got RB tags.
> In the respun series 
> (http://lists.freedesktop.org/archives/intel-gfx/2015-September/075235.html
> addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
> Can you check updated patches and issue R-B tags for remaining ones?

I think your trigger finger is a bit overly sensitive :) We still had
open issues in this series so posting another one makes things somewhat
messy.

-- 
Ville Syrjälä
Intel OTC
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-09 Thread Konduru, Chandra 
> > > > > > +
> > > > > > +   /* workaround for NV12 */
> > > > > > +   skl_wa_clkgate(dev_priv, pipe, 1);
> > > > >
> > > > > I wonder what's the cost of having this
> > > > > a) always enabled
> > > > > b) enabled when the pipe is enabled
> > > > > c) enabled only when NV12 is used
> > > > > ?
> > > >
> > > > Initially optimized to enable only when nv12 is used,
> > > > but there are some corner cases when planes switch to and
> > > > from nv12 to non-nv12 and SV recommendation is to enable
> > > > always; and SV evaluated cost, and it isn't a big concern.
> > >
> > > So, based on that we could just stuff it into init_clock_gating and
> > > forget about it.
> >
> > Couldn't include into init_clock_gating because this requires
> > a pipe based check.
> 
> init_clock_gating()
> {
>   ...
>   enable for pipe A
>   enable for pipe B
>   ...
> }
> 
> or
> 
> for_each_pipe(pipe)
>   if (pipe != C)
>   enable w/a
> 

Oh yeah, it can be done that way.
OK, will move it there. 

> 
> >
> > By the way, so far 4 patches got RB tags.
> > In the respun series (http://lists.freedesktop.org/archives/intel-gfx/2015-
> September/075235.html
> > addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
> > Can you check updated patches and issue R-B tags for remaining ones?
> 
> I think your trigger finger is a bit overly sensitive :) We still had
> open issues in this series so posting another one makes things somewhat
> messy.
There is one other open relate to tile-Yf Y, UV plane pitch/stride check. 
I just responded to that one. 

> 
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-08 Thread Konduru, Chandra 
> > > > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > > > +   int pipe, int enable)
> > > > +{
> > > > +   if (pipe == PIPE_A || pipe == PIPE_B) {
> > > > +   if (enable)
> > > > +   I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > > +   DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> > > > +   else
> > > > +   I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > > +   I915_READ(CLKGATE_DIS_PSL(pipe) &
> > > > +   ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
> > > > +   }
> > > > +}
> > > > +
> > > >  static void haswell_crtc_enable(struct drm_crtc *crtc)
> > > >  {
> > > > struct drm_device *dev = crtc->dev;
> > > > @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc
> > > *crtc)
> > > > intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > > > intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > > > }
> > > > +
> > > > +   /* workaround for NV12 */
> > > > +   skl_wa_clkgate(dev_priv, pipe, 1);
> > >
> > > I wonder what's the cost of having this
> > > a) always enabled
> > > b) enabled when the pipe is enabled
> > > c) enabled only when NV12 is used
> > > ?
> >
> > Initially optimized to enable only when nv12 is used,
> > but there are some corner cases when planes switch to and
> > from nv12 to non-nv12 and SV recommendation is to enable
> > always; and SV evaluated cost, and it isn't a big concern.
> 
> So, based on that we could just stuff it into init_clock_gating and
> forget about it.

Couldn't include into init_clock_gating because this requires
a pipe based check.

By the way, so far 4 patches got RB tags.
In the respun series 
(http://lists.freedesktop.org/archives/intel-gfx/2015-September/075235.html
addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
Can you check updated patches and issue R-B tags for remaining ones?

> 
> But we'll run into problems as soon as render compression enters the
> picure. But I don't have a problem leaving it up to the render
> compression patches to solve.
> 
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-05 Thread Ville Syrjälä 
On Sat, Sep 05, 2015 at 01:28:56AM +, Konduru, Chandra wrote:
> > > +
> > > + if (((IS_SKYLAKE(dev) && intel_get_stepping(dev) == 'C') ||
> > > + (IS_BROXTON(dev) && intel_get_stepping(dev) == 'A')) &&
> > > + fb->pixel_format == DRM_FORMAT_NV12) {
> > > + I915_WRITE(CHICKEN_PIPESL(pipe),
> > > + I915_READ(CHICKEN_PIPESL(pipe)) |
> > DISABLE_STREAMER_FIX);
> > > + }
> > 
> > According to Bspec this would need to be disabled for render
> > compression. And to do that we'd need to add some vblank waits to make
> > sure we don't disable it too soon. But since it's pre-production
> > hardware anyway I guess we might not care too much.
> 
> Render compression related checks will be coming as part of that patch.
> 
> > 
> > I would probably drop SKL from these since I'd assume almost everyone
> > has D+ by now. And maybe just stuff it in init_clock_gating for BXT
> > since we're going to eliminate it soonish anyway.
> 
> Last checked some folks are still using, so keep it intact for completeness.
> 
> > > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > > + int pipe, int enable)
> > > +{
> > > + if (pipe == PIPE_A || pipe == PIPE_B) {
> > > + if (enable)
> > > + I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > + DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> > > + else
> > > + I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > + I915_READ(CLKGATE_DIS_PSL(pipe) &
> > > + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
> > > + }
> > > +}
> > > +
> > >  static void haswell_crtc_enable(struct drm_crtc *crtc)
> > >  {
> > >   struct drm_device *dev = crtc->dev;
> > > @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc
> > *crtc)
> > >   intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > >   intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > >   }
> > > +
> > > + /* workaround for NV12 */
> > > + skl_wa_clkgate(dev_priv, pipe, 1);
> > 
> > I wonder what's the cost of having this
> > a) always enabled
> > b) enabled when the pipe is enabled
> > c) enabled only when NV12 is used
> > ?
> 
> Initially optimized to enable only when nv12 is used,
> but there are some corner cases when planes switch to and 
> from nv12 to non-nv12 and SV recommendation is to enable
> always; and SV evaluated cost, and it isn't a big concern.

So, based on that we could just stuff it into init_clock_gating and
forget about it.

But we'll run into problems as soon as render compression enters the
picure. But I don't have a problem leaving it up to the render
compression patches to solve.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-04 Thread Ville Syrjälä 
On Wed, Aug 19, 2015 at 06:02:35PM -0700, Chandra Konduru wrote:
> Adding driver workarounds for nv12.
> 
> Signed-off-by: Chandra Konduru 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |   20 
>  drivers/gpu/drm/i915/intel_csr.c |2 +-
>  drivers/gpu/drm/i915/intel_display.c |   31 +++
>  drivers/gpu/drm/i915/intel_drv.h |1 +
>  drivers/gpu/drm/i915/intel_sprite.c  |7 +++
>  5 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c4d732f..3192837 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5354,6 +5354,26 @@ enum skl_disp_power_wells {
>  #define PLANE_NV12_BUF_CFG(pipe, plane)  \
>   _PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), _PLANE_NV12_BUF_CFG_2(pipe))
>  
> +/*
> + * Skylake Chicken registers
> + */
> +#define _CHICKEN_PIPESL_A  0x420B0
> +#define _CHICKEN_PIPESL_B  0x420B4
> +#define _CHICKEN_PIPESL_C  0x420B8
> +#define  DISABLE_STREAMER_FIX  (1 << 22)
> +#define CHICKEN_PIPESL(pipe) _PIPE(pipe, _CHICKEN_PIPESL_A, 
> _CHICKEN_PIPESL_B)
> +
> +#define CHICKEN_DCPR_1 0x46430
> +#define IDLE_WAKEMEM_MASK  (1 << 13)
> +
> +#define CLKGATE_DIS_PSL_A0x46520
> +#define CLKGATE_DIS_PSL_B0x46524
> +#define CLKGATE_DIS_PSL_C0x46528
> +#define DUPS1_GATING_DIS (1 << 15)
> +#define DUPS2_GATING_DIS (1 << 19)
> +#define DUPS3_GATING_DIS (1 << 23)
> +#define CLKGATE_DIS_PSL(pipe)  _PIPE(pipe, CLKGATE_DIS_PSL_A, 
> CLKGATE_DIS_PSL_B)
> +
>  /* SKL new cursor registers */
>  #define _CUR_BUF_CFG_A   0x7017c
>  #define _CUR_BUF_CFG_B   0x7117c
> diff --git a/drivers/gpu/drm/i915/intel_csr.c 
> b/drivers/gpu/drm/i915/intel_csr.c
> index ba1ae03..559a7f5 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -181,7 +181,7 @@ static const struct stepping_info skl_stepping_info[] = {
>   {'G', '0'}, {'H', '0'}, {'I', '0'}
>  };
>  
> -static char intel_get_stepping(struct drm_device *dev)
> +char intel_get_stepping(struct drm_device *dev)
>  {
>   if (IS_SKYLAKE(dev) && (dev->pdev->revision <
>   ARRAY_SIZE(skl_stepping_info)))
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 419660d..2158b8f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3196,6 +3196,16 @@ static void skylake_update_primary_plane(struct 
> drm_crtc *crtc,
>   I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
>   I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset << 16 | 
> aux_x_offset);
>  
> + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> + IS_SKYLAKE(dev), IS_BROXTON(dev));
> +
> + if (((IS_SKYLAKE(dev) && intel_get_stepping(dev) == 'C') ||
> + (IS_BROXTON(dev) && intel_get_stepping(dev) == 'A')) &&
> + fb->pixel_format == DRM_FORMAT_NV12) {
> + I915_WRITE(CHICKEN_PIPESL(pipe),
> + I915_READ(CHICKEN_PIPESL(pipe)) | 
> DISABLE_STREAMER_FIX);
> + }

According to Bspec this would need to be disabled for render
compression. And to do that we'd need to add some vblank waits to make
sure we don't disable it too soon. But since it's pre-production
hardware anyway I guess we might not care too much.

I would probably drop SKL from these since I'd assume almost everyone
has D+ by now. And maybe just stuff it in init_clock_gating for BXT
since we're going to eliminate it soonish anyway.

> +
>   if (scaler_id >= 0) {
>   uint32_t ps_ctrl = 0;
>  
> @@ -5004,6 +5014,21 @@ static bool hsw_crtc_supports_ips(struct intel_crtc 
> *crtc)
>   return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
>  }
>  
> +
> +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> + int pipe, int enable)
> +{
> + if (pipe == PIPE_A || pipe == PIPE_B) {
> + if (enable)
> + I915_WRITE(CLKGATE_DIS_PSL(pipe),
> + DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> + else
> + I915_WRITE(CLKGATE_DIS_PSL(pipe),
> + I915_READ(CLKGATE_DIS_PSL(pipe) &
> + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
> + }
> +}
> +
>  static void haswell_crtc_enable(struct drm_crtc *crtc)
>  {
>   struct drm_device *dev = crtc->dev;
> @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>   intel_wait_for_vblank(dev, hsw_workaround_pipe);
>   intel_wait_for_vblank(dev, hsw_workaround_pipe);
>   }
> +
> + /* workaround for NV12 */
> + skl_wa_clkgate(dev_priv, pipe, 1);

I wonder what's the cost

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-04 Thread Konduru, Chandra 
> > +
> > +   if (((IS_SKYLAKE(dev) && intel_get_stepping(dev) == 'C') ||
> > +   (IS_BROXTON(dev) && intel_get_stepping(dev) == 'A')) &&
> > +   fb->pixel_format == DRM_FORMAT_NV12) {
> > +   I915_WRITE(CHICKEN_PIPESL(pipe),
> > +   I915_READ(CHICKEN_PIPESL(pipe)) |
> DISABLE_STREAMER_FIX);
> > +   }
> 
> According to Bspec this would need to be disabled for render
> compression. And to do that we'd need to add some vblank waits to make
> sure we don't disable it too soon. But since it's pre-production
> hardware anyway I guess we might not care too much.

Render compression related checks will be coming as part of that patch.

> 
> I would probably drop SKL from these since I'd assume almost everyone
> has D+ by now. And maybe just stuff it in init_clock_gating for BXT
> since we're going to eliminate it soonish anyway.

Last checked some folks are still using, so keep it intact for completeness.

> > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > +   int pipe, int enable)
> > +{
> > +   if (pipe == PIPE_A || pipe == PIPE_B) {
> > +   if (enable)
> > +   I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +   DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> > +   else
> > +   I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +   I915_READ(CLKGATE_DIS_PSL(pipe) &
> > +   ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
> > +   }
> > +}
> > +
> >  static void haswell_crtc_enable(struct drm_crtc *crtc)
> >  {
> > struct drm_device *dev = crtc->dev;
> > @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc
> *crtc)
> > intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > }
> > +
> > +   /* workaround for NV12 */
> > +   skl_wa_clkgate(dev_priv, pipe, 1);
> 
> I wonder what's the cost of having this
> a) always enabled
> b) enabled when the pipe is enabled
> c) enabled only when NV12 is used
> ?

Initially optimized to enable only when nv12 is used,
but there are some corner cases when planes switch to and 
from nv12 to non-nv12 and SV recommendation is to enable
always; and SV evaluated cost, and it isn't a big concern.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-04 Thread Konduru, Chandra 
> > > On Thu, Aug 27, 2015 at 01:44:06AM +, Konduru, Chandra wrote:
> > > > > > -static char intel_get_stepping(struct drm_device *dev)
> > > > > > +char intel_get_stepping(struct drm_device *dev)
> > > > >
> > > > > I guess we should have a new home for this now that it's used outside 
> > > > > of
> > > > > intel_csr.c Plus kerneldoc, as usual.
> > > >
> > > > Will add kerneldoc header and respun, but where do you want to move to?
> > >
> > > If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
> > > better home.
> > > -Daniel
> > I sent updated patch series couple days ago moving to intel_display.c.
> > I hope that is fine.
> 
> Imo intel_display.c is the wrong place since this isn't display-specific
> really - stepping is for the entire device. intel_uncore.c, i915_drv.c or
> i915_dma.c would all be suitable places.

Moved to i915_drv.c. Sending out updated patches shortly.

> 
> > By the way, can you start giving RB tags for the patches you reviewed?
> 
> This isn't a full review, just a small thing I spotted while reading
> patches.
OK, Ville did/is doing full review. So covered.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-04 Thread Daniel Vetter 
On Thu, Sep 03, 2015 at 06:33:25PM +, Konduru, Chandra wrote:
> 
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel 
> > Vetter
> > Sent: Wednesday, September 02, 2015 1:02 AM
> > To: Konduru, Chandra
> > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Vetter, Daniel; 
> > Syrjala, Ville
> > Subject: Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds
> > 
> > On Thu, Aug 27, 2015 at 01:44:06AM +, Konduru, Chandra wrote:
> > > > > -static char intel_get_stepping(struct drm_device *dev)
> > > > > +char intel_get_stepping(struct drm_device *dev)
> > > >
> > > > I guess we should have a new home for this now that it's used outside of
> > > > intel_csr.c Plus kerneldoc, as usual.
> > >
> > > Will add kerneldoc header and respun, but where do you want to move to?
> > 
> > If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
> > better home.
> > -Daniel
> I sent updated patch series couple days ago moving to intel_display.c.
> I hope that is fine. 

Imo intel_display.c is the wrong place since this isn't display-specific
really - stepping is for the entire device. intel_uncore.c, i915_drv.c or
i915_dma.c would all be suitable places.

> By the way, can you start giving RB tags for the patches you reviewed?

This isn't a full review, just a small thing I spotted while reading
patches.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-03 Thread Konduru, Chandra 

> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, September 02, 2015 1:02 AM
> To: Konduru, Chandra
> Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, 
> Ville
> Subject: Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds
> 
> On Thu, Aug 27, 2015 at 01:44:06AM +, Konduru, Chandra wrote:
> > > > -static char intel_get_stepping(struct drm_device *dev)
> > > > +char intel_get_stepping(struct drm_device *dev)
> > >
> > > I guess we should have a new home for this now that it's used outside of
> > > intel_csr.c Plus kerneldoc, as usual.
> >
> > Will add kerneldoc header and respun, but where do you want to move to?
> 
> If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
> better home.
> -Daniel
I sent updated patch series couple days ago moving to intel_display.c.
I hope that is fine. 
By the way, can you start giving RB tags for the patches you reviewed?

> 
> >
> > >
> > > >  {
> > > > if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> > > > ARRAY_SIZE(skl_stepping_info)))
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 419660d..2158b8f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -3196,6 +3196,16 @@ static void
> skylake_update_primary_plane(struct
> > > drm_crtc *crtc,
> > > > I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
> > > > I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset << 16 |
> > > aux_x_offset);
> > > >
> > > > +   DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> > > > +   IS_SKYLAKE(dev), IS_BROXTON(dev));
> > > > +
> > >
> > > Wa comments are missing here. Also generally we do 1 wa per commit.
> > > -Daniel
> >
> > Sure, will split and respun.
> >
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-09-02 Thread Daniel Vetter 
On Thu, Aug 27, 2015 at 01:44:06AM +, Konduru, Chandra wrote:
> > > -static char intel_get_stepping(struct drm_device *dev)
> > > +char intel_get_stepping(struct drm_device *dev)
> > 
> > I guess we should have a new home for this now that it's used outside of
> > intel_csr.c Plus kerneldoc, as usual.
> 
> Will add kerneldoc header and respun, but where do you want to move to?

If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
better home.
-Daniel

> 
> > 
> > >  {
> > >   if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> > >   ARRAY_SIZE(skl_stepping_info)))
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > > index 419660d..2158b8f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3196,6 +3196,16 @@ static void skylake_update_primary_plane(struct
> > drm_crtc *crtc,
> > >   I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
> > >   I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset << 16 |
> > aux_x_offset);
> > >
> > > + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> > > + IS_SKYLAKE(dev), IS_BROXTON(dev));
> > > +
> > 
> > Wa comments are missing here. Also generally we do 1 wa per commit.
> > -Daniel
> 
> Sure, will split and respun.
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-08-26 Thread Daniel Vetter 
On Wed, Aug 19, 2015 at 06:02:35PM -0700, Chandra Konduru wrote:
 Adding driver workarounds for nv12.
 
 Signed-off-by: Chandra Konduru chandra.kond...@intel.com
 ---
  drivers/gpu/drm/i915/i915_reg.h  |   20 
  drivers/gpu/drm/i915/intel_csr.c |2 +-
  drivers/gpu/drm/i915/intel_display.c |   31 +++
  drivers/gpu/drm/i915/intel_drv.h |1 +
  drivers/gpu/drm/i915/intel_sprite.c  |7 +++
  5 files changed, 60 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index c4d732f..3192837 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -5354,6 +5354,26 @@ enum skl_disp_power_wells {
  #define PLANE_NV12_BUF_CFG(pipe, plane)  \
   _PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), _PLANE_NV12_BUF_CFG_2(pipe))
  
 +/*
 + * Skylake Chicken registers
 + */
 +#define _CHICKEN_PIPESL_A  0x420B0
 +#define _CHICKEN_PIPESL_B  0x420B4
 +#define _CHICKEN_PIPESL_C  0x420B8
 +#define  DISABLE_STREAMER_FIX  (1  22)
 +#define CHICKEN_PIPESL(pipe) _PIPE(pipe, _CHICKEN_PIPESL_A, 
 _CHICKEN_PIPESL_B)
 +
 +#define CHICKEN_DCPR_1 0x46430
 +#define IDLE_WAKEMEM_MASK  (1  13)
 +
 +#define CLKGATE_DIS_PSL_A0x46520
 +#define CLKGATE_DIS_PSL_B0x46524
 +#define CLKGATE_DIS_PSL_C0x46528
 +#define DUPS1_GATING_DIS (1  15)
 +#define DUPS2_GATING_DIS (1  19)
 +#define DUPS3_GATING_DIS (1  23)
 +#define CLKGATE_DIS_PSL(pipe)  _PIPE(pipe, CLKGATE_DIS_PSL_A, 
 CLKGATE_DIS_PSL_B)
 +
  /* SKL new cursor registers */
  #define _CUR_BUF_CFG_A   0x7017c
  #define _CUR_BUF_CFG_B   0x7117c
 diff --git a/drivers/gpu/drm/i915/intel_csr.c 
 b/drivers/gpu/drm/i915/intel_csr.c
 index ba1ae03..559a7f5 100644
 --- a/drivers/gpu/drm/i915/intel_csr.c
 +++ b/drivers/gpu/drm/i915/intel_csr.c
 @@ -181,7 +181,7 @@ static const struct stepping_info skl_stepping_info[] = {
   {'G', '0'}, {'H', '0'}, {'I', '0'}
  };
  
 -static char intel_get_stepping(struct drm_device *dev)
 +char intel_get_stepping(struct drm_device *dev)

I guess we should have a new home for this now that it's used outside of
intel_csr.c Plus kerneldoc, as usual.

  {
   if (IS_SKYLAKE(dev)  (dev-pdev-revision 
   ARRAY_SIZE(skl_stepping_info)))
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 419660d..2158b8f 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3196,6 +3196,16 @@ static void skylake_update_primary_plane(struct 
 drm_crtc *crtc,
   I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
   I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset  16 | 
 aux_x_offset);
  
 + DRM_DEBUG_KMS(KCM: is_skl = %d is_bxt = %d\n,
 + IS_SKYLAKE(dev), IS_BROXTON(dev));
 +

Wa comments are missing here. Also generally we do 1 wa per commit.
-Daniel

 + if (((IS_SKYLAKE(dev)  intel_get_stepping(dev) == 'C') ||
 + (IS_BROXTON(dev)  intel_get_stepping(dev) == 'A')) 
 + fb-pixel_format == DRM_FORMAT_NV12) {
 + I915_WRITE(CHICKEN_PIPESL(pipe),
 + I915_READ(CHICKEN_PIPESL(pipe)) | 
 DISABLE_STREAMER_FIX);
 + }
 +
   if (scaler_id = 0) {
   uint32_t ps_ctrl = 0;
  
 @@ -5004,6 +5014,21 @@ static bool hsw_crtc_supports_ips(struct intel_crtc 
 *crtc)
   return HAS_IPS(crtc-base.dev)  crtc-pipe == PIPE_A;
  }
  
 +
 +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
 + int pipe, int enable)
 +{
 + if (pipe == PIPE_A || pipe == PIPE_B) {
 + if (enable)
 + I915_WRITE(CLKGATE_DIS_PSL(pipe),
 + DUPS1_GATING_DIS | DUPS2_GATING_DIS);
 + else
 + I915_WRITE(CLKGATE_DIS_PSL(pipe),
 + I915_READ(CLKGATE_DIS_PSL(pipe) 
 + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
 + }
 +}
 +
  static void haswell_crtc_enable(struct drm_crtc *crtc)
  {
   struct drm_device *dev = crtc-dev;
 @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
   intel_wait_for_vblank(dev, hsw_workaround_pipe);
   intel_wait_for_vblank(dev, hsw_workaround_pipe);
   }
 +
 + /* workaround for NV12 */
 + skl_wa_clkgate(dev_priv, pipe, 1);
  }
  
  static void ironlake_pfit_disable(struct intel_crtc *crtc)
 @@ -5211,6 +5239,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
  
   intel_crtc-active = false;
   intel_update_watermarks(crtc);
 +
 + /* workaround for NV12 */
 + skl_wa_clkgate(dev_priv, intel_crtc-pipe, 0);
  }
  
  static void i9xx_pfit_enable(struct intel_crtc *crtc)
 diff --git a/drivers/gpu/drm/i915/intel_drv.h

Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

2015-08-26 Thread Konduru, Chandra 
  -static char intel_get_stepping(struct drm_device *dev)
  +char intel_get_stepping(struct drm_device *dev)
 
 I guess we should have a new home for this now that it's used outside of
 intel_csr.c Plus kerneldoc, as usual.

Will add kerneldoc header and respun, but where do you want to move to?

 
   {
  if (IS_SKYLAKE(dev)  (dev-pdev-revision 
  ARRAY_SIZE(skl_stepping_info)))
  diff --git a/drivers/gpu/drm/i915/intel_display.c
 b/drivers/gpu/drm/i915/intel_display.c
  index 419660d..2158b8f 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -3196,6 +3196,16 @@ static void skylake_update_primary_plane(struct
 drm_crtc *crtc,
  I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
  I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset  16 |
 aux_x_offset);
 
  +   DRM_DEBUG_KMS(KCM: is_skl = %d is_bxt = %d\n,
  +   IS_SKYLAKE(dev), IS_BROXTON(dev));
  +
 
 Wa comments are missing here. Also generally we do 1 wa per commit.
 -Daniel

Sure, will split and respun.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx