Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes. (v2)

[Ville Syrjälä]

On Mon, Aug 31, 2015 at 02:03:30PM -0700, Bob Paauwe wrote:
> Extend this to SKL and BXT as it's needed for these platforms as well.
> 
> v2: Change if condition to HAS_DDI() instead of listing each platform
> Signed-off-by: Bob Paauwe 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 88f9764..ba180f6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10001,7 +10001,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
> u32 base)
>   }
>   cntl |= pipe << 28; /* Connect to correct pipe */
>  
> - if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + if (HAS_DDI(dev))

Yeah, while cursors themselves don't change between non-DDI and DDI
platforms, the reason for this stuff is the fact that that we use
the pipe csc for HDMI color range mangling on DDI platforms, since
they no longer have any other pipe/port controls for it.

So makese sense to check for DDI here too I think:
Reviewed-by: Ville Syrjälä 

>   cntl |= CURSOR_PIPE_CSC_ENABLE;
>   }
>  
> -- 
> 2.1.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes. (v2)

[Daniel Vetter]

On Thu, Oct 15, 2015 at 03:49:44PM +0300, Ville Syrjälä wrote:
> On Mon, Aug 31, 2015 at 02:03:30PM -0700, Bob Paauwe wrote:
> > Extend this to SKL and BXT as it's needed for these platforms as well.
> > 
> > v2: Change if condition to HAS_DDI() instead of listing each platform
> > Signed-off-by: Bob Paauwe 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 88f9764..ba180f6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10001,7 +10001,7 @@ static void i9xx_update_cursor(struct drm_crtc 
> > *crtc, u32 base)
> > }
> > cntl |= pipe << 28; /* Connect to correct pipe */
> >  
> > -   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > +   if (HAS_DDI(dev))
> 
> Yeah, while cursors themselves don't change between non-DDI and DDI
> platforms, the reason for this stuff is the fact that that we use
> the pipe csc for HDMI color range mangling on DDI platforms, since
> they no longer have any other pipe/port controls for it.
> 
> So makese sense to check for DDI here too I think:
> Reviewed-by: Ville Syrjälä 

Both patches applied, thanks.
-Daniel

> 
> > cntl |= CURSOR_PIPE_CSC_ENABLE;
> > }
> >  
> > -- 
> > 2.1.0
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes.

[shuang . he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 7280
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
ILK -1  302/302  301/302
SNB  315/315  315/315
IVB  336/336  336/336
BYT  283/283  283/283
HSW  378/378  378/378
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*ILK  igt@kms_flip@wf_vblank-vs-modeset-interruptible  PASS(1)  
DMESG_WARN(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes.

[Daniel Stone]

Hi Bob,

On 27 August 2015 at 21:46, Bob Paauwe bob.j.paa...@intel.com wrote:
 Extend this to SKL and BXT as it's needed for these platforms as well.

 Signed-off-by: Bob Paauwe bob.j.paa...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 88f9764..007bf7d 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -10001,7 +10001,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
 u32 base)
 }
 cntl |= pipe  28; /* Connect to correct pipe */

 -   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 +   if (IS_HASWELL(dev) || IS_BROADWELL(dev) ||
 +   IS_SKYLAKE(dev) || IS_BROXTON(dev))
 cntl |= CURSOR_PIPE_CSC_ENABLE;

For both this and the previous patch, cf. the corresponding patch for
HSW/BDW[0], have you ensured these values are sanitised at startup,
even if UEFI hasn't set something clever? Enabling fastboot on my
(UEFI-based) BDW caused a black screen because were enabling CSC but
with an empty table.

Cheers,
Daniel

[0]: https://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg67294.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes.

[Bob Paauwe]

On Fri, 28 Aug 2015 15:19:04 +0100
Daniel Stone dan...@fooishbar.org wrote:

 Hi Bob,
 
 On 27 August 2015 at 21:46, Bob Paauwe bob.j.paa...@intel.com wrote:
  Extend this to SKL and BXT as it's needed for these platforms as well.
 
  Signed-off-by: Bob Paauwe bob.j.paa...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 88f9764..007bf7d 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -10001,7 +10001,8 @@ static void i9xx_update_cursor(struct drm_crtc 
  *crtc, u32 base)
  }
  cntl |= pipe  28; /* Connect to correct pipe */
 
  -   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
  +   if (IS_HASWELL(dev) || IS_BROADWELL(dev) ||
  +   IS_SKYLAKE(dev) || IS_BROXTON(dev))
  cntl |= CURSOR_PIPE_CSC_ENABLE;
 
 For both this and the previous patch, cf. the corresponding patch for
 HSW/BDW[0], have you ensured these values are sanitised at startup,
 even if UEFI hasn't set something clever? Enabling fastboot on my
 (UEFI-based) BDW caused a black screen because were enabling CSC but
 with an empty table.
 
 Cheers,
 Daniel
 
 [0]: 
 https://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg67294.html

