On Fri, 31 May 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> wrote:
> On 5/21/19 1:45 AM, Jani Nikula wrote:
>> On Thu, 16 May 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> wrote:
>>> As a first step towards updating the code to work on the runtime_pm
>>> structure instead of i915, rework all the internals to use and pass
>>> around that.
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/i915/i915_drv.h | 2 +
>>> drivers/gpu/drm/i915/intel_drv.h | 10 +-
>>> drivers/gpu/drm/i915/intel_runtime_pm.c | 152 ++++++++++++------------
>>> 3 files changed, 82 insertions(+), 82 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 5801f5407589..474074c7f395 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1177,6 +1177,8 @@ struct skl_wm_params {
>>> */
>>> struct i915_runtime_pm {
>>> atomic_t wakeref_count;
>>> + struct device *kdev;
>>
>> This could use a small comment to say what device this is.
>>
>
> Would something like:
>
> /* the intel gpu we're loaded on */
I meant more literally something like "set to
i915->drm->pdev->dev". (With . instead of -> in some places...)
BR,
Jani.
> work? Or should I just rename it to i915_kdev like we use in other parts
> of the driver?
>
> Thanks,
> Daniele
>
>> BR,
>> Jani.
>>
>>> + bool available;
>>> bool suspended;
>>> bool irqs_enabled;
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 30b2d6ed2d53..bd04f394fbd3 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1662,13 +1662,17 @@ ____assert_rpm_wakelock_held(struct i915_runtime_pm *rpm, int wakeref_count)
>>> }
>>>
>>> static inline void
>>> -assert_rpm_raw_wakeref_held(struct drm_i915_private *i915)
>>> +__assert_rpm_raw_wakeref_held(struct i915_runtime_pm *rpm)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> -
>>> ____assert_rpm_raw_wakeref_held(rpm, atomic_read(&rpm->wakeref_count));
>>> }
>>>
>>> +static inline void
>>> +assert_rpm_raw_wakeref_held(struct drm_i915_private *i915)
>>> +{
>>> + __assert_rpm_raw_wakeref_held(&i915->runtime_pm);
>>> +}
>>> +
>>> static inline void
>>> __assert_rpm_wakelock_held(struct i915_runtime_pm *rpm)
>>> {
>>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> index b4abababdf6c..2e21f562df44 100644
>>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> @@ -60,19 +60,19 @@
>>> * present for a given platform.
>>> */
>>>
>>> -static intel_wakeref_t intel_runtime_pm_get_raw(struct drm_i915_private *i915);
>>> +static intel_wakeref_t intel_runtime_pm_get_raw(struct i915_runtime_pm *rpm);
>>> static void
>>> -__intel_runtime_pm_put(struct drm_i915_private *i915, intel_wakeref_t wref,
>>> +__intel_runtime_pm_put(struct i915_runtime_pm *rpm, intel_wakeref_t wref,
>>> bool wakelock);
>>>
>>> #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>>> static void
>>> -intel_runtime_pm_put_raw(struct drm_i915_private *i915, intel_wakeref_t wref);
>>> +intel_runtime_pm_put_raw(struct i915_runtime_pm *rpm, intel_wakeref_t wref);
>>> #else
>>> -static inline void intel_runtime_pm_put_raw(struct drm_i915_private *i915,
>>> +static inline void intel_runtime_pm_put_raw(struct i915_runtime_pm *rpm,
>>> intel_wakeref_t wref)
>>> {
>>> - __intel_runtime_pm_put(i915, -1, false);
>>> + __intel_runtime_pm_put(rpm, -1, false);
>>> }
>>> #endif
>>>
>>> @@ -112,21 +112,18 @@ static void __print_depot_stack(depot_stack_handle_t stack,
>>> snprint_stack_trace(buf, sz, &trace, indent);
>>> }
>>>
>>> -static void init_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
>>> +static void init_intel_runtime_pm_wakeref(struct i915_runtime_pm *rpm)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> -
>>> spin_lock_init(&rpm->debug.lock);
>>> }
>>>
>>> static noinline depot_stack_handle_t
>>> -track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
>>> +track_intel_runtime_pm_wakeref(struct i915_runtime_pm *rpm)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> depot_stack_handle_t stack, *stacks;
>>> unsigned long flags;
>>>
>>> - if (!HAS_RUNTIME_PM(i915))
>>> + if (!rpm->available)
>>> return -1;
>>>
>>> stack = __save_depot_stack();
>>> @@ -153,10 +150,9 @@ track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
>>> return stack;
>>> }
>>>
>>> -static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
>>> +static void untrack_intel_runtime_pm_wakeref(struct i915_runtime_pm *rpm,
>>> depot_stack_handle_t stack)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> unsigned long flags, n;
>>> bool found = false;
>>>
>>> @@ -274,9 +270,8 @@ dump_and_free_wakeref_tracking(struct intel_runtime_pm_debug *debug)
>>> }
>>>
>>> static noinline void
>>> -__intel_wakeref_dec_and_check_tracking(struct drm_i915_private *i915)
>>> +__intel_wakeref_dec_and_check_tracking(struct i915_runtime_pm *rpm)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> struct intel_runtime_pm_debug dbg = {};
>>> unsigned long flags;
>>>
>>> @@ -292,9 +287,8 @@ __intel_wakeref_dec_and_check_tracking(struct drm_i915_private *i915)
>>> }
>>>
>>> static noinline void
>>> -untrack_all_intel_runtime_pm_wakerefs(struct drm_i915_private *i915)
>>> +untrack_all_intel_runtime_pm_wakerefs(struct i915_runtime_pm *rpm)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> struct intel_runtime_pm_debug dbg = {};
>>> unsigned long flags;
>>>
>>> @@ -345,61 +339,57 @@ void print_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
>>>
>>> #else
>>>
>>> -static void init_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
>>> +static void init_intel_runtime_pm_wakeref(struct i915_runtime_pm *rpm)
>>> {
>>> }
>>>
>>> static depot_stack_handle_t
>>> -track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
>>> +track_intel_runtime_pm_wakeref(struct i915_runtime_pm *rpm)
>>> {
>>> return -1;
>>> }
>>>
>>> -static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
>>> +static void untrack_intel_runtime_pm_wakeref(struct i915_runtime_pm *rpm,
>>> intel_wakeref_t wref)
>>> {
>>> }
>>>
>>> static void
>>> -__intel_wakeref_dec_and_check_tracking(struct drm_i915_private *i915)
>>> +__intel_wakeref_dec_and_check_tracking(struct i915_runtime_pm *rpm)
>>> {
>>> - atomic_dec(&i915->runtime_pm.wakeref_count);
>>> + atomic_dec(&rpm->wakeref_count);
>>> }
>>>
>>> static void
>>> -untrack_all_intel_runtime_pm_wakerefs(struct drm_i915_private *i915)
>>> +untrack_all_intel_runtime_pm_wakerefs(struct i915_runtime_pm *rpm)
>>> {
>>> }
>>>
>>> #endif
>>>
>>> static void
>>> -intel_runtime_pm_acquire(struct drm_i915_private *i915, bool wakelock)
>>> +intel_runtime_pm_acquire(struct i915_runtime_pm *rpm, bool wakelock)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> -
>>> if (wakelock) {
>>> atomic_add(1 + INTEL_RPM_WAKELOCK_BIAS, &rpm->wakeref_count);
>>> - assert_rpm_wakelock_held(i915);
>>> + __assert_rpm_wakelock_held(rpm);
>>> } else {
>>> atomic_inc(&rpm->wakeref_count);
>>> - assert_rpm_raw_wakeref_held(i915);
>>> + __assert_rpm_raw_wakeref_held(rpm);
>>> }
>>> }
>>>
>>> static void
>>> -intel_runtime_pm_release(struct drm_i915_private *i915, int wakelock)
>>> +intel_runtime_pm_release(struct i915_runtime_pm *rpm, int wakelock)
>>> {
>>> - struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> -
>>> if (wakelock) {
>>> - assert_rpm_wakelock_held(i915);
>>> + __assert_rpm_wakelock_held(rpm);
>>> atomic_sub(INTEL_RPM_WAKELOCK_BIAS, &rpm->wakeref_count);
>>> } else {
>>> - assert_rpm_raw_wakeref_held(i915);
>>> + __assert_rpm_raw_wakeref_held(rpm);
>>> }
>>>
>>> - __intel_wakeref_dec_and_check_tracking(i915);
>>> + __intel_wakeref_dec_and_check_tracking(rpm);
>>> }
>>>
>>> bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
>>> @@ -2030,7 +2020,7 @@ intel_display_power_grab_async_put_ref(struct drm_i915_private *dev_priv,
>>> goto out_verify;
>>>
>>> cancel_delayed_work(&power_domains->async_put_work);
>>> - intel_runtime_pm_put_raw(dev_priv,
>>> + intel_runtime_pm_put_raw(&dev_priv->runtime_pm,
>>> fetch_and_zero(&power_domains->async_put_wakeref));
>>> out_verify:
>>> verify_async_put_domains_state(power_domains);
>>> @@ -2219,7 +2209,8 @@ intel_display_power_put_async_work(struct work_struct *work)
>>> container_of(work, struct drm_i915_private,
>>> power_domains.async_put_work.work);
>>> struct i915_power_domains *power_domains = &dev_priv->power_domains;
>>> - intel_wakeref_t new_work_wakeref = intel_runtime_pm_get_raw(dev_priv);
>>> + struct i915_runtime_pm *rpm = &dev_priv->runtime_pm;
>>> + intel_wakeref_t new_work_wakeref = intel_runtime_pm_get_raw(rpm);
>>> intel_wakeref_t old_work_wakeref = 0;
>>>
>>> mutex_lock(&power_domains->lock);
>>> @@ -2249,9 +2240,9 @@ intel_display_power_put_async_work(struct work_struct *work)
>>> mutex_unlock(&power_domains->lock);
>>>
>>> if (old_work_wakeref)
>>> - intel_runtime_pm_put_raw(dev_priv, old_work_wakeref);
>>> + intel_runtime_pm_put_raw(rpm, old_work_wakeref);
>>> if (new_work_wakeref)
>>> - intel_runtime_pm_put_raw(dev_priv, new_work_wakeref);
>>> + intel_runtime_pm_put_raw(rpm, new_work_wakeref);
>>> }
>>>
>>> /**
>>> @@ -2269,7 +2260,8 @@ void __intel_display_power_put_async(struct drm_i915_private *i915,
>>> intel_wakeref_t wakeref)
>>> {
>>> struct i915_power_domains *power_domains = &i915->power_domains;
>>> - intel_wakeref_t work_wakeref = intel_runtime_pm_get_raw(i915);
>>> + struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> + intel_wakeref_t work_wakeref = intel_runtime_pm_get_raw(rpm);
>>>
>>> mutex_lock(&power_domains->lock);
>>>
>>> @@ -2296,7 +2288,7 @@ void __intel_display_power_put_async(struct drm_i915_private *i915,
>>> mutex_unlock(&power_domains->lock);
>>>
>>> if (work_wakeref)
>>> - intel_runtime_pm_put_raw(i915, work_wakeref);
>>> + intel_runtime_pm_put_raw(rpm, work_wakeref);
>>>
>>> intel_runtime_pm_put(i915, wakeref);
>>> }
>>> @@ -2334,7 +2326,7 @@ void intel_display_power_flush_work(struct drm_i915_private *i915)
>>> mutex_unlock(&power_domains->lock);
>>>
>>> if (work_wakeref)
>>> - intel_runtime_pm_put_raw(i915, work_wakeref);
>>> + intel_runtime_pm_put_raw(&i915->runtime_pm, work_wakeref);
>>> }
>>>
>>> /**
>>> @@ -4996,24 +4988,22 @@ static void intel_power_domains_verify_state(struct drm_i915_private *i915)
>>>
>>> #endif
>>>
>>> -static intel_wakeref_t __intel_runtime_pm_get(struct drm_i915_private *i915,
>>> +static intel_wakeref_t __intel_runtime_pm_get(struct i915_runtime_pm *rpm,
>>> bool wakelock)
>>> {
>>> - struct pci_dev *pdev = i915->drm.pdev;
>>> - struct device *kdev = &pdev->dev;
>>> int ret;
>>>
>>> - ret = pm_runtime_get_sync(kdev);
>>> + ret = pm_runtime_get_sync(rpm->kdev);
>>> WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
>>>
>>> - intel_runtime_pm_acquire(i915, wakelock);
>>> + intel_runtime_pm_acquire(rpm, wakelock);
>>>
>>> - return track_intel_runtime_pm_wakeref(i915);
>>> + return track_intel_runtime_pm_wakeref(rpm);
>>> }
>>>
>>> -static intel_wakeref_t intel_runtime_pm_get_raw(struct drm_i915_private *i915)
>>> +static intel_wakeref_t intel_runtime_pm_get_raw(struct i915_runtime_pm *rpm)
>>> {
>>> - return __intel_runtime_pm_get(i915, false);
>>> + return __intel_runtime_pm_get(rpm, false);
>>> }
>>>
>>> /**
>>> @@ -5030,7 +5020,7 @@ static intel_wakeref_t intel_runtime_pm_get_raw(struct drm_i915_private *i915)
>>> */
>>> intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915)
>>> {
>>> - return __intel_runtime_pm_get(i915, true);
>>> + return __intel_runtime_pm_get(&i915->runtime_pm, true);
>>> }
>>>
>>> /**
>>> @@ -5049,23 +5039,22 @@ intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915)
>>> */
>>> intel_wakeref_t intel_runtime_pm_get_if_in_use(struct drm_i915_private *i915)
>>> {
>>> - if (IS_ENABLED(CONFIG_PM)) {
>>> - struct pci_dev *pdev = i915->drm.pdev;
>>> - struct device *kdev = &pdev->dev;
>>> + struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>>
>>> + if (IS_ENABLED(CONFIG_PM)) {
>>> /*
>>> * In cases runtime PM is disabled by the RPM core and we get
>>> * an -EINVAL return value we are not supposed to call this
>>> * function, since the power state is undefined. This applies
>>> * atm to the late/early system suspend/resume handlers.
>>> */
>>> - if (pm_runtime_get_if_in_use(kdev) <= 0)
>>> + if (pm_runtime_get_if_in_use(rpm->kdev) <= 0)
>>> return 0;
>>> }
>>>
>>> - intel_runtime_pm_acquire(i915, true);
>>> + intel_runtime_pm_acquire(rpm, true);
>>>
>>> - return track_intel_runtime_pm_wakeref(i915);
>>> + return track_intel_runtime_pm_wakeref(rpm);
>>> }
>>>
>>> /**
>>> @@ -5089,27 +5078,25 @@ intel_wakeref_t intel_runtime_pm_get_if_in_use(struct drm_i915_private *i915)
>>> */
>>> intel_wakeref_t intel_runtime_pm_get_noresume(struct drm_i915_private *i915)
>>> {
>>> - struct pci_dev *pdev = i915->drm.pdev;
>>> - struct device *kdev = &pdev->dev;
>>> + struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>>
>>> - assert_rpm_wakelock_held(i915);
>>> - pm_runtime_get_noresume(kdev);
>>> + __assert_rpm_wakelock_held(rpm);
>>> + pm_runtime_get_noresume(rpm->kdev);
>>>
>>> - intel_runtime_pm_acquire(i915, true);
>>> + intel_runtime_pm_acquire(rpm, true);
>>>
>>> - return track_intel_runtime_pm_wakeref(i915);
>>> + return track_intel_runtime_pm_wakeref(rpm);
>>> }
>>>
>>> -static void __intel_runtime_pm_put(struct drm_i915_private *i915,
>>> +static void __intel_runtime_pm_put(struct i915_runtime_pm *rpm,
>>> intel_wakeref_t wref,
>>> bool wakelock)
>>> {
>>> - struct pci_dev *pdev = i915->drm.pdev;
>>> - struct device *kdev = &pdev->dev;
>>> + struct device *kdev = rpm->kdev;
>>>
>>> - untrack_intel_runtime_pm_wakeref(i915, wref);
>>> + untrack_intel_runtime_pm_wakeref(rpm, wref);
>>>
>>> - intel_runtime_pm_release(i915, wakelock);
>>> + intel_runtime_pm_release(rpm, wakelock);
>>>
>>> pm_runtime_mark_last_busy(kdev);
>>> pm_runtime_put_autosuspend(kdev);
>>> @@ -5117,9 +5104,9 @@ static void __intel_runtime_pm_put(struct drm_i915_private *i915,
>>>
>>> #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>>> static void
>>> -intel_runtime_pm_put_raw(struct drm_i915_private *i915, intel_wakeref_t wref)
>>> +intel_runtime_pm_put_raw(struct i915_runtime_pm *rpm, intel_wakeref_t wref)
>>> {
>>> - __intel_runtime_pm_put(i915, wref, false);
>>> + __intel_runtime_pm_put(rpm, wref, false);
>>> }
>>> #endif
>>>
>>> @@ -5137,7 +5124,7 @@ intel_runtime_pm_put_raw(struct drm_i915_private *i915, intel_wakeref_t wref)
>>> */
>>> void intel_runtime_pm_put_unchecked(struct drm_i915_private *i915)
>>> {
>>> - __intel_runtime_pm_put(i915, -1, true);
>>> + __intel_runtime_pm_put(&i915->runtime_pm, -1, true);
>>> }
>>>
>>> #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>>> @@ -5152,7 +5139,7 @@ void intel_runtime_pm_put_unchecked(struct drm_i915_private *i915)
>>> */
>>> void intel_runtime_pm_put(struct drm_i915_private *i915, intel_wakeref_t wref)
>>> {
>>> - __intel_runtime_pm_put(i915, wref, true);
>>> + __intel_runtime_pm_put(&i915->runtime_pm, wref, true);
>>> }
>>> #endif
>>>
>>> @@ -5168,8 +5155,8 @@ void intel_runtime_pm_put(struct drm_i915_private *i915, intel_wakeref_t wref)
>>> */
>>> void intel_runtime_pm_enable(struct drm_i915_private *i915)
>>> {
>>> - struct pci_dev *pdev = i915->drm.pdev;
>>> - struct device *kdev = &pdev->dev;
>>> + struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> + struct device *kdev = rpm->kdev;
>>>
>>> /*
>>> * Disable the system suspend direct complete optimization, which can
>>> @@ -5190,7 +5177,7 @@ void intel_runtime_pm_enable(struct drm_i915_private *i915)
>>> * so the driver's own RPM reference tracking asserts also work on
>>> * platforms without RPM support.
>>> */
>>> - if (!HAS_RUNTIME_PM(i915)) {
>>> + if (!rpm->available) {
>>> int ret;
>>>
>>> pm_runtime_dont_use_autosuspend(kdev);
>>> @@ -5210,8 +5197,8 @@ void intel_runtime_pm_enable(struct drm_i915_private *i915)
>>>
>>> void intel_runtime_pm_disable(struct drm_i915_private *i915)
>>> {
>>> - struct pci_dev *pdev = i915->drm.pdev;
>>> - struct device *kdev = &pdev->dev;
>>> + struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> + struct device *kdev = rpm->kdev;
>>>
>>> /* Transfer rpm ownership back to core */
>>> WARN(pm_runtime_get_sync(kdev) < 0,
>>> @@ -5219,7 +5206,7 @@ void intel_runtime_pm_disable(struct drm_i915_private *i915)
>>>
>>> pm_runtime_dont_use_autosuspend(kdev);
>>>
>>> - if (!HAS_RUNTIME_PM(i915))
>>> + if (!rpm->available)
>>> pm_runtime_put(kdev);
>>> }
>>>
>>> @@ -5233,10 +5220,17 @@ void intel_runtime_pm_cleanup(struct drm_i915_private *i915)
>>> intel_rpm_raw_wakeref_count(count),
>>> intel_rpm_wakelock_count(count));
>>>
>>> - untrack_all_intel_runtime_pm_wakerefs(i915);
>>> + untrack_all_intel_runtime_pm_wakerefs(rpm);
>>> }
>>>
>>> void intel_runtime_pm_init_early(struct drm_i915_private *i915)
>>> {
>>> - init_intel_runtime_pm_wakeref(i915);
>>> + struct i915_runtime_pm *rpm = &i915->runtime_pm;
>>> + struct pci_dev *pdev = i915->drm.pdev;
>>> + struct device *kdev = &pdev->dev;
>>> +
>>> + rpm->kdev = kdev;
>>> + rpm->available = HAS_RUNTIME_PM(i915);
>>> +
>>> + init_intel_runtime_pm_wakeref(rpm);
>>> }
>>
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
