[Intel-gfx] [PATCH 1/2] drm/i915/display/vlv_dsi: Do not skip panel_pwr_cycle_delay when disabling the panel

[Hans de Goede]

Hi,

On 4/7/21 3:57 PM, Ville Syrjälä wrote:
> On Wed, Apr 07, 2021 at 03:50:35PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 4/7/21 2:34 PM, Ville Syrjälä wrote:
>>> On Tue, Apr 06, 2021 at 03:57:32PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 3/25/21 12:48 PM, Hans de Goede wrote:
>>>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down
>>>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based
>>>>> Predia Basic tablet would no longer properly light up after reboot.
>>>>>
>>>>> I've managed to reproduce this without rebooting by doing:
>>>>> chvt 3; echo 1 > /sys/class/graphics/fb0/blank;\
>>>>> echo 0 > /sys/class/graphics/fb0/blank
>>>>>
>>>>> Which rapidly turns the panel off and back on again.
>>>>>
>>>>> The vlv_dsi.c code uses an intel_dsi_msleep() helper for the various delays
>>>>> used for panel on/off, since starting with MIPI-sequences version >= 3 the
>>>>> delays are already included inside the MIPI-sequences.
>>>>>
>>>>> The problems exposed by the "Shut down displays gracefully on reboot"
>>>>> change, show that using this helper for the panel_pwr_cycle_delay is
>>>>> not the right thing to do. This has not been noticed until now because
>>>>> normally the panel never is cycled off and directly on again in quick
>>>>> succession.
>>>>>
>>>>> Change the msleep for the panel_pwr_cycle_delay to a normal msleep()
>>>>> call to avoid the panel staying black after a quick off + on cycle.
>>>>>
>>>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>>> Fixes: fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot")
>>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>>
>>>> Ping? Ville AFAICT this is ready for merging, can you review this please so that I can push it to drm-intel-next ?
>>>
>>> Didn't get the original mail, but lgtm.
>>
>> Yeah, these bounced I mentioned that in a p.s. in one of the emails
>> in our private threads about the mail issues, with patchwork links,
>> but I guess the p.s. was hidden in all the other stuff in that thread.
>> Anyways this is solved now.
>>
>>> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>
>> Thank you, note this is patch 1/2 does the Reviewed-by apply to
>> both?  Patch 2/2 is here:
>>
>> https://patchwork.freedesktop.org/patch/425983/
> 
> That one looks good as well.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Thank you.

I've pushed both to drm-intel-next now.

Regards,

Hans



>>>>> ---
>>>>>  drivers/gpu/drm/i915/display/vlv_dsi.c | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
>>>>> index d5a3f69c5df3..38d5a1f3ded5 100644
>>>>> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
>>>>> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
>>>>> @@ -996,14 +996,14 @@ static void intel_dsi_post_disable(struct intel_atomic_state *state,
>>>>>  	 * FIXME As we do with eDP, just make a note of the time here
>>>>>  	 * and perform the wait before the next panel power on.
>>>>>  	 */
>>>>> -	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
>>>>> +	msleep(intel_dsi->panel_pwr_cycle_delay);
>>>>>  }
>>>>>  
>>>>>  static void intel_dsi_shutdown(struct intel_encoder *encoder)
>>>>>  {
>>>>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>>>>  
>>>>> -	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
>>>>> +	msleep(intel_dsi->panel_pwr_cycle_delay);
>>>>>  }
>>>>>  
>>>>>  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>>>>>
>>>
>