On 04/05/16 15:30, Ville Syrjälä wrote:
On Wed, May 04, 2016 at 02:40:34PM +0100, Lionel Landwerlin wrote:
We are currently missing the color management update condition to
commit planes on crtc.
v2: add comment about moving the commit of color management registers
to an async worker
v3: Commit color management register right after vblank
Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218d..c6acfe5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13688,6 +13688,11 @@ static int intel_atomic_commit(struct drm_device *dev,
if (dev_priv->display.optimize_watermarks)
dev_priv->display.optimize_watermarks(intel_cstate);
+
+ if (crtc->state->color_mgmt_changed) {
+ intel_color_set_csc(crtc->state);
As I said earlier, csc shouldn't be here, at least on pch
platforms. And someone should actually double check whether
vlv/chv have double buffered csc registers or not. Oh and
with these frankensocs the double buffering scheme used
(if any) might be totally crazy, like it is for the pipe B
primary plane scaler on chv. A fact which the spec fails
to explain IIRC. So I'd recommend poking at the hardware
to figure out how it actually works.
Where would you put this for pch platforms?
If this patch is wrong the surely the content of intel_begin_crtc_commit
is too right?
I'm also struggling to understand why the double buffering of the CSC
registers matters.
Most people will want to configure this at the same time they configure
the gamma/degamma
LUTs to achieve color management and if the LUTs aren't double buffered
then why is it
relevant for the CSC?
Thanks for your time :)
+ intel_color_load_luts(crtc->state);
+ }
}
for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
--
2.8.0.rc3.226.g39d4020
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
