[Intel-gfx] [PATCH v2 1/2] drm/i915/edp: Get the Panel Power Off timestamp after panel is off

[Manasi Navare]

On Thu, Oct 05, 2017 at 09:54:19AM +0300, Jani Nikula wrote:
> On Wed, 04 Oct 2017, Manasi Navare <manasi.d.navare at 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 at ffwll.ch>
> > Cc: Jani Nikula <jani.nikula at 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 at ffwll.ch>
> > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > Reviewed-by:  Daniel Vetter <daniel.vetter at ffwll.ch>
> > Reviewed-by: Jani Nikula <jani.nikula at 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 at 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