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.
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