Hmm, no I didn't.  I assumed that it was all set up correctly since for
SKL+ the primary plane always has PIPE_CSC enabled. My two patches are
just to ensure that both the cursor and sprite planes also have it
enabled.  If all the planes are configured the same, it causes a lot of
CRC failures in the igt tests.  

Unless I'm missing something (very possible), the pipe CSC setup/lack of
setup is a separate issue.

Looking at Maarten's patch, it looks like mine above should have been
written as 

  if (HAS_DDI(dev))

instead of all the separate conditions. 

Bob


-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes.

[Daniel Stone]

Hi,

On 28 August 2015 at 16:55, Bob Paauwe bob.j.paa...@intel.com wrote:
 On Fri, 28 Aug 2015 15:19:04 +0100
 Daniel Stone dan...@fooishbar.org wrote:
 For both this and the previous patch, cf. the corresponding patch for
 HSW/BDW[0], have you ensured these values are sanitised at startup,
 even if UEFI hasn't set something clever? Enabling fastboot on my
 (UEFI-based) BDW caused a black screen because were enabling CSC but
 with an empty table.

 Cheers,
 Daniel

 [0]: 
 https://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg67294.html

 Hmm, no I didn't.  I assumed that it was all set up correctly since for
 SKL+ the primary plane always has PIPE_CSC enabled.

So does HSW/BDW. ;) The problem is that a CSC is only applied in the
modeset path; when using 'fastboot' to skip the original modeset (i.e.
to achieve flicker-free and quicker boot), the plane configuration
applies CSC/gamma, but the tables are only applied on modeset. So you
may end up with an invalid configuration (and blank screen), without a
similar patch.

 My two patches are
 just to ensure that both the cursor and sprite planes also have it
 enabled.  If all the planes are configured the same, it causes a lot of
 CRC failures in the igt tests.

 Unless I'm missing something (very possible), the pipe CSC setup/lack of
 setup is a separate issue.

Yeah, it is. But it'd be good to make sure Maarten's patch gets dragged in too.

 Looking at Maarten's patch, it looks like mine above should have been
 written as

   if (HAS_DDI(dev))

 instead of all the separate conditions.

Yeah, indeed. I'd misread the HAS_DDI bit myself, so it seems fine.
Are you able to test Maarten's patch to always program the CSC tables
and make sure it doesn't break SKL/BXT?

Thanks, and sorry for the confusion.

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes.

[Bob Paauwe]

On Fri, 28 Aug 2015 17:12:12 +0100
Daniel Stone dan...@fooishbar.org wrote:

 Hi,
 
 On 28 August 2015 at 16:55, Bob Paauwe bob.j.paa...@intel.com wrote:
  On Fri, 28 Aug 2015 15:19:04 +0100
  Daniel Stone dan...@fooishbar.org wrote:
  For both this and the previous patch, cf. the corresponding patch for
  HSW/BDW[0], have you ensured these values are sanitised at startup,
  even if UEFI hasn't set something clever? Enabling fastboot on my
  (UEFI-based) BDW caused a black screen because were enabling CSC but
  with an empty table.
 
  Cheers,
  Daniel
 
  [0]: 
  https://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg67294.html
 
  Hmm, no I didn't.  I assumed that it was all set up correctly since for
  SKL+ the primary plane always has PIPE_CSC enabled.
 
 So does HSW/BDW. ;) The problem is that a CSC is only applied in the
 modeset path; when using 'fastboot' to skip the original modeset (i.e.
 to achieve flicker-free and quicker boot), the plane configuration
 applies CSC/gamma, but the tables are only applied on modeset. So you
 may end up with an invalid configuration (and blank screen), without a
 similar patch.
 
  My two patches are
  just to ensure that both the cursor and sprite planes also have it
  enabled.  If all the planes are configured the same, it causes a lot of
  CRC failures in the igt tests.
 
  Unless I'm missing something (very possible), the pipe CSC setup/lack of
  setup is a separate issue.
 
 Yeah, it is. But it'd be good to make sure Maarten's patch gets dragged in 
 too.

That's what I thought you meant, but wanted to make sure I wasn't
confused. 

 
  Looking at Maarten's patch, it looks like mine above should have been
  written as
 
if (HAS_DDI(dev))
 
  instead of all the separate conditions.
 
 Yeah, indeed. I'd misread the HAS_DDI bit myself, so it seems fine.
 Are you able to test Maarten's patch to always program the CSC tables
 and make sure it doesn't break SKL/BXT?

I did apply his series and it doesn't change the behavior on my SKL,
but I also don't have any issues if I enable fastboot without his
series.  Which I guess means that my UEFI is setting up the table
correctly.
 
 Thanks, and sorry for the confusion.

You made me dig into how CSC works a bit more and that a good thing!
Thanks for bring it up.

 
 Cheers,
 Daniel

Bob

-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

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