From patchwork Sun Oct 20 18:19:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 11201071 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6C3D4112B for ; Sun, 20 Oct 2019 18:19:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 446A6218BA for ; Sun, 20 Oct 2019 18:19:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 446A6218BA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2DEA989A4F; Sun, 20 Oct 2019 18:19:45 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id B972C89A4F for ; Sun, 20 Oct 2019 18:19:43 +0000 (UTC) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-295-HhBXooJ3PU61AyvC2dJ_dQ-1; Sun, 20 Oct 2019 14:19:38 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 582B91005500; Sun, 20 Oct 2019 18:19:37 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-116-98.ams2.redhat.com [10.36.116.98]) by smtp.corp.redhat.com (Postfix) with ESMTP id A85AC5D6A3; Sun, 20 Oct 2019 18:19:35 +0000 (UTC) From: Hans de Goede To: Maarten Lankhorst , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= Date: Sun, 20 Oct 2019 20:19:33 +0200 Message-Id: <20191020181933.54829-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-MC-Unique: HhBXooJ3PU61AyvC2dJ_dQ-1 X-Mimecast-Spam-Score: 0 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1571595582; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Vox23DInnqzKWHhCSR/qZP9c9d3PYc2CN3zl60k2Mjw=; b=YexZZnrpLoouELzIqJMjXgAnS9RUdTi4/7gQl3hj2IvE17ovVJj3Js+3f67ps0E/jerp0I B5PhqjUSxfzx9oOYsfMC+ezYkt5fgBzY/slc3yM6+L5Bv1JC2R1Gbs/GCcCEXuDs904awf 9Pde7MUDZaAKHs/Umka3xi77mZ7GQ3Q= Subject: [Intel-gfx] [PATCH] drm/i915: Program LUT before intel_color_commit() if LUT was not previously set X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx , dri-devel@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Since commit 051a6d8d3ca0 ("drm/i915: Move LUT programming to happen after vblank waits"), I am seeing an ugly colored flash of the first few display lines on 2 Cherry Trail devices when the gamma table gets set for the first time. A blue flash on a GPD win and a yellow flash on an Asus T100HA. The problem is that since this change, the LUT is programmed after the write *and latching* of the double-buffered register which causes the LUT to be used starting at the next frame. This means that the old LUT is still used for the first couple of lines of the display. If no LUT was in use before then the LUT registers may contain bogus values. This leads to messed up colors until the new LUT values are written. At least on CHT DSI panels this causes messed up colors on the first few lines. This commit fixes this by adding a load_lut_before_commit boolean, modifying intel_begin_crtc_commit to load the luts earlier if this is set, and setting this from intel_color_check when a LUT table was not in use before (and thus may contain bogus values), or when the table size changes. Fixes: 051a6d8d3ca0 ("drm/i915: Move LUT programming to happen after vblank waits") Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/display/intel_color.c | 26 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_display.c | 7 +++++ .../drm/i915/display/intel_display_types.h | 3 +++ 3 files changed, 36 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 71a0201437a9..0da6dcc5bebd 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1052,6 +1052,32 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state) new_crtc_state->update_planes |= BIT(plane->id); } + /* + * Normally we load the LUTs after vblank / after the double-buffer + * registers written by commit have been latched, this avoids a + * gamma change mid-way the screen. This does mean that the first + * few lines of the display will (sometimes) still use the old + * table. This is fine when changing an existing LUT, but if this + * is the first time the LUT gets loaded, then the hw may contain + * random values, causing the first lines to have funky colors. + * + * So if were enabling a LUT for the first time or changing the table + * size, then we must do this before the commit to avoid corrupting + * the first lines of the display. + */ + if (!old_crtc_state->base.gamma_lut && new_crtc_state->base.gamma_lut) + new_crtc_state->load_lut_before_commit = true; + else if (!old_crtc_state->base.degamma_lut && + new_crtc_state->base.degamma_lut) + new_crtc_state->load_lut_before_commit = true; + else if (old_crtc_state->base.gamma_lut && + new_crtc_state->base.gamma_lut && + lut_is_legacy(old_crtc_state->base.gamma_lut) != + lut_is_legacy(new_crtc_state->base.gamma_lut)) + new_crtc_state->load_lut_before_commit = true; + else + new_crtc_state->load_lut_before_commit = false; + return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index aa54bb22796d..21442b0dd134 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -14033,6 +14033,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { if (new_crtc_state->base.active && !needs_modeset(new_crtc_state) && + !new_crtc_state->load_lut_before_commit && (new_crtc_state->base.color_mgmt_changed || new_crtc_state->update_pipe)) intel_color_load_luts(new_crtc_state); @@ -14529,6 +14530,12 @@ static void intel_begin_crtc_commit(struct intel_atomic_state *state, intel_atomic_get_new_crtc_state(state, crtc); bool modeset = needs_modeset(new_crtc_state); + if (!modeset && + new_crtc_state->load_lut_before_commit && + (new_crtc_state->base.color_mgmt_changed || + new_crtc_state->update_pipe)) + intel_color_load_luts(new_crtc_state); + /* Perform vblank evasion around commit operation */ intel_pipe_update_start(new_crtc_state); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 449abaea619f..bbdeb3be64e6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -973,6 +973,9 @@ struct intel_crtc_state { /* enable pipe csc? */ bool csc_enable; + /* load luts before color settings commit */ + bool load_lut_before_commit; + /* Display Stream compression state */ struct { bool compression_enable;