| Message ID | 1412067410-9346-15-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-09-30 5:56 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>: > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Patch 10/14: Needs rebase, but ok. Patch 11/14: Ok. Patch 12/14: Nice one, but: - I've seen you complaining about lines bigger than 80 columns before, so I obviously have to do the same here :) - Can't we now unexport (make them static) intel_set_{pch,cpu}_fifo_underrun_reporting()? Due to the rebasing problem I couldn't check this. Patch 13/14: Ok. Patch 14/14: Since you recently submitted a patch fixing spelling errors, I'll ask you to run a spell checker here too. I usually just go with ":set spell" in the only text editor that exists. Some stuff: PIPESTATE, occurance, interrup, interrup. For all patches, with or without changes: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > Documentation/DocBook/drm.tmpl | 5 ++ > drivers/gpu/drm/i915/intel_fifo_underrun.c | 82 +++++++++++++++++++++++------- > 2 files changed, 70 insertions(+), 17 deletions(-) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index e8ef0f9c7396..9356f16847d9 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -3831,6 +3831,11 @@ int num_ioctls;</synopsis> > !Fdrivers/gpu/drm/i915/i915_gem.c i915_gem_track_fb > </sect2> > <sect2> > + <title>Display FIFO Underrun Reporting</title> > +!Pdrivers/gpu/drm/i915/intel_fifo_underrun.c fifo underrun handling > +!Idrivers/gpu/drm/i915/intel_fifo_underrun.c > + </sect2> > + <sect2> > <title>Plane Configuration</title> > <para> > This section covers plane configuration and composition with the > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c > index 02909259bfb6..dfffad41664e 100644 > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c > @@ -28,6 +28,26 @@ > #include "i915_drv.h" > #include "intel_drv.h" > > +/** > + * DOC: fifo underrun handling > + * > + * The i915 driver checks for display fifo underruns using the interrupt signals > + * provided by the hardware. This is enabled by default and fairly useful to > + * debug display issues, especially watermark settings. > + * > + * If an underrun is detected this is logged into dmesg. To avoid flooding logs > + * and occupying the cpu underrun interrupts are disabled after the first > + * occurance until the next modeset on a given pipe. > + * > + * Note that underrun detection on gmch platforms is a bit more ugly since there > + * is no interrupt (despite that the signalling bit is in the PIPESTATE pipe > + * interrupt register). Also on some other platforms underrun interrupts are > + * shared, which means that if we detect an underrun we need to disable underrun > + * reporting on all pipes. > + * > + * The code also supports underrun detection on the PCH transcoder. > + */ > + > static bool ivb_can_enable_err_int(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -64,6 +84,14 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev) > return true; > } > > +/** > + * i9xx_check_fifo_underruns - check for fifo underruns > + * @dev_priv: i915 device instance > + * > + * This function checks for fifo underruns on GMCH platforms. This needs to be > + * done manually on modeset to make sure that we catch all underruns since they > + * do not generate an interrupt by themselves on these platforms. > + */ > void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv) > { > struct intel_crtc *crtc; > @@ -199,20 +227,6 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, > } > } > > -/** > - * intel_set_cpu_fifo_underrun_reporting - enable/disable FIFO underrun messages > - * @dev: drm device > - * @pipe: pipe > - * @enable: true if we want to report FIFO underrun errors, false otherwise > - * > - * This function makes us disable or enable CPU fifo underruns for a specific > - * pipe. Notice that on some Gens (e.g. IVB, HSW), disabling FIFO underrun > - * reporting for one pipe may also disable all the other CPU error interruts for > - * the other pipes, due to the fact that there's just one interrupt mask/enable > - * bit for all the pipes. > - * > - * Returns the previous state of underrun reporting. > - */ > static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, > enum pipe pipe, bool enable) > { > @@ -238,6 +252,22 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, > return old; > } > > +/** > + * intel_set_cpu_fifo_underrun_reporting - set cpu fifo underrrun reporting state > + * @dev_priv: i915 device instance > + * @pipe: (CPU) pipe to set state for > + * @enable: whether underruns should be reported or not > + * > + * This function sets the fifo underrun state for @pipe. It is used in the > + * modeset code to avoid false positives since on many platforms underruns are > + * expected when disabling or enabling the pipe. > + * > + * Notice that on some platforms disabling underrun reports for one pipe > + * disables for all due to shared interrupts. Actual reporting is still per-pipe > + * though. > + * > + * Returns the previous state of underrun reporting. > + */ > bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > enum pipe pipe, bool enable) > { > @@ -262,10 +292,10 @@ static bool __cpu_fifo_underrun_reporting_enabled(struct drm_i915_private *dev_p > } > > /** > - * intel_set_pch_fifo_underrun_reporting - enable/disable FIFO underrun messages > - * @dev: drm device > + * intel_set_pch_fifo_underrun_reporting - set PCH fifo underrun reporting state > + * @dev_priv: i915 device instance > * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older) > - * @enable: true if we want to report FIFO underrun errors, false otherwise > + * @enable: whether underruns should be reported or not > * > * This function makes us disable or enable PCH fifo underruns for a specific > * PCH transcoder. Notice that on some PCHs (e.g. CPT/PPT), disabling FIFO > @@ -309,6 +339,15 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > return old; > } > > +/** > + * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrup > + * @dev_priv: i915 device instance > + * @pipe: (CPU) pipe to set state for > + * > + * This handles a CPU fifo underrun interrupt, generating an underrun warning > + * into dmesg if underrun reporting is enabled and then disables the underrun > + * interrupt to avoid an irq storm. > + */ > void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > enum pipe pipe) > { > @@ -322,6 +361,15 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > pipe_name(pipe)); > } > > +/** > + * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrup > + * @dev_priv: i915 device instance > + * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older) > + * > + * This handles a PCH fifo underrun interrupt, generating an underrun warning > + * into dmesg if underrun reporting is enabled and then disables the underrun > + * interrupt to avoid an irq storm. > + */ > void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > enum transcoder pch_transcoder) > { > -- > 2.1.1 >
On Fri, Oct 03, 2014 at 03:00:46PM -0300, Paulo Zanoni wrote: > 2014-09-30 5:56 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>: > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Patch 10/14: Needs rebase, but ok. Was just an || IS_GEN9 from the skl patches. > Patch 11/14: Ok. > Patch 12/14: Nice one, but: > - I've seen you complaining about lines bigger than 80 columns before, > so I obviously have to do the same here :) > - Can't we now unexport (make them static) Fixed. > intel_set_{pch,cpu}_fifo_underrun_reporting()? Due to the rebasing > problem I couldn't check this. Still needed to hide underruns in some parts of the modeset sequence, so gets called from intel_display.c. Kerneldoc even explains this ;-) > Patch 13/14: Ok. > Patch 14/14: Since you recently submitted a patch fixing spelling > errors, I'll ask you to run a spell checker here too. I usually just > go with ":set spell" in the only text editor that exists. Some stuff: > PIPESTATE, occurance, interrup, interrup. Fixed. > For all patches, with or without changes: > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Thanks a lot for your review. Cheers, Daniel > > > --- > > Documentation/DocBook/drm.tmpl | 5 ++ > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 82 +++++++++++++++++++++++------- > > 2 files changed, 70 insertions(+), 17 deletions(-) > > > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > > index e8ef0f9c7396..9356f16847d9 100644 > > --- a/Documentation/DocBook/drm.tmpl > > +++ b/Documentation/DocBook/drm.tmpl > > @@ -3831,6 +3831,11 @@ int num_ioctls;</synopsis> > > !Fdrivers/gpu/drm/i915/i915_gem.c i915_gem_track_fb > > </sect2> > > <sect2> > > + <title>Display FIFO Underrun Reporting</title> > > +!Pdrivers/gpu/drm/i915/intel_fifo_underrun.c fifo underrun handling > > +!Idrivers/gpu/drm/i915/intel_fifo_underrun.c > > + </sect2> > > + <sect2> > > <title>Plane Configuration</title> > > <para> > > This section covers plane configuration and composition with the > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c > > index 02909259bfb6..dfffad41664e 100644 > > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c > > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c > > @@ -28,6 +28,26 @@ > > #include "i915_drv.h" > > #include "intel_drv.h" > > > > +/** > > + * DOC: fifo underrun handling > > + * > > + * The i915 driver checks for display fifo underruns using the interrupt signals > > + * provided by the hardware. This is enabled by default and fairly useful to > > + * debug display issues, especially watermark settings. > > + * > > + * If an underrun is detected this is logged into dmesg. To avoid flooding logs > > + * and occupying the cpu underrun interrupts are disabled after the first > > + * occurance until the next modeset on a given pipe. > > + * > > + * Note that underrun detection on gmch platforms is a bit more ugly since there > > + * is no interrupt (despite that the signalling bit is in the PIPESTATE pipe > > + * interrupt register). Also on some other platforms underrun interrupts are > > + * shared, which means that if we detect an underrun we need to disable underrun > > + * reporting on all pipes. > > + * > > + * The code also supports underrun detection on the PCH transcoder. > > + */ > > + > > static bool ivb_can_enable_err_int(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -64,6 +84,14 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev) > > return true; > > } > > > > +/** > > + * i9xx_check_fifo_underruns - check for fifo underruns > > + * @dev_priv: i915 device instance > > + * > > + * This function checks for fifo underruns on GMCH platforms. This needs to be > > + * done manually on modeset to make sure that we catch all underruns since they > > + * do not generate an interrupt by themselves on these platforms. > > + */ > > void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv) > > { > > struct intel_crtc *crtc; > > @@ -199,20 +227,6 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, > > } > > } > > > > -/** > > - * intel_set_cpu_fifo_underrun_reporting - enable/disable FIFO underrun messages > > - * @dev: drm device > > - * @pipe: pipe > > - * @enable: true if we want to report FIFO underrun errors, false otherwise > > - * > > - * This function makes us disable or enable CPU fifo underruns for a specific > > - * pipe. Notice that on some Gens (e.g. IVB, HSW), disabling FIFO underrun > > - * reporting for one pipe may also disable all the other CPU error interruts for > > - * the other pipes, due to the fact that there's just one interrupt mask/enable > > - * bit for all the pipes. > > - * > > - * Returns the previous state of underrun reporting. > > - */ > > static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, > > enum pipe pipe, bool enable) > > { > > @@ -238,6 +252,22 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, > > return old; > > } > > > > +/** > > + * intel_set_cpu_fifo_underrun_reporting - set cpu fifo underrrun reporting state > > + * @dev_priv: i915 device instance > > + * @pipe: (CPU) pipe to set state for > > + * @enable: whether underruns should be reported or not > > + * > > + * This function sets the fifo underrun state for @pipe. It is used in the > > + * modeset code to avoid false positives since on many platforms underruns are > > + * expected when disabling or enabling the pipe. > > + * > > + * Notice that on some platforms disabling underrun reports for one pipe > > + * disables for all due to shared interrupts. Actual reporting is still per-pipe > > + * though. > > + * > > + * Returns the previous state of underrun reporting. > > + */ > > bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > > enum pipe pipe, bool enable) > > { > > @@ -262,10 +292,10 @@ static bool __cpu_fifo_underrun_reporting_enabled(struct drm_i915_private *dev_p > > } > > > > /** > > - * intel_set_pch_fifo_underrun_reporting - enable/disable FIFO underrun messages > > - * @dev: drm device > > + * intel_set_pch_fifo_underrun_reporting - set PCH fifo underrun reporting state > > + * @dev_priv: i915 device instance > > * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older) > > - * @enable: true if we want to report FIFO underrun errors, false otherwise > > + * @enable: whether underruns should be reported or not > > * > > * This function makes us disable or enable PCH fifo underruns for a specific > > * PCH transcoder. Notice that on some PCHs (e.g. CPT/PPT), disabling FIFO > > @@ -309,6 +339,15 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > > return old; > > } > > > > +/** > > + * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrup > > + * @dev_priv: i915 device instance > > + * @pipe: (CPU) pipe to set state for > > + * > > + * This handles a CPU fifo underrun interrupt, generating an underrun warning > > + * into dmesg if underrun reporting is enabled and then disables the underrun > > + * interrupt to avoid an irq storm. > > + */ > > void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > > enum pipe pipe) > > { > > @@ -322,6 +361,15 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > > pipe_name(pipe)); > > } > > > > +/** > > + * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrup > > + * @dev_priv: i915 device instance > > + * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older) > > + * > > + * This handles a PCH fifo underrun interrupt, generating an underrun warning > > + * into dmesg if underrun reporting is enabled and then disables the underrun > > + * interrupt to avoid an irq storm. > > + */ > > void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > > enum transcoder pch_transcoder) > > { > > -- > > 2.1.1 > > > > > > -- > Paulo Zanoni
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index e8ef0f9c7396..9356f16847d9 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3831,6 +3831,11 @@ int num_ioctls;</synopsis> !Fdrivers/gpu/drm/i915/i915_gem.c i915_gem_track_fb </sect2> <sect2> + <title>Display FIFO Underrun Reporting</title> +!Pdrivers/gpu/drm/i915/intel_fifo_underrun.c fifo underrun handling +!Idrivers/gpu/drm/i915/intel_fifo_underrun.c + </sect2> + <sect2> <title>Plane Configuration</title> <para> This section covers plane configuration and composition with the diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index 02909259bfb6..dfffad41664e 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -28,6 +28,26 @@ #include "i915_drv.h" #include "intel_drv.h" +/** + * DOC: fifo underrun handling + * + * The i915 driver checks for display fifo underruns using the interrupt signals + * provided by the hardware. This is enabled by default and fairly useful to + * debug display issues, especially watermark settings. + * + * If an underrun is detected this is logged into dmesg. To avoid flooding logs + * and occupying the cpu underrun interrupts are disabled after the first + * occurance until the next modeset on a given pipe. + * + * Note that underrun detection on gmch platforms is a bit more ugly since there + * is no interrupt (despite that the signalling bit is in the PIPESTATE pipe + * interrupt register). Also on some other platforms underrun interrupts are + * shared, which means that if we detect an underrun we need to disable underrun + * reporting on all pipes. + * + * The code also supports underrun detection on the PCH transcoder. + */ + static bool ivb_can_enable_err_int(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -64,6 +84,14 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev) return true; } +/** + * i9xx_check_fifo_underruns - check for fifo underruns + * @dev_priv: i915 device instance + * + * This function checks for fifo underruns on GMCH platforms. This needs to be + * done manually on modeset to make sure that we catch all underruns since they + * do not generate an interrupt by themselves on these platforms. + */ void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv) { struct intel_crtc *crtc; @@ -199,20 +227,6 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, } } -/** - * intel_set_cpu_fifo_underrun_reporting - enable/disable FIFO underrun messages - * @dev: drm device - * @pipe: pipe - * @enable: true if we want to report FIFO underrun errors, false otherwise - * - * This function makes us disable or enable CPU fifo underruns for a specific - * pipe. Notice that on some Gens (e.g. IVB, HSW), disabling FIFO underrun - * reporting for one pipe may also disable all the other CPU error interruts for - * the other pipes, due to the fact that there's just one interrupt mask/enable - * bit for all the pipes. - * - * Returns the previous state of underrun reporting. - */ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, enum pipe pipe, bool enable) { @@ -238,6 +252,22 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, return old; } +/** + * intel_set_cpu_fifo_underrun_reporting - set cpu fifo underrrun reporting state + * @dev_priv: i915 device instance + * @pipe: (CPU) pipe to set state for + * @enable: whether underruns should be reported or not + * + * This function sets the fifo underrun state for @pipe. It is used in the + * modeset code to avoid false positives since on many platforms underruns are + * expected when disabling or enabling the pipe. + * + * Notice that on some platforms disabling underrun reports for one pipe + * disables for all due to shared interrupts. Actual reporting is still per-pipe + * though. + * + * Returns the previous state of underrun reporting. + */ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable) { @@ -262,10 +292,10 @@ static bool __cpu_fifo_underrun_reporting_enabled(struct drm_i915_private *dev_p } /** - * intel_set_pch_fifo_underrun_reporting - enable/disable FIFO underrun messages - * @dev: drm device + * intel_set_pch_fifo_underrun_reporting - set PCH fifo underrun reporting state + * @dev_priv: i915 device instance * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older) - * @enable: true if we want to report FIFO underrun errors, false otherwise + * @enable: whether underruns should be reported or not * * This function makes us disable or enable PCH fifo underruns for a specific * PCH transcoder. Notice that on some PCHs (e.g. CPT/PPT), disabling FIFO @@ -309,6 +339,15 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv, return old; } +/** + * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrup + * @dev_priv: i915 device instance + * @pipe: (CPU) pipe to set state for + * + * This handles a CPU fifo underrun interrupt, generating an underrun warning + * into dmesg if underrun reporting is enabled and then disables the underrun + * interrupt to avoid an irq storm. + */ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, enum pipe pipe) { @@ -322,6 +361,15 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, pipe_name(pipe)); } +/** + * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrup + * @dev_priv: i915 device instance + * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older) + * + * This handles a PCH fifo underrun interrupt, generating an underrun warning + * into dmesg if underrun reporting is enabled and then disables the underrun + * interrupt to avoid an irq storm. + */ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, enum transcoder pch_transcoder) {
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/DocBook/drm.tmpl | 5 ++ drivers/gpu/drm/i915/intel_fifo_underrun.c | 82 +++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 17 deletions(-)