| Message ID | 20240925111802.2227604-1-sai.teja.pottumuttu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Remove unused underrun interrupt bits | expand |
On Wed, Sep 25, 2024 at 04:48:02PM +0530, Sai Teja Pottumuttu wrote: > Underrun recovery was defeatured and was never brought into usage. > Thus we can safely remove the interrupt register bits introduced by the > feature for detecting soft and hard underruns. > > Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> > --- > .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- > .../gpu/drm/i915/display/intel_display_irq.h | 1 - > .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- There's a lot more related stuff in that file. > drivers/gpu/drm/i915/i915_reg.h | 2 -- > 4 files changed, 5 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c > index 6878dde85031..9d8a101b2415 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c > @@ -1031,17 +1031,6 @@ static u32 gen8_de_pipe_flip_done_mask(struct drm_i915_private *i915) > return GEN8_PIPE_PRIMARY_FLIP_DONE; > } > > -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *dev_priv) > -{ > - u32 mask = GEN8_PIPE_FIFO_UNDERRUN; > - > - if (DISPLAY_VER(dev_priv) >= 13) > - mask |= XELPD_PIPE_SOFT_UNDERRUN | > - XELPD_PIPE_HARD_UNDERRUN; > - > - return mask; > -} > - > static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_iir, u32 *pica_iir) > { > u32 pica_ier = 0; > @@ -1187,7 +1176,7 @@ void gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) > if (iir & GEN8_PIPE_CDCLK_CRC_DONE) > hsw_pipe_crc_irq_handler(dev_priv, pipe); > > - if (iir & gen8_de_pipe_underrun_mask(dev_priv)) > + if (iir & GEN8_PIPE_FIFO_UNDERRUN) > intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); > > fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); > @@ -1607,8 +1596,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, > u8 pipe_mask) > { > struct intel_uncore *uncore = &dev_priv->uncore; > - u32 extra_ier = GEN8_PIPE_VBLANK | > - gen8_de_pipe_underrun_mask(dev_priv) | > + u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | > gen8_de_pipe_flip_done_mask(dev_priv); > enum pipe pipe; > > @@ -1797,8 +1785,7 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) > GEN12_DSB_INT(INTEL_DSB_2); > > de_pipe_enables = de_pipe_masked | > - GEN8_PIPE_VBLANK | > - gen8_de_pipe_underrun_mask(dev_priv) | > + GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | > gen8_de_pipe_flip_done_mask(dev_priv); > > de_port_enables = de_port_masked; > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h > index 093e356a2894..1b3f559a0638 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_irq.h > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h > @@ -33,7 +33,6 @@ void ibx_disable_display_interrupt(struct drm_i915_private *i915, u32 bits); > > void gen8_irq_power_well_post_enable(struct drm_i915_private *i915, u8 pipe_mask); > void gen8_irq_power_well_pre_disable(struct drm_i915_private *i915, u8 pipe_mask); > -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *i915); > > int i8xx_enable_vblank(struct drm_crtc *crtc); > int i915gm_enable_vblank(struct drm_crtc *crtc); > diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > index 745ce22afb89..fb01c128e1c5 100644 > --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > @@ -209,7 +209,6 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, > enum pipe pipe, bool enable) > { > struct drm_i915_private *dev_priv = to_i915(dev); > - u32 mask = gen8_de_pipe_underrun_mask(dev_priv); > > if (enable) { > if (DISPLAY_VER(dev_priv) >= 11) > @@ -217,9 +216,9 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, > ICL_PIPESTATUS(dev_priv, pipe), > icl_pipe_status_underrun_mask(dev_priv)); > > - bdw_enable_pipe_irq(dev_priv, pipe, mask); > + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); > } else { > - bdw_disable_pipe_irq(dev_priv, pipe, mask); > + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); > } > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 7396fc630e29..c379d875f432 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2491,9 +2491,7 @@ > #define GEN12_PIPEDMC_INTERRUPT REG_BIT(26) /* tgl+ */ > #define GEN12_PIPEDMC_FAULT REG_BIT(25) /* tgl+ */ > #define MTL_PIPEDMC_ATS_FAULT REG_BIT(24) /* mtl+ */ > -#define XELPD_PIPE_SOFT_UNDERRUN REG_BIT(22) /* adl/dg2+ */ > #define GEN11_PIPE_PLANE7_FAULT REG_BIT(22) /* icl/tgl */ > -#define XELPD_PIPE_HARD_UNDERRUN REG_BIT(21) /* adl/dg2+ */ > #define GEN11_PIPE_PLANE6_FAULT REG_BIT(21) /* icl/tgl */ > #define GEN11_PIPE_PLANE5_FAULT REG_BIT(20) /* icl+ */ > #define GEN12_PIPE_VBLANK_UNMOD REG_BIT(19) /* tgl+ */ > -- > 2.34.1
On 25-09-2024 19:33, Ville Syrjälä wrote: > On Wed, Sep 25, 2024 at 04:48:02PM +0530, Sai Teja Pottumuttu wrote: >> Underrun recovery was defeatured and was never brought into usage. >> Thus we can safely remove the interrupt register bits introduced by the >> feature for detecting soft and hard underruns. >> >> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> >> --- >> .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- >> .../gpu/drm/i915/display/intel_display_irq.h | 1 - >> .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- > There's a lot more related stuff in that file. Assuming that you are referring to the ICL_PIPE_STATUS register and the bits added there to detect soft, hard, port underruns, I have not removed those bits in this patch because we are logging these bits in dmesg and specifically the PIPE_STATUS_PORT_UNDERRUN_XELPD, PIPE_STATUS_UNDERRUN bits seems to be set always because of which "port, transcoder" string appears in a lot of underrun issues filed by CI. I was under an assumption that if we remove these bits and the log, it would create trouble with CI filters and we might start seeing duplicates of the existing issues as new issues. So, my plan was to remove those bits at some later point. What would be your suggestion? Should we remove them now itself and work with CI to see that filters still detect the existing issues without "port, transcoder" as well. >> drivers/gpu/drm/i915/i915_reg.h | 2 -- >> 4 files changed, 5 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c >> index 6878dde85031..9d8a101b2415 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c >> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c >> @@ -1031,17 +1031,6 @@ static u32 gen8_de_pipe_flip_done_mask(struct drm_i915_private *i915) >> return GEN8_PIPE_PRIMARY_FLIP_DONE; >> } >> >> -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *dev_priv) >> -{ >> - u32 mask = GEN8_PIPE_FIFO_UNDERRUN; >> - >> - if (DISPLAY_VER(dev_priv) >= 13) >> - mask |= XELPD_PIPE_SOFT_UNDERRUN | >> - XELPD_PIPE_HARD_UNDERRUN; >> - >> - return mask; >> -} >> - >> static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_iir, u32 *pica_iir) >> { >> u32 pica_ier = 0; >> @@ -1187,7 +1176,7 @@ void gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) >> if (iir & GEN8_PIPE_CDCLK_CRC_DONE) >> hsw_pipe_crc_irq_handler(dev_priv, pipe); >> >> - if (iir & gen8_de_pipe_underrun_mask(dev_priv)) >> + if (iir & GEN8_PIPE_FIFO_UNDERRUN) >> intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); >> >> fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); >> @@ -1607,8 +1596,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, >> u8 pipe_mask) >> { >> struct intel_uncore *uncore = &dev_priv->uncore; >> - u32 extra_ier = GEN8_PIPE_VBLANK | >> - gen8_de_pipe_underrun_mask(dev_priv) | >> + u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | >> gen8_de_pipe_flip_done_mask(dev_priv); >> enum pipe pipe; >> >> @@ -1797,8 +1785,7 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) >> GEN12_DSB_INT(INTEL_DSB_2); >> >> de_pipe_enables = de_pipe_masked | >> - GEN8_PIPE_VBLANK | >> - gen8_de_pipe_underrun_mask(dev_priv) | >> + GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | >> gen8_de_pipe_flip_done_mask(dev_priv); >> >> de_port_enables = de_port_masked; >> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h >> index 093e356a2894..1b3f559a0638 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_irq.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h >> @@ -33,7 +33,6 @@ void ibx_disable_display_interrupt(struct drm_i915_private *i915, u32 bits); >> >> void gen8_irq_power_well_post_enable(struct drm_i915_private *i915, u8 pipe_mask); >> void gen8_irq_power_well_pre_disable(struct drm_i915_private *i915, u8 pipe_mask); >> -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *i915); >> >> int i8xx_enable_vblank(struct drm_crtc *crtc); >> int i915gm_enable_vblank(struct drm_crtc *crtc); >> diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >> index 745ce22afb89..fb01c128e1c5 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >> +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >> @@ -209,7 +209,6 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, >> enum pipe pipe, bool enable) >> { >> struct drm_i915_private *dev_priv = to_i915(dev); >> - u32 mask = gen8_de_pipe_underrun_mask(dev_priv); >> >> if (enable) { >> if (DISPLAY_VER(dev_priv) >= 11) >> @@ -217,9 +216,9 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, >> ICL_PIPESTATUS(dev_priv, pipe), >> icl_pipe_status_underrun_mask(dev_priv)); >> >> - bdw_enable_pipe_irq(dev_priv, pipe, mask); >> + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); >> } else { >> - bdw_disable_pipe_irq(dev_priv, pipe, mask); >> + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); >> } >> } >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 7396fc630e29..c379d875f432 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -2491,9 +2491,7 @@ >> #define GEN12_PIPEDMC_INTERRUPT REG_BIT(26) /* tgl+ */ >> #define GEN12_PIPEDMC_FAULT REG_BIT(25) /* tgl+ */ >> #define MTL_PIPEDMC_ATS_FAULT REG_BIT(24) /* mtl+ */ >> -#define XELPD_PIPE_SOFT_UNDERRUN REG_BIT(22) /* adl/dg2+ */ >> #define GEN11_PIPE_PLANE7_FAULT REG_BIT(22) /* icl/tgl */ >> -#define XELPD_PIPE_HARD_UNDERRUN REG_BIT(21) /* adl/dg2+ */ >> #define GEN11_PIPE_PLANE6_FAULT REG_BIT(21) /* icl/tgl */ >> #define GEN11_PIPE_PLANE5_FAULT REG_BIT(20) /* icl+ */ >> #define GEN12_PIPE_VBLANK_UNMOD REG_BIT(19) /* tgl+ */ >> -- >> 2.34.1
On 26.09.2024 10:52, Pottumuttu, Sai Teja wrote: > > On 25-09-2024 19:33, Ville Syrjälä wrote: > > On Wed, Sep 25, 2024 at 04:48:02PM +0530, Sai Teja Pottumuttu wrote: > > > Underrun recovery was defeatured and was never brought into usage. > > > Thus we can safely remove the interrupt register bits introduced by the > > > feature for detecting soft and hard underruns. > > > > > > Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> > > > --- > > > .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- > > > .../gpu/drm/i915/display/intel_display_irq.h | 1 - > > > .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- > > There's a lot more related stuff in that file. > > Assuming that you are referring to the ICL_PIPE_STATUS register and the bits > added there to detect soft, hard, port underruns, Is it only the underrun recovery defeatured? Do we have the reporting of underruns caused downstream by the port/transcoder working? Then it makes sense to me to still keep the Port/Transcoder underrun reporting as it would help in debugging any underruns. Still there would be stuff related to Hard/Soft underruns which can be removed from the driver like those in function icl_pipe_status_underrun_mask, printing of soft/hard underruns in intel_cpu_fifo_underrun_irq_handler. Regards, Bala > > I have not removed those bits in this patch because we are logging these > bits in dmesg and specifically the PIPE_STATUS_PORT_UNDERRUN_XELPD, > PIPE_STATUS_UNDERRUN bits seems to be set always because of which "port, > transcoder" string appears in a lot of underrun issues filed by CI. I was > under an assumption that if we remove these bits and the log, it would > create trouble with CI filters and we might start seeing duplicates of the > existing issues as new issues. So, my plan was to remove those bits at some > later point. > > What would be your suggestion? Should we remove them now itself and work > with CI to see that filters still detect the existing issues without "port, > transcoder" as well. > > > > drivers/gpu/drm/i915/i915_reg.h | 2 -- > > > 4 files changed, 5 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c > > > index 6878dde85031..9d8a101b2415 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c > > > @@ -1031,17 +1031,6 @@ static u32 gen8_de_pipe_flip_done_mask(struct drm_i915_private *i915) > > > return GEN8_PIPE_PRIMARY_FLIP_DONE; > > > } > > > -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *dev_priv) > > > -{ > > > - u32 mask = GEN8_PIPE_FIFO_UNDERRUN; > > > - > > > - if (DISPLAY_VER(dev_priv) >= 13) > > > - mask |= XELPD_PIPE_SOFT_UNDERRUN | > > > - XELPD_PIPE_HARD_UNDERRUN; > > > - > > > - return mask; > > > -} > > > - > > > static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_iir, u32 *pica_iir) > > > { > > > u32 pica_ier = 0; > > > @@ -1187,7 +1176,7 @@ void gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) > > > if (iir & GEN8_PIPE_CDCLK_CRC_DONE) > > > hsw_pipe_crc_irq_handler(dev_priv, pipe); > > > - if (iir & gen8_de_pipe_underrun_mask(dev_priv)) > > > + if (iir & GEN8_PIPE_FIFO_UNDERRUN) > > > intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); > > > fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); > > > @@ -1607,8 +1596,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, > > > u8 pipe_mask) > > > { > > > struct intel_uncore *uncore = &dev_priv->uncore; > > > - u32 extra_ier = GEN8_PIPE_VBLANK | > > > - gen8_de_pipe_underrun_mask(dev_priv) | > > > + u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | > > > gen8_de_pipe_flip_done_mask(dev_priv); > > > enum pipe pipe; > > > @@ -1797,8 +1785,7 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) > > > GEN12_DSB_INT(INTEL_DSB_2); > > > de_pipe_enables = de_pipe_masked | > > > - GEN8_PIPE_VBLANK | > > > - gen8_de_pipe_underrun_mask(dev_priv) | > > > + GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | > > > gen8_de_pipe_flip_done_mask(dev_priv); > > > de_port_enables = de_port_masked; > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h > > > index 093e356a2894..1b3f559a0638 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_irq.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h > > > @@ -33,7 +33,6 @@ void ibx_disable_display_interrupt(struct drm_i915_private *i915, u32 bits); > > > void gen8_irq_power_well_post_enable(struct drm_i915_private *i915, u8 pipe_mask); > > > void gen8_irq_power_well_pre_disable(struct drm_i915_private *i915, u8 pipe_mask); > > > -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *i915); > > > int i8xx_enable_vblank(struct drm_crtc *crtc); > > > int i915gm_enable_vblank(struct drm_crtc *crtc); > > > diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > > > index 745ce22afb89..fb01c128e1c5 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > > > +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > > > @@ -209,7 +209,6 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, > > > enum pipe pipe, bool enable) > > > { > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > - u32 mask = gen8_de_pipe_underrun_mask(dev_priv); > > > if (enable) { > > > if (DISPLAY_VER(dev_priv) >= 11) > > > @@ -217,9 +216,9 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, > > > ICL_PIPESTATUS(dev_priv, pipe), > > > icl_pipe_status_underrun_mask(dev_priv)); > > > - bdw_enable_pipe_irq(dev_priv, pipe, mask); > > > + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); > > > } else { > > > - bdw_disable_pipe_irq(dev_priv, pipe, mask); > > > + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); > > > } > > > } > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 7396fc630e29..c379d875f432 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -2491,9 +2491,7 @@ > > > #define GEN12_PIPEDMC_INTERRUPT REG_BIT(26) /* tgl+ */ > > > #define GEN12_PIPEDMC_FAULT REG_BIT(25) /* tgl+ */ > > > #define MTL_PIPEDMC_ATS_FAULT REG_BIT(24) /* mtl+ */ > > > -#define XELPD_PIPE_SOFT_UNDERRUN REG_BIT(22) /* adl/dg2+ */ > > > #define GEN11_PIPE_PLANE7_FAULT REG_BIT(22) /* icl/tgl */ > > > -#define XELPD_PIPE_HARD_UNDERRUN REG_BIT(21) /* adl/dg2+ */ > > > #define GEN11_PIPE_PLANE6_FAULT REG_BIT(21) /* icl/tgl */ > > > #define GEN11_PIPE_PLANE5_FAULT REG_BIT(20) /* icl+ */ > > > #define GEN12_PIPE_VBLANK_UNMOD REG_BIT(19) /* tgl+ */ > > > -- > > > 2.34.1
On 09-10-2024 13:05, Vivekanandan, Balasubramani wrote: > On 26.09.2024 10:52, Pottumuttu, Sai Teja wrote: >> On 25-09-2024 19:33, Ville Syrjälä wrote: >>> On Wed, Sep 25, 2024 at 04:48:02PM +0530, Sai Teja Pottumuttu wrote: >>>> Underrun recovery was defeatured and was never brought into usage. >>>> Thus we can safely remove the interrupt register bits introduced by the >>>> feature for detecting soft and hard underruns. >>>> >>>> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> >>>> --- >>>> .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- >>>> .../gpu/drm/i915/display/intel_display_irq.h | 1 - >>>> .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- >>> There's a lot more related stuff in that file. >> Assuming that you are referring to the ICL_PIPE_STATUS register and the bits >> added there to detect soft, hard, port underruns, > Is it only the underrun recovery defeatured? Do we have the reporting of > underruns caused downstream by the port/transcoder working? > Then it makes sense to me to still keep the Port/Transcoder underrun > reporting as it would help in debugging any underruns. > Still there would be stuff related to Hard/Soft underruns which can be > removed from the driver like those in function > icl_pipe_status_underrun_mask, printing of soft/hard underruns in > intel_cpu_fifo_underrun_irq_handler. > > Regards, > Bala The reporting is also de-featured. So, as part of the logging/reporting we had 4 things being reported and following are there statuses 1. PIPE_STATUS_SOFT_UNDERRUN_XELPD: This can safely be removed. 2. PIPE_STATUS_HARD_UNDERRUN_XELPD: This can be removed as well. 3. PIPE_STATUS_PORT_UNDERRUN_XELPD: This seems to be the problem, this is de-featured as well but currently is always set and thus the string "port" appears in the ci bugs. Removing this might cause duplications in ci bugs if it fails to understand that its the same bug even without "port". 4. PIPE_STATUS_UNDERRUN: This still tells that the underrun happened on the transcoder attached to this pipe. But then as far as I understand, the underrun interrupt itself tells that its an underrun on the transcoder so we need not use this bit specificially I believe. But then again removing this and the "transcoder" string there might cause ci issues. So, we have two options here 1. Either just remove the SOFT/HARD underrun bits/reporting for now and remove the other ones at a later point of time. 2. Or remove all these bits and make sure CI doesn't start filing duplicate bugs. What would be your suggestion here? > >> I have not removed those bits in this patch because we are logging these >> bits in dmesg and specifically the PIPE_STATUS_PORT_UNDERRUN_XELPD, >> PIPE_STATUS_UNDERRUN bits seems to be set always because of which "port, >> transcoder" string appears in a lot of underrun issues filed by CI. I was >> under an assumption that if we remove these bits and the log, it would >> create trouble with CI filters and we might start seeing duplicates of the >> existing issues as new issues. So, my plan was to remove those bits at some >> later point. >> >> What would be your suggestion? Should we remove them now itself and work >> with CI to see that filters still detect the existing issues without "port, >> transcoder" as well. >> >>>> drivers/gpu/drm/i915/i915_reg.h | 2 -- >>>> 4 files changed, 5 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c >>>> index 6878dde85031..9d8a101b2415 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c >>>> @@ -1031,17 +1031,6 @@ static u32 gen8_de_pipe_flip_done_mask(struct drm_i915_private *i915) >>>> return GEN8_PIPE_PRIMARY_FLIP_DONE; >>>> } >>>> -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *dev_priv) >>>> -{ >>>> - u32 mask = GEN8_PIPE_FIFO_UNDERRUN; >>>> - >>>> - if (DISPLAY_VER(dev_priv) >= 13) >>>> - mask |= XELPD_PIPE_SOFT_UNDERRUN | >>>> - XELPD_PIPE_HARD_UNDERRUN; >>>> - >>>> - return mask; >>>> -} >>>> - >>>> static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_iir, u32 *pica_iir) >>>> { >>>> u32 pica_ier = 0; >>>> @@ -1187,7 +1176,7 @@ void gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) >>>> if (iir & GEN8_PIPE_CDCLK_CRC_DONE) >>>> hsw_pipe_crc_irq_handler(dev_priv, pipe); >>>> - if (iir & gen8_de_pipe_underrun_mask(dev_priv)) >>>> + if (iir & GEN8_PIPE_FIFO_UNDERRUN) >>>> intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); >>>> fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); >>>> @@ -1607,8 +1596,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, >>>> u8 pipe_mask) >>>> { >>>> struct intel_uncore *uncore = &dev_priv->uncore; >>>> - u32 extra_ier = GEN8_PIPE_VBLANK | >>>> - gen8_de_pipe_underrun_mask(dev_priv) | >>>> + u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | >>>> gen8_de_pipe_flip_done_mask(dev_priv); >>>> enum pipe pipe; >>>> @@ -1797,8 +1785,7 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) >>>> GEN12_DSB_INT(INTEL_DSB_2); >>>> de_pipe_enables = de_pipe_masked | >>>> - GEN8_PIPE_VBLANK | >>>> - gen8_de_pipe_underrun_mask(dev_priv) | >>>> + GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | >>>> gen8_de_pipe_flip_done_mask(dev_priv); >>>> de_port_enables = de_port_masked; >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h >>>> index 093e356a2894..1b3f559a0638 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_display_irq.h >>>> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h >>>> @@ -33,7 +33,6 @@ void ibx_disable_display_interrupt(struct drm_i915_private *i915, u32 bits); >>>> void gen8_irq_power_well_post_enable(struct drm_i915_private *i915, u8 pipe_mask); >>>> void gen8_irq_power_well_pre_disable(struct drm_i915_private *i915, u8 pipe_mask); >>>> -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *i915); >>>> int i8xx_enable_vblank(struct drm_crtc *crtc); >>>> int i915gm_enable_vblank(struct drm_crtc *crtc); >>>> diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >>>> index 745ce22afb89..fb01c128e1c5 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >>>> @@ -209,7 +209,6 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, >>>> enum pipe pipe, bool enable) >>>> { >>>> struct drm_i915_private *dev_priv = to_i915(dev); >>>> - u32 mask = gen8_de_pipe_underrun_mask(dev_priv); >>>> if (enable) { >>>> if (DISPLAY_VER(dev_priv) >= 11) >>>> @@ -217,9 +216,9 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, >>>> ICL_PIPESTATUS(dev_priv, pipe), >>>> icl_pipe_status_underrun_mask(dev_priv)); >>>> - bdw_enable_pipe_irq(dev_priv, pipe, mask); >>>> + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); >>>> } else { >>>> - bdw_disable_pipe_irq(dev_priv, pipe, mask); >>>> + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); >>>> } >>>> } >>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >>>> index 7396fc630e29..c379d875f432 100644 >>>> --- a/drivers/gpu/drm/i915/i915_reg.h >>>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>>> @@ -2491,9 +2491,7 @@ >>>> #define GEN12_PIPEDMC_INTERRUPT REG_BIT(26) /* tgl+ */ >>>> #define GEN12_PIPEDMC_FAULT REG_BIT(25) /* tgl+ */ >>>> #define MTL_PIPEDMC_ATS_FAULT REG_BIT(24) /* mtl+ */ >>>> -#define XELPD_PIPE_SOFT_UNDERRUN REG_BIT(22) /* adl/dg2+ */ >>>> #define GEN11_PIPE_PLANE7_FAULT REG_BIT(22) /* icl/tgl */ >>>> -#define XELPD_PIPE_HARD_UNDERRUN REG_BIT(21) /* adl/dg2+ */ >>>> #define GEN11_PIPE_PLANE6_FAULT REG_BIT(21) /* icl/tgl */ >>>> #define GEN11_PIPE_PLANE5_FAULT REG_BIT(20) /* icl+ */ >>>> #define GEN12_PIPE_VBLANK_UNMOD REG_BIT(19) /* tgl+ */ >>>> -- >>>> 2.34.1
On 09.10.2024 15:35, Pottumuttu, Sai Teja wrote: > > On 09-10-2024 13:05, Vivekanandan, Balasubramani wrote: > > On 26.09.2024 10:52, Pottumuttu, Sai Teja wrote: > > > On 25-09-2024 19:33, Ville Syrjälä wrote: > > > > On Wed, Sep 25, 2024 at 04:48:02PM +0530, Sai Teja Pottumuttu wrote: > > > > > Underrun recovery was defeatured and was never brought into usage. > > > > > Thus we can safely remove the interrupt register bits introduced by the > > > > > feature for detecting soft and hard underruns. > > > > > > > > > > Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> > > > > > --- > > > > > .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- > > > > > .../gpu/drm/i915/display/intel_display_irq.h | 1 - > > > > > .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- > > > > There's a lot more related stuff in that file. > > > Assuming that you are referring to the ICL_PIPE_STATUS register and the bits > > > added there to detect soft, hard, port underruns, > > Is it only the underrun recovery defeatured? Do we have the reporting of > > underruns caused downstream by the port/transcoder working? > > Then it makes sense to me to still keep the Port/Transcoder underrun > > reporting as it would help in debugging any underruns. > > Still there would be stuff related to Hard/Soft underruns which can be > > removed from the driver like those in function > > icl_pipe_status_underrun_mask, printing of soft/hard underruns in > > intel_cpu_fifo_underrun_irq_handler. > > > > Regards, > > Bala > > The reporting is also de-featured. So, as part of the logging/reporting we > had 4 things being reported and following are there statuses > > 1. PIPE_STATUS_SOFT_UNDERRUN_XELPD: This can safely be removed. > 2. PIPE_STATUS_HARD_UNDERRUN_XELPD: This can be removed as well. > 3. PIPE_STATUS_PORT_UNDERRUN_XELPD: This seems to be the problem, this is > de-featured as well but currently is always set and thus the string "port" > appears in the ci bugs. Removing this might cause duplications in ci bugs if > it fails to understand that its the same bug even without "port". > 4. PIPE_STATUS_UNDERRUN: This still tells that the underrun happened on the > transcoder attached to this pipe. But then as far as I understand, the > underrun interrupt itself tells that its an underrun on the transcoder so we > need not use this bit specificially I believe. But then again removing this > and the "transcoder" string there might cause ci issues. > > So, we have two options here > > 1. Either just remove the SOFT/HARD underrun bits/reporting for now and > remove the other ones at a later point of time. > 2. Or remove all these bits and make sure CI doesn't start filing duplicate > bugs. > > What would be your suggestion here? We can just remove the SOFT/HARD underrun reporting for now to avoid impacting CI. Regards, Bala > > > > > > I have not removed those bits in this patch because we are logging these > > > bits in dmesg and specifically the PIPE_STATUS_PORT_UNDERRUN_XELPD, > > > PIPE_STATUS_UNDERRUN bits seems to be set always because of which "port, > > > transcoder" string appears in a lot of underrun issues filed by CI. I was > > > under an assumption that if we remove these bits and the log, it would > > > create trouble with CI filters and we might start seeing duplicates of the > > > existing issues as new issues. So, my plan was to remove those bits at some > > > later point. > > > > > > What would be your suggestion? Should we remove them now itself and work > > > with CI to see that filters still detect the existing issues without "port, > > > transcoder" as well. > > > > > > > > drivers/gpu/drm/i915/i915_reg.h | 2 -- > > > > > 4 files changed, 5 insertions(+), 22 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c > > > > > index 6878dde85031..9d8a101b2415 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c > > > > > @@ -1031,17 +1031,6 @@ static u32 gen8_de_pipe_flip_done_mask(struct drm_i915_private *i915) > > > > > return GEN8_PIPE_PRIMARY_FLIP_DONE; > > > > > } > > > > > -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *dev_priv) > > > > > -{ > > > > > - u32 mask = GEN8_PIPE_FIFO_UNDERRUN; > > > > > - > > > > > - if (DISPLAY_VER(dev_priv) >= 13) > > > > > - mask |= XELPD_PIPE_SOFT_UNDERRUN | > > > > > - XELPD_PIPE_HARD_UNDERRUN; > > > > > - > > > > > - return mask; > > > > > -} > > > > > - > > > > > static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_iir, u32 *pica_iir) > > > > > { > > > > > u32 pica_ier = 0; > > > > > @@ -1187,7 +1176,7 @@ void gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) > > > > > if (iir & GEN8_PIPE_CDCLK_CRC_DONE) > > > > > hsw_pipe_crc_irq_handler(dev_priv, pipe); > > > > > - if (iir & gen8_de_pipe_underrun_mask(dev_priv)) > > > > > + if (iir & GEN8_PIPE_FIFO_UNDERRUN) > > > > > intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); > > > > > fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); > > > > > @@ -1607,8 +1596,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, > > > > > u8 pipe_mask) > > > > > { > > > > > struct intel_uncore *uncore = &dev_priv->uncore; > > > > > - u32 extra_ier = GEN8_PIPE_VBLANK | > > > > > - gen8_de_pipe_underrun_mask(dev_priv) | > > > > > + u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | > > > > > gen8_de_pipe_flip_done_mask(dev_priv); > > > > > enum pipe pipe; > > > > > @@ -1797,8 +1785,7 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) > > > > > GEN12_DSB_INT(INTEL_DSB_2); > > > > > de_pipe_enables = de_pipe_masked | > > > > > - GEN8_PIPE_VBLANK | > > > > > - gen8_de_pipe_underrun_mask(dev_priv) | > > > > > + GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | > > > > > gen8_de_pipe_flip_done_mask(dev_priv); > > > > > de_port_enables = de_port_masked; > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h > > > > > index 093e356a2894..1b3f559a0638 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_irq.h > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h > > > > > @@ -33,7 +33,6 @@ void ibx_disable_display_interrupt(struct drm_i915_private *i915, u32 bits); > > > > > void gen8_irq_power_well_post_enable(struct drm_i915_private *i915, u8 pipe_mask); > > > > > void gen8_irq_power_well_pre_disable(struct drm_i915_private *i915, u8 pipe_mask); > > > > > -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *i915); > > > > > int i8xx_enable_vblank(struct drm_crtc *crtc); > > > > > int i915gm_enable_vblank(struct drm_crtc *crtc); > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > > > > > index 745ce22afb89..fb01c128e1c5 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c > > > > > @@ -209,7 +209,6 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, > > > > > enum pipe pipe, bool enable) > > > > > { > > > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > > - u32 mask = gen8_de_pipe_underrun_mask(dev_priv); > > > > > if (enable) { > > > > > if (DISPLAY_VER(dev_priv) >= 11) > > > > > @@ -217,9 +216,9 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, > > > > > ICL_PIPESTATUS(dev_priv, pipe), > > > > > icl_pipe_status_underrun_mask(dev_priv)); > > > > > - bdw_enable_pipe_irq(dev_priv, pipe, mask); > > > > > + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); > > > > > } else { > > > > > - bdw_disable_pipe_irq(dev_priv, pipe, mask); > > > > > + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); > > > > > } > > > > > } > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > > > index 7396fc630e29..c379d875f432 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > @@ -2491,9 +2491,7 @@ > > > > > #define GEN12_PIPEDMC_INTERRUPT REG_BIT(26) /* tgl+ */ > > > > > #define GEN12_PIPEDMC_FAULT REG_BIT(25) /* tgl+ */ > > > > > #define MTL_PIPEDMC_ATS_FAULT REG_BIT(24) /* mtl+ */ > > > > > -#define XELPD_PIPE_SOFT_UNDERRUN REG_BIT(22) /* adl/dg2+ */ > > > > > #define GEN11_PIPE_PLANE7_FAULT REG_BIT(22) /* icl/tgl */ > > > > > -#define XELPD_PIPE_HARD_UNDERRUN REG_BIT(21) /* adl/dg2+ */ > > > > > #define GEN11_PIPE_PLANE6_FAULT REG_BIT(21) /* icl/tgl */ > > > > > #define GEN11_PIPE_PLANE5_FAULT REG_BIT(20) /* icl+ */ > > > > > #define GEN12_PIPE_VBLANK_UNMOD REG_BIT(19) /* tgl+ */ > > > > > -- > > > > > 2.34.1
On Thu, Oct 10, 2024 at 10:43:44PM +0530, Vivekanandan, Balasubramani wrote: > On 09.10.2024 15:35, Pottumuttu, Sai Teja wrote: > > > > On 09-10-2024 13:05, Vivekanandan, Balasubramani wrote: > > > On 26.09.2024 10:52, Pottumuttu, Sai Teja wrote: > > > > On 25-09-2024 19:33, Ville Syrjälä wrote: > > > > > On Wed, Sep 25, 2024 at 04:48:02PM +0530, Sai Teja Pottumuttu wrote: > > > > > > Underrun recovery was defeatured and was never brought into usage. > > > > > > Thus we can safely remove the interrupt register bits introduced by the > > > > > > feature for detecting soft and hard underruns. > > > > > > > > > > > > Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> > > > > > > --- > > > > > > .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- > > > > > > .../gpu/drm/i915/display/intel_display_irq.h | 1 - > > > > > > .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- > > > > > There's a lot more related stuff in that file. > > > > Assuming that you are referring to the ICL_PIPE_STATUS register and the bits > > > > added there to detect soft, hard, port underruns, > > > Is it only the underrun recovery defeatured? Do we have the reporting of > > > underruns caused downstream by the port/transcoder working? > > > Then it makes sense to me to still keep the Port/Transcoder underrun > > > reporting as it would help in debugging any underruns. > > > Still there would be stuff related to Hard/Soft underruns which can be > > > removed from the driver like those in function > > > icl_pipe_status_underrun_mask, printing of soft/hard underruns in > > > intel_cpu_fifo_underrun_irq_handler. > > > > > > Regards, > > > Bala > > > > The reporting is also de-featured. So, as part of the logging/reporting we > > had 4 things being reported and following are there statuses > > > > 1. PIPE_STATUS_SOFT_UNDERRUN_XELPD: This can safely be removed. > > 2. PIPE_STATUS_HARD_UNDERRUN_XELPD: This can be removed as well. > > 3. PIPE_STATUS_PORT_UNDERRUN_XELPD: This seems to be the problem, this is > > de-featured as well but currently is always set and thus the string "port" > > appears in the ci bugs. Removing this might cause duplications in ci bugs if > > it fails to understand that its the same bug even without "port". > > 4. PIPE_STATUS_UNDERRUN: This still tells that the underrun happened on the > > transcoder attached to this pipe. But then as far as I understand, the > > underrun interrupt itself tells that its an underrun on the transcoder so we > > need not use this bit specificially I believe. But then again removing this > > and the "transcoder" string there might cause ci issues. > > > > So, we have two options here > > > > 1. Either just remove the SOFT/HARD underrun bits/reporting for now and > > remove the other ones at a later point of time. > > 2. Or remove all these bits and make sure CI doesn't start filing duplicate > > bugs. > > > > What would be your suggestion here? > > We can just remove the SOFT/HARD underrun reporting for now to avoid impacting CI. IMO just rip all of it out. It'll be completely gone from the hardware soon enough anyway, so we're just putting off the inevitable if we try to do some kind of partial surgery.
diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c index 6878dde85031..9d8a101b2415 100644 --- a/drivers/gpu/drm/i915/display/intel_display_irq.c +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c @@ -1031,17 +1031,6 @@ static u32 gen8_de_pipe_flip_done_mask(struct drm_i915_private *i915) return GEN8_PIPE_PRIMARY_FLIP_DONE; } -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *dev_priv) -{ - u32 mask = GEN8_PIPE_FIFO_UNDERRUN; - - if (DISPLAY_VER(dev_priv) >= 13) - mask |= XELPD_PIPE_SOFT_UNDERRUN | - XELPD_PIPE_HARD_UNDERRUN; - - return mask; -} - static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_iir, u32 *pica_iir) { u32 pica_ier = 0; @@ -1187,7 +1176,7 @@ void gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) if (iir & GEN8_PIPE_CDCLK_CRC_DONE) hsw_pipe_crc_irq_handler(dev_priv, pipe); - if (iir & gen8_de_pipe_underrun_mask(dev_priv)) + if (iir & GEN8_PIPE_FIFO_UNDERRUN) intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); @@ -1607,8 +1596,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, u8 pipe_mask) { struct intel_uncore *uncore = &dev_priv->uncore; - u32 extra_ier = GEN8_PIPE_VBLANK | - gen8_de_pipe_underrun_mask(dev_priv) | + u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | gen8_de_pipe_flip_done_mask(dev_priv); enum pipe pipe; @@ -1797,8 +1785,7 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) GEN12_DSB_INT(INTEL_DSB_2); de_pipe_enables = de_pipe_masked | - GEN8_PIPE_VBLANK | - gen8_de_pipe_underrun_mask(dev_priv) | + GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN | gen8_de_pipe_flip_done_mask(dev_priv); de_port_enables = de_port_masked; diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h index 093e356a2894..1b3f559a0638 100644 --- a/drivers/gpu/drm/i915/display/intel_display_irq.h +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h @@ -33,7 +33,6 @@ void ibx_disable_display_interrupt(struct drm_i915_private *i915, u32 bits); void gen8_irq_power_well_post_enable(struct drm_i915_private *i915, u8 pipe_mask); void gen8_irq_power_well_pre_disable(struct drm_i915_private *i915, u8 pipe_mask); -u32 gen8_de_pipe_underrun_mask(struct drm_i915_private *i915); int i8xx_enable_vblank(struct drm_crtc *crtc); int i915gm_enable_vblank(struct drm_crtc *crtc); diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c index 745ce22afb89..fb01c128e1c5 100644 --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c @@ -209,7 +209,6 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, enum pipe pipe, bool enable) { struct drm_i915_private *dev_priv = to_i915(dev); - u32 mask = gen8_de_pipe_underrun_mask(dev_priv); if (enable) { if (DISPLAY_VER(dev_priv) >= 11) @@ -217,9 +216,9 @@ static void bdw_set_fifo_underrun_reporting(struct drm_device *dev, ICL_PIPESTATUS(dev_priv, pipe), icl_pipe_status_underrun_mask(dev_priv)); - bdw_enable_pipe_irq(dev_priv, pipe, mask); + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); } else { - bdw_disable_pipe_irq(dev_priv, pipe, mask); + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); } } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7396fc630e29..c379d875f432 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2491,9 +2491,7 @@ #define GEN12_PIPEDMC_INTERRUPT REG_BIT(26) /* tgl+ */ #define GEN12_PIPEDMC_FAULT REG_BIT(25) /* tgl+ */ #define MTL_PIPEDMC_ATS_FAULT REG_BIT(24) /* mtl+ */ -#define XELPD_PIPE_SOFT_UNDERRUN REG_BIT(22) /* adl/dg2+ */ #define GEN11_PIPE_PLANE7_FAULT REG_BIT(22) /* icl/tgl */ -#define XELPD_PIPE_HARD_UNDERRUN REG_BIT(21) /* adl/dg2+ */ #define GEN11_PIPE_PLANE6_FAULT REG_BIT(21) /* icl/tgl */ #define GEN11_PIPE_PLANE5_FAULT REG_BIT(20) /* icl+ */ #define GEN12_PIPE_VBLANK_UNMOD REG_BIT(19) /* tgl+ */
Underrun recovery was defeatured and was never brought into usage. Thus we can safely remove the interrupt register bits introduced by the feature for detecting soft and hard underruns. Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> --- .../gpu/drm/i915/display/intel_display_irq.c | 19 +++---------------- .../gpu/drm/i915/display/intel_display_irq.h | 1 - .../drm/i915/display/intel_fifo_underrun.c | 5 ++--- drivers/gpu/drm/i915/i915_reg.h | 2 -- 4 files changed, 5 insertions(+), 22 deletions(-)