[PATCH 1/2] drm/i915/skl: Enable pipe gamma for sprite planes.

[PATCH 1/2] drm/i915/skl: Enable pipe gamma for sprite planes.

[Bob Paauwe]

Since SKL has universal planes, we should configure the sprite planes
and the primary plane the same.  For the primary plane we do enable
the pipe gamma on the plane so do the same for the non-primary planes.

Without this, the pipe CRC values will be different for something
displayed on the primary plane and something displayed on a sprite
plane when the ARGB8888 format is used.

Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9d8af2f..048d397 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -189,6 +189,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	int scaler_id;
 
 	plane_ctl = PLANE_CTL_ENABLE |
+		PLANE_CTL_PIPE_GAMMA_ENABLE |
 		PLANE_CTL_PIPE_CSC_ENABLE;
 
 	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
-- 
2.1.0

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

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

[Bob Paauwe]

Extend this to SKL and BXT as it's needed for these platforms as well.

Signed-off-by: Bob Paauwe <bob.j.paauwe@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;
 	}
 
-- 
2.1.0

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

Re: [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.paauwe@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.paauwe@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: [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 <daniel@fooishbar.org> wrote:

> Hi Bob,
> 
> On 27 August 2015 at 21:46, Bob Paauwe <bob.j.paauwe@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.paauwe@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.Paauwe@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: [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.paauwe@intel.com> wrote:
> On Fri, 28 Aug 2015 15:19:04 +0100
> Daniel Stone <daniel@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: [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 <daniel@fooishbar.org> wrote:

> Hi,
> 
> On 28 August 2015 at 16:55, Bob Paauwe <bob.j.paauwe@intel.com> wrote:
> > On Fri, 28 Aug 2015 15:19:04 +0100
> > Daniel Stone <daniel@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.Paauwe@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

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

[Bob Paauwe]

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 <bob.j.paauwe@intel.com>
---
 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))
 			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

Re: [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.he@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  Test                                drm-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: [PATCH 1/2] drm/i915/skl: Enable pipe gamma for sprite planes.

[Ville Syrjälä]

On Thu, Aug 27, 2015 at 01:46:30PM -0700, Bob Paauwe wrote:
> Since SKL has universal planes, we should configure the sprite planes
> and the primary plane the same.  For the primary plane we do enable
> the pipe gamma on the plane so do the same for the non-primary planes.
> 
> Without this, the pipe CRC values will be different for something
> displayed on the primary plane and something displayed on a sprite
> plane when the ARGB8888 format is used.
> 
> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9d8af2f..048d397 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -189,6 +189,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	int scaler_id;
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
> +		PLANE_CTL_PIPE_GAMMA_ENABLE |
>  		PLANE_CTL_PIPE_CSC_ENABLE;
>  
>  	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
> -- 
> 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: [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 <bob.j.paauwe@intel.com>
> ---
>  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ä <ville.syrjala@linux.intel.com>

>  			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: [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 <bob.j.paauwe@intel.com>
> > ---
> >  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ä <ville.syrjala@linux.intel.com>

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