On Thu, 22 Aug 2024, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> On Thu, Aug 22, 2024 at 07:04:51PM +0300, Jani Nikula wrote:
>> Going forward, struct intel_display shall replace struct
>> drm_i915_private as the main display device data pointer type. Convert
>> intel_vblank.[ch] to struct intel_display.
>>
>> Some stragglers are left behind where needed.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/display/intel_vblank.c | 97 +++++++++++----------
>> 1 file changed, 50 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
>> index 2073e8075af4..838b55ecb1d8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
>> @@ -67,7 +67,7 @@
>> */
>> u32 i915_get_vblank_counter(struct drm_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>> + struct intel_display *display = to_intel_display(crtc->dev);
>> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> const struct drm_display_mode *mode = &vblank->hwmode;
>> enum pipe pipe = to_intel_crtc(crtc)->pipe;
>> @@ -103,8 +103,8 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc)
>> * we get a low value that's stable across two reads of the high
>> * register.
>> */
>> - frame = intel_de_read64_2x32(dev_priv, PIPEFRAMEPIXEL(dev_priv, pipe),
>> - PIPEFRAME(dev_priv, pipe));
>> + frame = intel_de_read64_2x32(display, PIPEFRAMEPIXEL(display, pipe),
>> + PIPEFRAME(display, pipe));
>>
>> pixel = frame & PIPE_PIXEL_MASK;
>> frame = (frame >> PIPE_FRAME_LOW_SHIFT) & 0xffffff;
>> @@ -119,19 +119,19 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc)
>>
>> u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>> + struct intel_display *display = to_intel_display(crtc->dev);
>> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>
>> if (!vblank->max_vblank_count)
>> return 0;
>>
>> - return intel_de_read(dev_priv, PIPE_FRMCOUNT_G4X(dev_priv, pipe));
>> + return intel_de_read(display, PIPE_FRMCOUNT_G4X(display, pipe));
>> }
>>
>> static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> + struct intel_display *display = to_intel_display(crtc);
>> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(&crtc->base);
>> const struct drm_display_mode *mode = &vblank->hwmode;
>> u32 htotal = mode->crtc_htotal;
>> @@ -150,16 +150,16 @@ static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
>> * pipe frame time stamp. The time stamp value
>> * is sampled at every start of vertical blank.
>> */
>> - scan_prev_time = intel_de_read_fw(dev_priv,
>> + scan_prev_time = intel_de_read_fw(display,
>> PIPE_FRMTMSTMP(crtc->pipe));
>>
>> /*
>> * The TIMESTAMP_CTR register has the current
>> * time stamp value.
>> */
>> - scan_curr_time = intel_de_read_fw(dev_priv, IVB_TIMESTAMP_CTR);
>> + scan_curr_time = intel_de_read_fw(display, IVB_TIMESTAMP_CTR);
>>
>> - scan_post_time = intel_de_read_fw(dev_priv,
>> + scan_post_time = intel_de_read_fw(display,
>> PIPE_FRMTMSTMP(crtc->pipe));
>> } while (scan_post_time != scan_prev_time);
>>
>> @@ -192,6 +192,7 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
>>
>> static int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state)
>> {
>> + struct intel_display *display = to_intel_display(crtc_state);
>> struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
>>
>> /*
>> @@ -220,7 +221,7 @@ static int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state)
>> * However if queried just before the start of vblank we'll get an
>> * answer that's slightly in the future.
>> */
>> - if (DISPLAY_VER(i915) == 2)
>> + if (DISPLAY_VER(display) == 2)
>> return -1;
>> else if (HAS_DDI(i915) && intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>> return 2;
>> @@ -234,8 +235,7 @@ static int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state)
>> */
>> static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>> {
>> - struct drm_device *dev = crtc->base.dev;
>> - struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct intel_display *display = to_intel_display(crtc);
>> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(&crtc->base);
>> const struct drm_display_mode *mode = &vblank->hwmode;
>> enum pipe pipe = crtc->pipe;
>> @@ -249,7 +249,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>>
>> vtotal = intel_mode_vtotal(mode);
>>
>> - position = intel_de_read_fw(dev_priv, PIPEDSL(dev_priv, pipe)) & PIPEDSL_LINE_MASK;
>> + position = intel_de_read_fw(display, PIPEDSL(display, pipe)) & PIPEDSL_LINE_MASK;
>>
>> /*
>> * On HSW, the DSL reg (0x70000) appears to return 0 if we
>> @@ -263,13 +263,13 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>> * problem. We may need to extend this to include other platforms,
>> * but so far testing only shows the problem on HSW.
>> */
>> - if (HAS_DDI(dev_priv) && !position) {
>> + if (HAS_DDI(display) && !position) {
>> int i, temp;
>>
>> for (i = 0; i < 100; i++) {
>> udelay(1);
>> - temp = intel_de_read_fw(dev_priv,
>> - PIPEDSL(dev_priv, pipe)) & PIPEDSL_LINE_MASK;
>> + temp = intel_de_read_fw(display,
>> + PIPEDSL(display, pipe)) & PIPEDSL_LINE_MASK;
>> if (temp != position) {
>> position = temp;
>> break;
>> @@ -304,23 +304,25 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int scanline)
>> * otherwise they may hang.
>> */
>> #ifdef I915
>> -static void intel_vblank_section_enter(struct drm_i915_private *i915)
>> +static void intel_vblank_section_enter(struct intel_display *display)
>> __acquires(i915->uncore.lock)
>
> I'm surprised this works!
> thought we would need to have something like
> __acquires(to_i915(display->drm)->uncore.lock)
It's actually not a C identifier, although one is commonly used. From
sparse Documentation/annotations.rst:
The first argument, *ctxt*, is an expression only used as documentation
to identify the context. Usually, what is used is a pointer to the structure
containing the context, for example, the structure protected by the lock.
> but anyway, if it works let's move on
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Agreed, and thanks. The series pushed to din.
BR,
Jani.
>
>> {
>> + struct drm_i915_private *i915 = to_i915(display->drm);
>> spin_lock(&i915->uncore.lock);
>> }
>>
>> -static void intel_vblank_section_exit(struct drm_i915_private *i915)
>> +static void intel_vblank_section_exit(struct intel_display *display)
>> __releases(i915->uncore.lock)
>> {
>> + struct drm_i915_private *i915 = to_i915(display->drm);
>> spin_unlock(&i915->uncore.lock);
>> }
>> #else
>> -static void intel_vblank_section_enter(struct drm_i915_private *i915)
>> +static void intel_vblank_section_enter(struct intel_display *display)
>> {
>> }
>>
>> -static void intel_vblank_section_exit(struct drm_i915_private *i915)
>> +static void intel_vblank_section_exit(struct intel_display *display)
>> {
>> }
>> #endif
>> @@ -331,19 +333,19 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
>> ktime_t *stime, ktime_t *etime,
>> const struct drm_display_mode *mode)
>> {
>> - struct drm_device *dev = _crtc->dev;
>> - struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct intel_display *display = to_intel_display(_crtc->dev);
>> + struct drm_i915_private *dev_priv = to_i915(display->drm);
>> struct intel_crtc *crtc = to_intel_crtc(_crtc);
>> enum pipe pipe = crtc->pipe;
>> int position;
>> int vbl_start, vbl_end, hsync_start, htotal, vtotal;
>> unsigned long irqflags;
>> - bool use_scanline_counter = DISPLAY_VER(dev_priv) >= 5 ||
>> - IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) == 2 ||
>> + bool use_scanline_counter = DISPLAY_VER(display) >= 5 ||
>> + IS_G4X(dev_priv) || DISPLAY_VER(display) == 2 ||
>> crtc->mode_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
>>
>> - if (drm_WARN_ON(&dev_priv->drm, !mode->crtc_clock)) {
>> - drm_dbg(&dev_priv->drm,
>> + if (drm_WARN_ON(display->drm, !mode->crtc_clock)) {
>> + drm_dbg(display->drm,
>> "trying to get scanoutpos for disabled pipe %c\n",
>> pipe_name(pipe));
>> return false;
>> @@ -361,7 +363,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
>> * preemption disabled, so the following code must not block.
>> */
>> local_irq_save(irqflags);
>> - intel_vblank_section_enter(dev_priv);
>> + intel_vblank_section_enter(display);
>>
>> /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
>>
>> @@ -393,7 +395,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
>> * We can split this into vertical and horizontal
>> * scanout position.
>> */
>> - position = (intel_de_read_fw(dev_priv, PIPEFRAMEPIXEL(dev_priv, pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
>> + position = (intel_de_read_fw(display, PIPEFRAMEPIXEL(display, pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
>>
>> /* convert to pixel counts */
>> vbl_start *= htotal;
>> @@ -429,7 +431,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
>>
>> /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
>>
>> - intel_vblank_section_exit(dev_priv);
>> + intel_vblank_section_exit(display);
>> local_irq_restore(irqflags);
>>
>> /*
>> @@ -464,42 +466,42 @@ bool intel_crtc_get_vblank_timestamp(struct drm_crtc *crtc, int *max_error,
>>
>> int intel_get_crtc_scanline(struct intel_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> + struct intel_display *display = to_intel_display(crtc);
>> unsigned long irqflags;
>> int position;
>>
>> local_irq_save(irqflags);
>> - intel_vblank_section_enter(dev_priv);
>> + intel_vblank_section_enter(display);
>>
>> position = __intel_get_crtc_scanline(crtc);
>>
>> - intel_vblank_section_exit(dev_priv);
>> + intel_vblank_section_exit(display);
>> local_irq_restore(irqflags);
>>
>> return position;
>> }
>>
>> -static bool pipe_scanline_is_moving(struct drm_i915_private *dev_priv,
>> +static bool pipe_scanline_is_moving(struct intel_display *display,
>> enum pipe pipe)
>> {
>> - i915_reg_t reg = PIPEDSL(dev_priv, pipe);
>> + i915_reg_t reg = PIPEDSL(display, pipe);
>> u32 line1, line2;
>>
>> - line1 = intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK;
>> + line1 = intel_de_read(display, reg) & PIPEDSL_LINE_MASK;
>> msleep(5);
>> - line2 = intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK;
>> + line2 = intel_de_read(display, reg) & PIPEDSL_LINE_MASK;
>>
>> return line1 != line2;
>> }
>>
>> static void wait_for_pipe_scanline_moving(struct intel_crtc *crtc, bool state)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> + struct intel_display *display = to_intel_display(crtc);
>> enum pipe pipe = crtc->pipe;
>>
>> /* Wait for the display line to settle/start moving */
>> - if (wait_for(pipe_scanline_is_moving(dev_priv, pipe) == state, 100))
>> - drm_err(&dev_priv->drm,
>> + if (wait_for(pipe_scanline_is_moving(display, pipe) == state, 100))
>> + drm_err(display->drm,
>> "pipe %c scanline %s wait timed out\n",
>> pipe_name(pipe), str_on_off(state));
>> }
>> @@ -517,8 +519,8 @@ void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc)
>> void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
>> bool vrr_enable)
>> {
>> + struct intel_display *display = to_intel_display(crtc_state);
>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>> - struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>> u8 mode_flags = crtc_state->mode_flags;
>> struct drm_display_mode adjusted_mode;
>> int vmax_vblank_start = 0;
>> @@ -527,7 +529,8 @@ void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
>> drm_mode_init(&adjusted_mode, &crtc_state->hw.adjusted_mode);
>>
>> if (vrr_enable) {
>> - drm_WARN_ON(&i915->drm, (mode_flags & I915_MODE_FLAG_VRR) == 0);
>> + drm_WARN_ON(display->drm,
>> + (mode_flags & I915_MODE_FLAG_VRR) == 0);
>>
>> adjusted_mode.crtc_vtotal = crtc_state->vrr.vmax;
>> adjusted_mode.crtc_vblank_end = crtc_state->vrr.vmax;
>> @@ -549,8 +552,8 @@ void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
>> * __intel_get_crtc_scanline()) with vblank_time_lock?
>> * Need to audit everything to make sure it's safe.
>> */
>> - spin_lock_irqsave(&i915->drm.vblank_time_lock, irqflags);
>> - intel_vblank_section_enter(i915);
>> + spin_lock_irqsave(&display->drm->vblank_time_lock, irqflags);
>> + intel_vblank_section_enter(display);
>>
>> drm_calc_timestamping_constants(&crtc->base, &adjusted_mode);
>>
>> @@ -559,8 +562,8 @@ void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
>> crtc->mode_flags = mode_flags;
>>
>> crtc->scanline_offset = intel_crtc_scanline_offset(crtc_state);
>> - intel_vblank_section_exit(i915);
>> - spin_unlock_irqrestore(&i915->drm.vblank_time_lock, irqflags);
>> + intel_vblank_section_exit(display);
>> + spin_unlock_irqrestore(&display->drm->vblank_time_lock, irqflags);
>> }
>>
>> int intel_mode_vdisplay(const struct drm_display_mode *mode)
>> @@ -666,7 +669,7 @@ void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
>> int intel_vblank_evade(struct intel_vblank_evade_ctx *evade)
>> {
>> struct intel_crtc *crtc = evade->crtc;
>> - struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>> + struct intel_display *display = to_intel_display(crtc);
>> long timeout = msecs_to_jiffies_timeout(1);
>> wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base);
>> DEFINE_WAIT(wait);
>> @@ -688,7 +691,7 @@ int intel_vblank_evade(struct intel_vblank_evade_ctx *evade)
>> break;
>>
>> if (!timeout) {
>> - drm_err(&i915->drm,
>> + drm_err(display->drm,
>> "Potential atomic update failure on pipe %c\n",
>> pipe_name(crtc->pipe));
>> break;
>> --
>> 2.39.2
>>
--
Jani Nikula, Intel
