On Thu, 3 Nov 2011 13:00:11 -0700, Jesse Barnes jbarnes@virtuousgeek.org wrote:
A few comments on this one (also, is it strictly required to fix your bug)?
I think so; the panel definitely was not happy when I turned the link off while the panel power was on. Having the mode setting and DPMS paths doing things in different orders definitely doesn't seem reasonable though. I nearly managed to share code between the two paths, but there are subtle differences in requirements and so I didn't bother.
Ok so you're making sure the panel has power to down the link, I think that's fine.
No, I'm turning the panel off *before* turning off the link. The panel goes nuts if you down the link before turning its power off; lots of technicolor.
Downing the link doesn't require any communication with the panel, so there's no issue with losing connectivity at this point:
ironlake_edp_backlight_off(intel_dp); ironlake_edp_panel_off(intel_dp); <= panel off
/* Wake up the sink first */ ironlake_edp_panel_vdd_on(intel_dp); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_link_down(intel_dp); <= link down ironlake_edp_panel_vdd_off(intel_dp, false);
But here it looks like you're shutting it off, then downing the link? Should this be reordered?
Nope, it's in the same order:
ironlake_edp_backlight_off(intel_dp); ironlake_edp_panel_off(intel_dp); <= panel off
ironlake_edp_panel_vdd_on(intel_dp); intel_dp_sink_dpms(intel_dp, mode); intel_dp_link_down(intel_dp); <= link down ironlake_edp_panel_vdd_off(intel_dp, false);
The only difference in these two code paths is that dp_prepare turns the sink to DPMS_ON, while dp_dpms turns the sink to DPMS_OFF.