[Intel-gfx] [PATCH v8 1/8] drm/i915: Add enable/disable flip done and flip done handler
Karthik B S
karthik.b.s at intel.com
Wed Sep 16 12:36:10 UTC 2020
On 9/15/2020 7:17 PM, Ville Syrjälä wrote:
> On Mon, Sep 14, 2020 at 11:26:26AM +0530, Karthik B S wrote:
>> Add enable/disable flip done functions and the flip done handler
>> function which handles the flip done interrupt.
>>
>> Enable the flip done interrupt in IER.
>>
>> Enable flip done function is called before writing the
>> surface address register as the write to this register triggers
>> the flip done interrupt
>>
>> Flip done handler is used to send the page flip event as soon as the
>> surface address is written as per the requirement of async flips.
>> The interrupt is disabled after the event is sent.
>>
>> v2: -Change function name from icl_* to skl_* (Paulo)
>> -Move flip handler to this patch (Paulo)
>> -Remove vblank_put() (Paulo)
>> -Enable flip done interrupt for gen9+ only (Paulo)
>> -Enable flip done interrupt in power_well_post_enable hook (Paulo)
>> -Removed the event check in flip done handler to handle async
>> flips without pageflip events.
>>
>> v3: -Move skl_disable_flip_done out of interrupt handler (Paulo)
>> -Make the pending vblank event NULL in the beginning of
>> flip_done_handler to remove sporadic WARN_ON that is seen.
>>
>> v4: -Calculate timestamps using flip done time stamp and current
>> timestamp for async flips (Ville)
>>
>> v5: -Fix the sparse warning by making the function 'g4x_get_flip_counter'
>> static.(Reported-by: kernel test robot <lkp at intel.com>)
>> -Fix the typo in commit message.
>>
>> v6: -Revert back to old time stamping code.
>> -Remove the break while calling skl_enable_flip_done. (Paulo)
>>
>> v7: -Rebased.
>>
>> v8: -Rebased.
>>
>> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
>> Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_display.c | 8 +++
>> drivers/gpu/drm/i915/i915_irq.c | 52 ++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_irq.h | 2 +
>> 3 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index ec148a8da2c2..48712fb0a251 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -15606,6 +15606,11 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>>
>> intel_dbuf_pre_plane_update(state);
>>
>> + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>> + if (new_crtc_state->uapi.async_flip)
>> + skl_enable_flip_done(&crtc->base);
>> + }
>> +
>> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
>> dev_priv->display.commit_modeset_enables(state);
>>
>> @@ -15627,6 +15632,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>> drm_atomic_helper_wait_for_flip_done(dev, &state->base);
>>
>> for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>> + if (new_crtc_state->uapi.async_flip)
>> + skl_disable_flip_done(&crtc->base);
>> +
>> if (new_crtc_state->hw.active &&
>> !needs_modeset(new_crtc_state) &&
>> !new_crtc_state->preload_luts &&
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index f113fe44572b..6cc129b031d3 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1296,6 +1296,23 @@ display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>> u32 crc4) {}
>> #endif
>>
>> +static void flip_done_handler(struct drm_i915_private *dev_priv,
>
> Pls use
> struct drm_i915_private *i915
> in new code.
Thanks for the review.
Sure. I'll change this.
>
>> + unsigned int pipe)
>
> enum pipe pipe
>
Noted.
>> +{
>> + struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>> + struct drm_crtc_state *crtc_state = crtc->base.state;
>> + struct drm_pending_vblank_event *e = crtc_state->event;
>> + struct drm_device *dev = &dev_priv->drm;
>> + unsigned long irqflags;
>> +
>> + spin_lock_irqsave(&dev->event_lock, irqflags);
>> +
>> + crtc_state->event = NULL;
>> +
>> + drm_crtc_send_vblank_event(&crtc->base, e);
>> +
>> + spin_unlock_irqrestore(&dev->event_lock, irqflags);
>> +}
>>
>> static void hsw_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>> enum pipe pipe)
>> @@ -2390,6 +2407,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>> if (iir & GEN8_PIPE_VBLANK)
>> intel_handle_vblank(dev_priv, pipe);
>>
>> + if (iir & GEN9_PIPE_PLANE1_FLIP_DONE)
>> + flip_done_handler(dev_priv, pipe);
>> +
>> if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>> hsw_pipe_crc_irq_handler(dev_priv, pipe);
>>
>> @@ -2711,6 +2731,19 @@ int bdw_enable_vblank(struct drm_crtc *crtc)
>> return 0;
>> }
>>
>> +void skl_enable_flip_done(struct drm_crtc *crtc)
>
> Pls use
> struct intel_crtc *crtc
> instead
>
Sure. I'll update this.
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
>> + unsigned long irqflags;
>> +
>> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>> +
>> + bdw_enable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
>> +
>> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> +}
>> +
>> /* Called from drm generic code, passed 'crtc' which
>> * we use as a pipe index
>> */
>> @@ -2771,6 +2804,19 @@ void bdw_disable_vblank(struct drm_crtc *crtc)
>> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> }
>>
>> +void skl_disable_flip_done(struct drm_crtc *crtc)
>
> struct intel_crtc *crtc
> here too
>
Sure. I'll update this.
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>
> s/dev_priv/i915/
>
I'll update this.
>> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
>> + unsigned long irqflags;
>> +
>> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>> +
>> + bdw_disable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
>> +
>> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> +}
>> +
>> static void ibx_irq_reset(struct drm_i915_private *dev_priv)
>> {
>> struct intel_uncore *uncore = &dev_priv->uncore;
>> @@ -2981,6 +3027,9 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
>> u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
>> enum pipe pipe;
>>
>> + if (INTEL_GEN(dev_priv) >= 9)
>> + extra_ier |= GEN9_PIPE_PLANE1_FLIP_DONE;
>> +
>> spin_lock_irq(&dev_priv->irq_lock);
>>
>> if (!intel_irqs_enabled(dev_priv)) {
>> @@ -3459,6 +3508,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>> de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
>> GEN8_PIPE_FIFO_UNDERRUN;
>>
>> + if (INTEL_GEN(dev_priv) >= 9)
>> + de_pipe_enables |= GEN9_PIPE_PLANE1_FLIP_DONE;
>> +
>> de_port_enables = de_port_masked;
>> if (IS_GEN9_LP(dev_priv))
>> de_port_enables |= BXT_DE_PORT_HOTPLUG_MASK;
>> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
>> index 25f25cd95818..2f10c8135116 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.h
>> +++ b/drivers/gpu/drm/i915/i915_irq.h
>> @@ -112,11 +112,13 @@ int i915gm_enable_vblank(struct drm_crtc *crtc);
>> int i965_enable_vblank(struct drm_crtc *crtc);
>> int ilk_enable_vblank(struct drm_crtc *crtc);
>> int bdw_enable_vblank(struct drm_crtc *crtc);
>> +void skl_enable_flip_done(struct drm_crtc *crtc);
>
> I wouldn't mix these with the vblank hooks like this.
>
Sure. I'll separate them out.
> With those this patch is
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Thanks for the RB.
Thanks,
Karthik.B.S
>
>> void i8xx_disable_vblank(struct drm_crtc *crtc);
>> void i915gm_disable_vblank(struct drm_crtc *crtc);
>> void i965_disable_vblank(struct drm_crtc *crtc);
>> void ilk_disable_vblank(struct drm_crtc *crtc);
>> void bdw_disable_vblank(struct drm_crtc *crtc);
>> +void skl_disable_flip_done(struct drm_crtc *crtc);
>>
>> void gen2_irq_reset(struct intel_uncore *uncore);
>> void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
>> --
>> 2.22.0
>
More information about the Intel-gfx
mailing list