* [PATCH] drm/i915/skl+: Enable gamma and CSC on bottom color.
@ 2015-10-20 16:04 Bob Paauwe
2015-10-20 18:16 ` Matt Roper
2015-10-21 16:45 ` [PATCH 1/2] drm/i915/skl+: Enable gamma and CSC on bottom color. (v2) Bob Paauwe
0 siblings, 2 replies; 4+ messages in thread
From: Bob Paauwe @ 2015-10-20 16:04 UTC (permalink / raw)
To: intel-gfx
To stay consisent with how we're programming all the other planes,
enable gamma and CSC on the bottom color. Without this, we fail the
the kms_universal_plane functional tests because the black primary plane
is brighter (gamma corrected) than the disabled plane case. If the bottom
color is also gamma/csc corrected, then the disiabled case will match the
black plane case.
testcase: igt/kms_universal_plane/universal-plane-pipe-[ABC]-functional
CC: Konduru, Chandra <chandra.konduru@intel.com>
cc: Kevin Strasser <kevin.strasser@linux.intel.com>
Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
drivers/gpu/drm/i915/intel_display.c | 7 +++++++
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 724f057..9ad330b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4479,6 +4479,16 @@ enum skl_disp_power_wells {
#define PIPEMISC_DITHER_TYPE_SP (0<<2)
#define PIPEMISC(pipe) _PIPE2(pipe, _PIPE_MISC_A)
+#define _PIPE_BOTTOM_COLOR_A 0x70034
+#define _PIPE_BOTTOM_COLOR_B 0x71034
+#define _PIPE_BOTTOM_COLOR_C 0x72034
+#define PIPE_BOTTOM_GAMMA_ENABLE (1<<31)
+#define PIPE_BOTTOM_CSC_ENABLE (1<<30)
+#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF
+#define PIPE_BOTTOM_COLOR(pipe) _PIPE3(pipe, _PIPE_BOTTOM_COLOR_A, \
+ _PIPE_BOTTOM_COLOR_B, \
+ _PIPE_BOTTOM_COLOR_C)
+
#define VLV_DPFLIPSTAT (VLV_DISPLAY_BASE + 0x70028)
#define PIPEB_LINE_COMPARE_INT_EN (1<<29)
#define PIPEB_HLINE_INT_EN (1<<28)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3b4bacf..7665730 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4949,6 +4949,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->state);
bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
+ u32 bottom;
if (WARN_ON(intel_crtc->active))
return;
@@ -5033,6 +5034,12 @@ 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);
}
+
+ if (INTEL_INFO(dev)->gen >= 9) {
+ bottom = I915_READ(PIPE_BOTTOM_COLOR(pipe));
+ bottom |= (PIPE_BOTTOM_CSC_ENABLE | PIPE_BOTTOM_GAMMA_ENABLE);
+ I915_WRITE(PIPE_BOTTOM_COLOR(pipe), bottom);
+ }
}
static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
--
2.4.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/skl+: Enable gamma and CSC on bottom color.
2015-10-20 16:04 [PATCH] drm/i915/skl+: Enable gamma and CSC on bottom color Bob Paauwe
@ 2015-10-20 18:16 ` Matt Roper
2015-10-21 6:47 ` Daniel Vetter
2015-10-21 16:45 ` [PATCH 1/2] drm/i915/skl+: Enable gamma and CSC on bottom color. (v2) Bob Paauwe
1 sibling, 1 reply; 4+ messages in thread
From: Matt Roper @ 2015-10-20 18:16 UTC (permalink / raw)
To: Bob Paauwe; +Cc: intel-gfx
On Tue, Oct 20, 2015 at 09:04:56AM -0700, Bob Paauwe wrote:
> To stay consisent with how we're programming all the other planes,
> enable gamma and CSC on the bottom color. Without this, we fail the
> the kms_universal_plane functional tests because the black primary plane
> is brighter (gamma corrected) than the disabled plane case. If the bottom
> color is also gamma/csc corrected, then the disiabled case will match the
> black plane case.
>
> testcase: igt/kms_universal_plane/universal-plane-pipe-[ABC]-functional
> CC: Konduru, Chandra <chandra.konduru@intel.com>
> cc: Kevin Strasser <kevin.strasser@linux.intel.com>
> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
> drivers/gpu/drm/i915/intel_display.c | 7 +++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 724f057..9ad330b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4479,6 +4479,16 @@ enum skl_disp_power_wells {
> #define PIPEMISC_DITHER_TYPE_SP (0<<2)
> #define PIPEMISC(pipe) _PIPE2(pipe, _PIPE_MISC_A)
>
> +#define _PIPE_BOTTOM_COLOR_A 0x70034
> +#define _PIPE_BOTTOM_COLOR_B 0x71034
> +#define _PIPE_BOTTOM_COLOR_C 0x72034
> +#define PIPE_BOTTOM_GAMMA_ENABLE (1<<31)
> +#define PIPE_BOTTOM_CSC_ENABLE (1<<30)
> +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF
> +#define PIPE_BOTTOM_COLOR(pipe) _PIPE3(pipe, _PIPE_BOTTOM_COLOR_A, \
> + _PIPE_BOTTOM_COLOR_B, \
> + _PIPE_BOTTOM_COLOR_C)
> +
> #define VLV_DPFLIPSTAT (VLV_DISPLAY_BASE + 0x70028)
> #define PIPEB_LINE_COMPARE_INT_EN (1<<29)
> #define PIPEB_HLINE_INT_EN (1<<28)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3b4bacf..7665730 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4949,6 +4949,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> struct intel_crtc_state *pipe_config =
> to_intel_crtc_state(crtc->state);
> bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
> + u32 bottom;
>
> if (WARN_ON(intel_crtc->active))
> return;
> @@ -5033,6 +5034,12 @@ 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);
> }
> +
> + if (INTEL_INFO(dev)->gen >= 9) {
> + bottom = I915_READ(PIPE_BOTTOM_COLOR(pipe));
Do we really want to do the r/m/w of the current color (meaning that we
could get something that isn't even black if our boot firmware messed
with the register), or should we just set it to explicit black with the
CSC/gamma bits turned on? Eventually that hardcoded black would change
once we land properties to allow userspace to change the color, but
setting it explicitly to black today seems reasonable to me.
Anyway, this does solve the i-g-t failures for me, so
Tested-by(BXT): Matt Roper <matthew.d.roper@intel.com>
Matt
> + bottom |= (PIPE_BOTTOM_CSC_ENABLE | PIPE_BOTTOM_GAMMA_ENABLE);
> + I915_WRITE(PIPE_BOTTOM_COLOR(pipe), bottom);
> + }
> }
>
> static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/skl+: Enable gamma and CSC on bottom color.
2015-10-20 18:16 ` Matt Roper
@ 2015-10-21 6:47 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-10-21 6:47 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Tue, Oct 20, 2015 at 11:16:20AM -0700, Matt Roper wrote:
> On Tue, Oct 20, 2015 at 09:04:56AM -0700, Bob Paauwe wrote:
> > To stay consisent with how we're programming all the other planes,
> > enable gamma and CSC on the bottom color. Without this, we fail the
> > the kms_universal_plane functional tests because the black primary plane
> > is brighter (gamma corrected) than the disabled plane case. If the bottom
> > color is also gamma/csc corrected, then the disiabled case will match the
> > black plane case.
> >
> > testcase: igt/kms_universal_plane/universal-plane-pipe-[ABC]-functional
> > CC: Konduru, Chandra <chandra.konduru@intel.com>
> > cc: Kevin Strasser <kevin.strasser@linux.intel.com>
> > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
> > drivers/gpu/drm/i915/intel_display.c | 7 +++++++
> > 2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 724f057..9ad330b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4479,6 +4479,16 @@ enum skl_disp_power_wells {
> > #define PIPEMISC_DITHER_TYPE_SP (0<<2)
> > #define PIPEMISC(pipe) _PIPE2(pipe, _PIPE_MISC_A)
> >
> > +#define _PIPE_BOTTOM_COLOR_A 0x70034
> > +#define _PIPE_BOTTOM_COLOR_B 0x71034
> > +#define _PIPE_BOTTOM_COLOR_C 0x72034
> > +#define PIPE_BOTTOM_GAMMA_ENABLE (1<<31)
> > +#define PIPE_BOTTOM_CSC_ENABLE (1<<30)
> > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF
> > +#define PIPE_BOTTOM_COLOR(pipe) _PIPE3(pipe, _PIPE_BOTTOM_COLOR_A, \
> > + _PIPE_BOTTOM_COLOR_B, \
> > + _PIPE_BOTTOM_COLOR_C)
> > +
> > #define VLV_DPFLIPSTAT (VLV_DISPLAY_BASE + 0x70028)
> > #define PIPEB_LINE_COMPARE_INT_EN (1<<29)
> > #define PIPEB_HLINE_INT_EN (1<<28)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3b4bacf..7665730 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4949,6 +4949,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > struct intel_crtc_state *pipe_config =
> > to_intel_crtc_state(crtc->state);
> > bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
> > + u32 bottom;
> >
> > if (WARN_ON(intel_crtc->active))
> > return;
> > @@ -5033,6 +5034,12 @@ 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);
> > }
> > +
> > + if (INTEL_INFO(dev)->gen >= 9) {
> > + bottom = I915_READ(PIPE_BOTTOM_COLOR(pipe));
>
> Do we really want to do the r/m/w of the current color (meaning that we
> could get something that isn't even black if our boot firmware messed
> with the register), or should we just set it to explicit black with the
> CSC/gamma bits turned on? Eventually that hardcoded black would change
> once we land properties to allow userspace to change the color, but
> setting it explicitly to black today seems reasonable to me.
rmw considered harmful (mostly because it just hides bugs which then show
up when we cleanse all registers with runtime PM while all outputs are
dpms off). Just write black.
> Anyway, this does solve the i-g-t failures for me, so
>
> Tested-by(BXT): Matt Roper <matthew.d.roper@intel.com>
As said in the other thread, I think an overall series with these fixes
for all recent-ish platforms would be great. Plus confirmation from hw
engineers.
-Daniel
>
>
> Matt
>
> > + bottom |= (PIPE_BOTTOM_CSC_ENABLE | PIPE_BOTTOM_GAMMA_ENABLE);
> > + I915_WRITE(PIPE_BOTTOM_COLOR(pipe), bottom);
> > + }
> > }
> >
> > static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] drm/i915/skl+: Enable gamma and CSC on bottom color. (v2)
2015-10-20 16:04 [PATCH] drm/i915/skl+: Enable gamma and CSC on bottom color Bob Paauwe
2015-10-20 18:16 ` Matt Roper
@ 2015-10-21 16:45 ` Bob Paauwe
1 sibling, 0 replies; 4+ messages in thread
From: Bob Paauwe @ 2015-10-21 16:45 UTC (permalink / raw)
To: intel-gfx
To stay consisent with how we're programming all the other planes,
enable gamma and CSC on the bottom color. Without this, we fail the
the kms_universal_plane functional tests because the black primary plane
is brighter (gamma corrected) than the disabled plane case. If the bottom
color is also gamma/csc corrected, then the disiabled case will match the
black plane case.
v2: Instead of preserving the existing bottom color, set it to black
as we don't yet have any way to set it to another color. (Matt)
testcase: igt/kms_universal_plane/universal-plane-pipe-[ABC]-functional
CC: Konduru, Chandra <chandra.konduru@intel.com>
cc: Kevin Strasser <kevin.strasser@linux.intel.com>
Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
drivers/gpu/drm/i915/intel_display.c | 6 ++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 724f057..9ad330b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4479,6 +4479,16 @@ enum skl_disp_power_wells {
#define PIPEMISC_DITHER_TYPE_SP (0<<2)
#define PIPEMISC(pipe) _PIPE2(pipe, _PIPE_MISC_A)
+#define _PIPE_BOTTOM_COLOR_A 0x70034
+#define _PIPE_BOTTOM_COLOR_B 0x71034
+#define _PIPE_BOTTOM_COLOR_C 0x72034
+#define PIPE_BOTTOM_GAMMA_ENABLE (1<<31)
+#define PIPE_BOTTOM_CSC_ENABLE (1<<30)
+#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF
+#define PIPE_BOTTOM_COLOR(pipe) _PIPE3(pipe, _PIPE_BOTTOM_COLOR_A, \
+ _PIPE_BOTTOM_COLOR_B, \
+ _PIPE_BOTTOM_COLOR_C)
+
#define VLV_DPFLIPSTAT (VLV_DISPLAY_BASE + 0x70028)
#define PIPEB_LINE_COMPARE_INT_EN (1<<29)
#define PIPEB_HLINE_INT_EN (1<<28)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3b4bacf..a0986cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4949,6 +4949,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->state);
bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
+ u32 bottom;
if (WARN_ON(intel_crtc->active))
return;
@@ -5033,6 +5034,11 @@ 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);
}
+
+ if (INTEL_INFO(dev)->gen >= 9) {
+ bottom = (PIPE_BOTTOM_CSC_ENABLE | PIPE_BOTTOM_GAMMA_ENABLE);
+ I915_WRITE(PIPE_BOTTOM_COLOR(pipe), bottom);
+ }
}
static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
--
2.4.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-21 16:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-20 16:04 [PATCH] drm/i915/skl+: Enable gamma and CSC on bottom color Bob Paauwe
2015-10-20 18:16 ` Matt Roper
2015-10-21 6:47 ` Daniel Vetter
2015-10-21 16:45 ` [PATCH 1/2] drm/i915/skl+: Enable gamma and CSC on bottom color. (v2) Bob Paauwe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox