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@xxxxxxxxxxxxxxx> >>>>> Fixes: fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot") >>>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >>>> >>>> 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@xxxxxxxxxxxxxxx> >> >> 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@xxxxxxxxxxxxxxx> 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, >>>>> >>> > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel