| Message ID | 1507135706-17147-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 04 Oct 2017, Manasi Navare <manasi.d.navare@intel.com> wrote: > Kernel stores the time in jiffies at which the eDP panel is turned > off. This should be obtained after the panel is off (after the > wait_panel_off). When we next attempt to turn the panel on, we > use the difference between the timestamp at which we want to turn the > panel on and timestamp at which panel was turned off to ensure that this > is equal to panel power cycle delay and if not we wait for the remaining > time. Not waiting for the panel power cycle delay can cause the panel to not > turn on giving rise to AUX timeouts for the attempted AUX transactions. > > v2: > * Separate lines for bugzilla (Jani Nikula) > * Suggested by tag (Daniel Vetter) > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101518 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101144 > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com> Pushed both to dinq, with cc: stable on the first. Thanks for the patches and the debugging efforts in particular! --- Please do try to make it a habit to run checkpatch on your patches before sending. We don't care about all of the more subjective warnings, but then at least you make the conscious decision to ignore them. I fixed these while applying: -:13: WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #13: time. Not waiting for the panel power cycle delay can cause the panel to not -:26: WARNING: Use a single space after Reviewed-by: #26: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> I use these to run checkpatch on the commits in my local tree before sending: alias checkpatch='checkpatch.pl -q --emacs --strict' checkbranch() { local commit local range if [ -z "$1" ]; then range="HEAD^..HEAD" elif [ -n "`echo $1 | grep '\.\.'`" ]; then range="$1" else range="$1..HEAD" fi for commit in `git rev-list --reverse $range`; do git --no-pager log --oneline -1 $commit git format-patch --stdout -1 $commit | checkpatch - done } and use like: $ checkbranch $tip where tip=drm-tip/drm-tip I also have similar stuff to run sparse on every single series I send out. Overall it's just so much more efficient to catch the trivial stuff early rather than late. I also consider it a courtesy to my fellow developers. BR, Jani. > --- > drivers/gpu/drm/i915/intel_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 90e756c..0fd41cd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2308,8 +2308,8 @@ static void edp_panel_off(struct intel_dp *intel_dp) > I915_WRITE(pp_ctrl_reg, pp); > POSTING_READ(pp_ctrl_reg); > > - intel_dp->panel_power_off_time = ktime_get_boottime(); > wait_panel_off(intel_dp); > + intel_dp->panel_power_off_time = ktime_get_boottime(); > > /* We got a reference when we enabled the VDD. */ > intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
On Thu, Oct 05, 2017 at 09:54:19AM +0300, Jani Nikula wrote: > On Wed, 04 Oct 2017, Manasi Navare <manasi.d.navare@intel.com> wrote: > > Kernel stores the time in jiffies at which the eDP panel is turned > > off. This should be obtained after the panel is off (after the > > wait_panel_off). When we next attempt to turn the panel on, we > > use the difference between the timestamp at which we want to turn the > > panel on and timestamp at which panel was turned off to ensure that this > > is equal to panel power cycle delay and if not we wait for the remaining > > time. Not waiting for the panel power cycle delay can cause the panel to not > > turn on giving rise to AUX timeouts for the attempted AUX transactions. > > > > v2: > > * Separate lines for bugzilla (Jani Nikula) > > * Suggested by tag (Daniel Vetter) > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101518 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101144 > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com> > > Pushed both to dinq, with cc: stable on the first. Thanks for the > patches and the debugging efforts in particular! > Thanks for merging the patch. Yes it was a pretty interesting debugging task. Hopefully it has fixed those bugs for good. > --- > > Please do try to make it a habit to run checkpatch on your patches > before sending. We don't care about all of the more subjective warnings, > but then at least you make the conscious decision to ignore them. > > I fixed these while applying: > > -:13: WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) > #13: > time. Not waiting for the panel power cycle delay can cause the panel to not > > -:26: WARNING: Use a single space after Reviewed-by: > #26: > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > I use these to run checkpatch on the commits in my local tree before > sending: > > alias checkpatch='checkpatch.pl -q --emacs --strict' > > checkbranch() > { > local commit > local range > > if [ -z "$1" ]; then > range="HEAD^..HEAD" > elif [ -n "`echo $1 | grep '\.\.'`" ]; then > range="$1" > else > range="$1..HEAD" > fi > > for commit in `git rev-list --reverse $range`; do > git --no-pager log --oneline -1 $commit > git format-patch --stdout -1 $commit | checkpatch - > done > } > > and use like: > > $ checkbranch $tip > > where tip=drm-tip/drm-tip > Thanks a lot for these pointers on checkpatch, so this gets added to bashrc? I will experiment with it and for sure seems like an efficient way of running it on every patch isntead of doing it manually. Regards Manasi > I also have similar stuff to run sparse on every single series I send > out. Overall it's just so much more efficient to catch the trivial stuff > early rather than late. I also consider it a courtesy to my fellow > developers. > > > BR, > Jani. > > > > --- > > drivers/gpu/drm/i915/intel_dp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 90e756c..0fd41cd 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2308,8 +2308,8 @@ static void edp_panel_off(struct intel_dp *intel_dp) > > I915_WRITE(pp_ctrl_reg, pp); > > POSTING_READ(pp_ctrl_reg); > > > > - intel_dp->panel_power_off_time = ktime_get_boottime(); > > wait_panel_off(intel_dp); > > + intel_dp->panel_power_off_time = ktime_get_boottime(); > > > > /* We got a reference when we enabled the VDD. */ > > intel_display_power_put(dev_priv, intel_dp->aux_power_domain); > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 90e756c..0fd41cd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2308,8 +2308,8 @@ static void edp_panel_off(struct intel_dp *intel_dp) I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); - intel_dp->panel_power_off_time = ktime_get_boottime(); wait_panel_off(intel_dp); + intel_dp->panel_power_off_time = ktime_get_boottime(); /* We got a reference when we enabled the VDD. */ intel_display_power_put(dev_priv, intel_dp->aux_power_domain);