[Intel-gfx] [PATCH] drm/i915/color: fix broken display in icl+

[Sharma, Swati2]

On 01-Oct-19 7:51 PM, Ville Syrjälä wrote:
> On Tue, Oct 01, 2019 at 11:03:08AM +0300, Jani Nikula wrote:
>> On Tue, 01 Oct 2019, Swati Sharma <swati2.sharma at intel.com> wrote:
>>> Premature gamma lut prepration and loading which was getting
>>> reflected in first modeset causing different colors on
>>> screen during boot.
>>>
>>> Issue: In BIOS, gamma is disabled by default. However,
>>> legacy_read_luts() was getting called even before the legacy_load_luts()
>>> which was setting crtc_state->base.gamma_lut and gamma_lut was
>>> programmed with junk values which led to visual artifacts (different
>>> colored screens instead of usual black during boot).
>>>
>>> Fix: Calling read_luts() only when gamma is enabled which will happen
>>> after first modeset.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111809
>>
>> I'm confused. Is there a current problem upstream after the revert
>> 1b8588741fdc ("Revert "drm/i915/color: Extract icl_read_luts()"")?
>>
>> Or does this fix a problem that only occurs in conjunction with the
>> reverted commit? Then say so.
>>
>> Note inline.
>>
>>> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_display.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index f1328c08f4ad..f89aa4bb9f42 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -10528,7 +10528,9 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>>   		i9xx_get_pipe_color_config(pipe_config);
>>>   	}
>>>   
>>> -	intel_color_get_config(pipe_config);
>>> +	if ((INTEL_GEN(dev_priv) >= 11 && (pipe_config->gamma_mode & POST_CSC_GAMMA_ENABLE)) ||
>>> +	   (INTEL_GEN(dev_priv) >= 9 && (pipe_config->gamma_enable)))
>>> +			intel_color_get_config(pipe_config);
>>
>> Put all of the conditions inside intel_color_get_config().
> 
> In fact inside the .read_luts() since these checks are platform
> specific.
> 
> Also this check is wrong for CHV since it has a separate
> enable knob for the CGM LUT (gamma_enable only deals with the
> legacy LUT) >
Inside read_luts() I already have. But the issue is first read_lut() 
will happen before load_lut() and it will replace 
crtc_state->base.gamma_lut and gamma_lut will be programmed with junk 
values which led to multiple colored screens. So we need a check to call
intel_color_get_config() itself.

-- 
~Swati Sharma