> -----Original Message-----
> From: Jani Nikula <jani.nikula@xxxxxxxxx>
> Sent: Friday, January 8, 2021 4:04 PM
> To: Gupta, Anshuman <anshuman.gupta@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 09/13] drm/i915/pps: rename
> intel_dp_check_edp to intel_pps_check_power_unlocked
>
> On Tue, 29 Dec 2020, Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> wrote:
> > On 2020-12-22 at 20:19:49 +0530, Jani Nikula wrote:
> >> Follow the usual naming pattern for functions.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> >> drivers/gpu/drm/i915/display/intel_pps.c | 2 +-
> >> drivers/gpu/drm/i915/display/intel_pps.h | 2 +-
> >> 3 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 334ba1775cd3..65406d4ccdbe 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -1046,7 +1046,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >> */
> >> cpu_latency_qos_update_request(&i915->pm_qos, 0);
> >>
> >> - intel_dp_check_edp(intel_dp);
> >> + intel_pps_check_power_unlocked(intel_dp);
> >>
> >> /* Try to wait for any previous AUX channel activity */
> >> for (try = 0; try < 3; try++) {
> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
> >> b/drivers/gpu/drm/i915/display/intel_pps.c
> >> index 3e62d1450682..dfd6722bc40e 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> >> @@ -431,7 +431,7 @@ static bool edp_have_panel_vdd(struct intel_dp
> *intel_dp)
> >> return intel_de_read(dev_priv, _pp_ctrl_reg(intel_dp)) &
> >> EDP_FORCE_VDD; }
> >>
> >> -void intel_dp_check_edp(struct intel_dp *intel_dp)
> >> +void intel_pps_check_power_unlocked(struct intel_dp *intel_dp)
> > IMHO comment to take pps_lock would be useful here.
>
> Part of the point of this change is to name it _unlocked to highlight it does
> not take the lock, i.e. you should be aware of locking. You see this pattern
> all over the kernel. It's self-documenting code.
>
> Moreover, after the edp check, the calls here have:
>
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> which both documents the requirement as well as ensures the proper usage
> in lockdep builds. I don't think a comment adds any value.
Agreeing with you on this.
Patch looks good to me.
Reviewed-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
>
> BR,
> Jani.
>
>
> > Thanks,
> > Anshuman.
> >> {
> >> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.h
> >> b/drivers/gpu/drm/i915/display/intel_pps.h
> >> index 4780b59a59df..8dda282abd42 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_pps.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.h
> >> @@ -22,7 +22,6 @@ intel_wakeref_t intel_pps_unlock(struct intel_dp
> *intel_dp, intel_wakeref_t wake
> >> #define with_intel_pps_lock(dp, wf)
> \
> >> for ((wf) = intel_pps_lock(dp); (wf); (wf) = intel_pps_unlock((dp),
> >> (wf)))
> >>
> >> -void intel_dp_check_edp(struct intel_dp *intel_dp); void
> >> intel_pps_backlight_on(struct intel_dp *intel_dp); void
> >> intel_pps_backlight_off(struct intel_dp *intel_dp); void
> >> intel_pps_backlight_power(struct intel_connector *connector, bool
> >> enable); @@ -31,6 +30,7 @@ bool intel_pps_vdd_on_unlocked(struct
> >> intel_dp *intel_dp); void intel_pps_vdd_off_unlocked(struct intel_dp
> >> *intel_dp, bool sync); void intel_pps_on_unlocked(struct intel_dp
> >> *intel_dp); void intel_pps_off_unlocked(struct intel_dp *intel_dp);
> >> +void intel_pps_check_power_unlocked(struct intel_dp *intel_dp);
> >>
> >> void intel_pps_vdd_on(struct intel_dp *intel_dp); void
> >> intel_pps_on(struct intel_dp *intel_dp);
> >> --
> >> 2.20.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
