[Intel-xe] [PATCH] drm/i915/rps: split out display rps parts to a separate file
Lucas De Marchi
lucas.demarchi at intel.com
Tue Mar 21 23:04:48 UTC 2023
On Tue, Mar 21, 2023 at 06:08:39PM +0200, Jani Nikula wrote:
>On Tue, 21 Mar 2023, Jani Nikula <jani.nikula at intel.com> wrote:
>> On Tue, 21 Mar 2023, Jani Nikula <jani.nikula at intel.com> wrote:
>>> Split out the RPS parts so they can be conditionally compiled out later.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> Link: https://patchwork.freedesktop.org/patch/msgid/20230302164936.3034161-1-jani.nikula@intel.com
>>> (cherry picked from commit 6dbbff25b39565c801c87379bc85933fb436518e)
>>>
>>> ---
>>>
>>> Here's an example of an upstream i915 commit that can help clean up
>>> cherry-pick as well as cleanup of the #ifdefs in one go, otherwise it
>>> breaks bisect.
>>
>> *sigh* I had something more here, but having #ifdef first on a line
>> dropped it as comments, and turned it into nonsense.
>>
>> It's a backport of upstream i915 commit to help clean up #ifdefs, but
>> how do we want to handle them?
>
>Moreover, even if we're stalling with adding such #ifdefs to upstream
>i915 now, we can however clean up existing usage in the xe branch to be
>more suitable for upstream. For example, I'd take the #ifdefs here
>pretty much without issues.
instead of a cherry-pick + squashed-ifdef-removal I think it's better
to do:
1) cherry-pick
2) remove ifdefs and commit with
git commit -s --fixup=38a139546384
... or whatever commit added them (there are a few before that commit
above)
The reason is that I don't think we will be able to avoid 1 more rebase
before being merged upstream. We can't do merge-approach neither
because of these commits we don't really want to ever show up in
upstream (and because we are on top of drm-tip rather than e.g.
drm-next). So doing the above at least makes a rebase more bearable:
Whenever we do a rebase (with --autosquash) the cherry-pick should
automatically be dropped by git and the fixup should automatically remove
the addition of the ifdefs in the commit that added them.
Lucas De Marchi
>
>BR,
>Jani.
>
>
>>
>>
>>> ---
>>> drivers/gpu/drm/i915/Makefile | 1 +
>>> .../gpu/drm/i915/display/intel_atomic_plane.c | 85 ++-----------------
>>> .../gpu/drm/i915/display/intel_display_rps.c | 81 ++++++++++++++++++
>>> .../gpu/drm/i915/display/intel_display_rps.h | 34 ++++++++
>>> 4 files changed, 123 insertions(+), 78 deletions(-)
>>> create mode 100644 drivers/gpu/drm/i915/display/intel_display_rps.c
>>> create mode 100644 drivers/gpu/drm/i915/display/intel_display_rps.h
>>>
>>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>>> index ae9259ab3261..f689132e1506 100644
>>> --- a/drivers/gpu/drm/i915/Makefile
>>> +++ b/drivers/gpu/drm/i915/Makefile
>>> @@ -239,6 +239,7 @@ i915-y += \
>>> display/intel_display_power.o \
>>> display/intel_display_power_map.o \
>>> display/intel_display_power_well.o \
>>> + display/intel_display_rps.o \
>>> display/intel_dmc.o \
>>> display/intel_dpio_phy.o \
>>> display/intel_dpll.o \
>>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>> index 3d37f44ec835..e666a5c1fd91 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>> @@ -34,13 +34,10 @@
>>> #include <drm/drm_atomic_helper.h>
>>> #include <drm/drm_fourcc.h>
>>>
>>> -#ifdef I915
>>> -#include "gt/intel_rps.h"
>>> -#endif
>>> -
>>> #include "i915_config.h"
>>> #include "intel_atomic_plane.h"
>>> #include "intel_cdclk.h"
>>> +#include "intel_display_rps.h"
>>> #include "intel_display_trace.h"
>>> #include "intel_display_types.h"
>>> #include "intel_fb.h"
>>> @@ -951,66 +948,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
>>> return 0;
>>> }
>>>
>>> -#ifdef I915
>>> -struct wait_rps_boost {
>>> - struct wait_queue_entry wait;
>>> -
>>> - struct drm_crtc *crtc;
>>> - struct i915_request *request;
>>> -};
>>> -
>>> -static int do_rps_boost(struct wait_queue_entry *_wait,
>>> - unsigned mode, int sync, void *key)
>>> -{
>>> - struct wait_rps_boost *wait = container_of(_wait, typeof(*wait), wait);
>>> - struct i915_request *rq = wait->request;
>>> -
>>> - /*
>>> - * If we missed the vblank, but the request is already running it
>>> - * is reasonable to assume that it will complete before the next
>>> - * vblank without our intervention, so leave RPS alone.
>>> - */
>>> - if (!i915_request_started(rq))
>>> - intel_rps_boost(rq);
>>> - i915_request_put(rq);
>>> -
>>> - drm_crtc_vblank_put(wait->crtc);
>>> -
>>> - list_del(&wait->wait.entry);
>>> - kfree(wait);
>>> - return 1;
>>> -}
>>> -
>>> -static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
>>> - struct dma_fence *fence)
>>> -{
>>> - struct wait_rps_boost *wait;
>>> -
>>> - if (!dma_fence_is_i915(fence))
>>> - return;
>>> -
>>> - if (DISPLAY_VER(to_i915(crtc->dev)) < 6)
>>> - return;
>>> -
>>> - if (drm_crtc_vblank_get(crtc))
>>> - return;
>>> -
>>> - wait = kmalloc(sizeof(*wait), GFP_KERNEL);
>>> - if (!wait) {
>>> - drm_crtc_vblank_put(crtc);
>>> - return;
>>> - }
>>> -
>>> - wait->request = to_request(dma_fence_get(fence));
>>> - wait->crtc = crtc;
>>> -
>>> - wait->wait.func = do_rps_boost;
>>> - wait->wait.flags = 0;
>>> -
>>> - add_wait_queue(drm_crtc_vblank_waitqueue(crtc), &wait->wait);
>>> -}
>>> -#endif
>>> -
>>> /**
>>> * intel_prepare_plane_fb - Prepare fb for usage on plane
>>> * @_plane: drm plane to prepare for
>>> @@ -1102,13 +1039,13 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
>>> dma_resv_iter_begin(&cursor, obj->base.resv,
>>> DMA_RESV_USAGE_WRITE);
>>> dma_resv_for_each_fence_unlocked(&cursor, fence) {
>>> - add_rps_boost_after_vblank(new_plane_state->hw.crtc,
>>> - fence);
>>> + intel_display_rps_boost_after_vblank(new_plane_state->hw.crtc,
>>> + fence);
>>> }
>>> dma_resv_iter_end(&cursor);
>>> } else {
>>> - add_rps_boost_after_vblank(new_plane_state->hw.crtc,
>>> - new_plane_state->uapi.fence);
>>> + intel_display_rps_boost_after_vblank(new_plane_state->hw.crtc,
>>> + new_plane_state->uapi.fence);
>>> }
>>>
>>> /*
>>> @@ -1119,10 +1056,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
>>> * that are not quite steady state without resorting to forcing
>>> * maximum clocks following a vblank miss (see do_rps_boost()).
>>> */
>>> - if (!state->rps_interactive) {
>>> - intel_rps_mark_interactive(&to_gt(dev_priv)->rps, true);
>>> - state->rps_interactive = true;
>>> - }
>>> + intel_display_rps_mark_interactive(dev_priv, state, true);
>>>
>>> return 0;
>>>
>>> @@ -1159,12 +1093,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>>> if (!obj)
>>> return;
>>>
>>> -#ifdef I915
>>> - if (state->rps_interactive) {
>>> - intel_rps_mark_interactive(&to_gt(dev_priv)->rps, false);
>>> - state->rps_interactive = false;
>>> - }
>>> -#endif
>>> + intel_display_rps_mark_interactive(dev_priv, state, false);
>>>
>>> /* Should only be called after a successful intel_prepare_plane_fb()! */
>>> intel_plane_unpin_fb(old_plane_state);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_rps.c b/drivers/gpu/drm/i915/display/intel_display_rps.c
>>> new file mode 100644
>>> index 000000000000..918d0327169a
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_rps.c
>>> @@ -0,0 +1,81 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + */
>>> +
>>> +#include <drm/drm_crtc.h>
>>> +#include <drm/drm_vblank.h>
>>> +
>>> +#include "gt/intel_rps.h"
>>> +#include "i915_drv.h"
>>> +#include "intel_display_rps.h"
>>> +#include "intel_display_types.h"
>>> +
>>> +struct wait_rps_boost {
>>> + struct wait_queue_entry wait;
>>> +
>>> + struct drm_crtc *crtc;
>>> + struct i915_request *request;
>>> +};
>>> +
>>> +static int do_rps_boost(struct wait_queue_entry *_wait,
>>> + unsigned mode, int sync, void *key)
>>> +{
>>> + struct wait_rps_boost *wait = container_of(_wait, typeof(*wait), wait);
>>> + struct i915_request *rq = wait->request;
>>> +
>>> + /*
>>> + * If we missed the vblank, but the request is already running it
>>> + * is reasonable to assume that it will complete before the next
>>> + * vblank without our intervention, so leave RPS alone.
>>> + */
>>> + if (!i915_request_started(rq))
>>> + intel_rps_boost(rq);
>>> + i915_request_put(rq);
>>> +
>>> + drm_crtc_vblank_put(wait->crtc);
>>> +
>>> + list_del(&wait->wait.entry);
>>> + kfree(wait);
>>> + return 1;
>>> +}
>>> +
>>> +void intel_display_rps_boost_after_vblank(struct drm_crtc *crtc,
>>> + struct dma_fence *fence)
>>> +{
>>> + struct wait_rps_boost *wait;
>>> +
>>> + if (!dma_fence_is_i915(fence))
>>> + return;
>>> +
>>> + if (DISPLAY_VER(to_i915(crtc->dev)) < 6)
>>> + return;
>>> +
>>> + if (drm_crtc_vblank_get(crtc))
>>> + return;
>>> +
>>> + wait = kmalloc(sizeof(*wait), GFP_KERNEL);
>>> + if (!wait) {
>>> + drm_crtc_vblank_put(crtc);
>>> + return;
>>> + }
>>> +
>>> + wait->request = to_request(dma_fence_get(fence));
>>> + wait->crtc = crtc;
>>> +
>>> + wait->wait.func = do_rps_boost;
>>> + wait->wait.flags = 0;
>>> +
>>> + add_wait_queue(drm_crtc_vblank_waitqueue(crtc), &wait->wait);
>>> +}
>>> +
>>> +void intel_display_rps_mark_interactive(struct drm_i915_private *i915,
>>> + struct intel_atomic_state *state,
>>> + bool interactive)
>>> +{
>>> + if (state->rps_interactive == interactive)
>>> + return;
>>> +
>>> + intel_rps_mark_interactive(&to_gt(i915)->rps, interactive);
>>> + state->rps_interactive = interactive;
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_rps.h b/drivers/gpu/drm/i915/display/intel_display_rps.h
>>> new file mode 100644
>>> index 000000000000..2cfb8a303b77
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_rps.h
>>> @@ -0,0 +1,34 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + */
>>> +
>>> +#ifndef __INTEL_DISPLAY_RPS_H__
>>> +#define __INTEL_DISPLAY_RPS_H__
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +struct dma_fence;
>>> +struct drm_crtc;
>>> +struct drm_i915_private;
>>> +struct intel_atomic_state;
>>> +
>>> +#ifdef I915
>>> +void intel_display_rps_boost_after_vblank(struct drm_crtc *crtc,
>>> + struct dma_fence *fence);
>>> +void intel_display_rps_mark_interactive(struct drm_i915_private *i915,
>>> + struct intel_atomic_state *state,
>>> + bool interactive);
>>> +#else
>>> +static inline void intel_display_rps_boost_after_vblank(struct drm_crtc *crtc,
>>> + struct dma_fence *fence)
>>> +{
>>> +}
>>> +static inline void intel_display_rps_mark_interactive(struct drm_i915_private *i915,
>>> + struct intel_atomic_state *state,
>>> + bool interactive)
>>> +{
>>> +}
>>> +#endif
>>> +
>>> +#endif /* __INTEL_DISPLAY_RPS_H__ */
>
>--
>Jani Nikula, Intel Open Source Graphics Center
More information about the Intel-xe
mailing list