| Message ID | 1424305990-17983-20-git-send-email-kamal@canonical.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Feb 18, 2015 at 04:31:46PM -0800, Kamal Mostafa wrote: > 3.13.11-ckt16 -stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com> > > commit 1f1c2e2468f937cefd6bcb645c959c7b5d9821df upstream. > > Only plane A is FBC capable on gen2 (like gen3), but the panel fitter > is hooked up to pipe B, so we want to prefer pipe B + plane A. FBC doesn't even work on gen2 (still missing my stolen mem patches) so this patch doesn't really seem like stable material to me. But I take it you backported it to get the prepare_page_flip() patch to apply cleanly? If you really want to do that, then you should also backport commit 38af6096781e2ed3d83b970218dedfe98e2f8f9c Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Tue Jan 7 16:15:37 2014 +0200 drm/i915: Don't swap planes on 830M othwerwise you may have some angry 830 users knocking at the door, assuming there are more than two of us :) Although 830 support has been more or less broken since forever until 3.18 so maybe no one would notice anyway. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > [danvet: Add the code comment Chris requested in his review.] > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 14 +++++++++----- > drivers/gpu/drm/i915/intel_display.c | 7 +++++-- > 2 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 3a10da2..5e0b5e4 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3155,10 +3155,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev) > * Returns true when a page flip has completed. > */ > static bool i8xx_handle_vblank(struct drm_device *dev, > - int pipe, u16 iir) > + int plane, int pipe, u32 iir) > { > drm_i915_private_t *dev_priv = dev->dev_private; > - u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(pipe); > + u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane); > > if (!drm_handle_vblank(dev, pipe)) > return false; > @@ -3166,7 +3166,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev, > if ((iir & flip_pending) == 0) > return false; > > - intel_prepare_page_flip(dev, pipe); > + intel_prepare_page_flip(dev, plane); > > /* We detect FlipDone by looking for the change in PendingFlip from '1' > * to '0' on the following vblank, i.e. IIR has the Pendingflip > @@ -3235,9 +3235,13 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) > notify_ring(dev, &dev_priv->ring[RCS]); > > for_each_pipe(pipe) { > + int plane = pipe; > + if (IS_MOBILE(dev)) > + plane = !plane; > + > if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS && > - i8xx_handle_vblank(dev, pipe, iir)) > - flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe); > + i8xx_handle_vblank(dev, plane, pipe, iir)) > + flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane); > > if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS) > i9xx_pipe_crc_irq_handler(dev, pipe); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2d2c1e1..ae3bc77 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9949,10 +9949,13 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > intel_crtc->lut_b[i] = i; > } > > - /* Swap pipes & planes for FBC on pre-965 */ > + /* > + * On gen2/3 only plane A can do fbc, but the panel fitter and lvds port > + * is hooked to plane B. Hence we want plane A feeding pipe B. > + */ > intel_crtc->pipe = pipe; > intel_crtc->plane = pipe; > - if (IS_MOBILE(dev) && IS_GEN3(dev)) { > + if (IS_MOBILE(dev) && INTEL_INFO(dev)->gen < 4) { > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); > intel_crtc->plane = !pipe; > } > -- > 1.9.1
On Thu, 2015-02-19 at 11:31 +0200, Ville Syrjälä wrote: > On Wed, Feb 18, 2015 at 04:31:46PM -0800, Kamal Mostafa wrote: > > 3.13.11-ckt16 -stable review patch. If anyone has any objections, please let me know. > > > > ------------------ > > > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com> > > > > commit 1f1c2e2468f937cefd6bcb645c959c7b5d9821df upstream. > > > > Only plane A is FBC capable on gen2 (like gen3), but the panel fitter > > is hooked up to pipe B, so we want to prefer pipe B + plane A. > > FBC doesn't even work on gen2 (still missing my stolen mem patches) so > this patch doesn't really seem like stable material to me. But I take > it you backported it to get the prepare_page_flip() patch to apply cleanly? Yes, exactly (and I forgot to note that in my commit message). > If you really want to do that, then you should also backport > > commit 38af6096781e2ed3d83b970218dedfe98e2f8f9c > Author: Ville Syrjälä <ville.syrjala@linux.intel.com> > Date: Tue Jan 7 16:15:37 2014 +0200 > > drm/i915: Don't swap planes on 830M I will pick up that one too. So this 3.13-stable release will include all three of these, barring any further objections: 7d47559 drm/i915: Don't call intel_prepare_page_flip() multiple times on gen2-4 38af609 drm/i915: Don't swap planes on 830M 1f1c2e2 drm/i915: Swap primary planes on gen2 for FBC > othwerwise you may have some angry 830 users knocking at the door, > assuming there are more than two of us :) Although 830 support has > been more or less broken since forever until 3.18 so maybe no one > would notice anyway. Thanks very much for the guidance, Ville. -Kamal > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > [danvet: Add the code comment Chris requested in his review.] > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 14 +++++++++----- > > drivers/gpu/drm/i915/intel_display.c | 7 +++++-- > > 2 files changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 3a10da2..5e0b5e4 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -3155,10 +3155,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev) > > * Returns true when a page flip has completed. > > */ > > static bool i8xx_handle_vblank(struct drm_device *dev, > > - int pipe, u16 iir) > > + int plane, int pipe, u32 iir) > > { > > drm_i915_private_t *dev_priv = dev->dev_private; > > - u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(pipe); > > + u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane); > > > > if (!drm_handle_vblank(dev, pipe)) > > return false; > > @@ -3166,7 +3166,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev, > > if ((iir & flip_pending) == 0) > > return false; > > > > - intel_prepare_page_flip(dev, pipe); > > + intel_prepare_page_flip(dev, plane); > > > > /* We detect FlipDone by looking for the change in PendingFlip from '1' > > * to '0' on the following vblank, i.e. IIR has the Pendingflip > > @@ -3235,9 +3235,13 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) > > notify_ring(dev, &dev_priv->ring[RCS]); > > > > for_each_pipe(pipe) { > > + int plane = pipe; > > + if (IS_MOBILE(dev)) > > + plane = !plane; > > + > > if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS && > > - i8xx_handle_vblank(dev, pipe, iir)) > > - flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe); > > + i8xx_handle_vblank(dev, plane, pipe, iir)) > > + flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane); > > > > if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS) > > i9xx_pipe_crc_irq_handler(dev, pipe); > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 2d2c1e1..ae3bc77 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9949,10 +9949,13 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > intel_crtc->lut_b[i] = i; > > } > > > > - /* Swap pipes & planes for FBC on pre-965 */ > > + /* > > + * On gen2/3 only plane A can do fbc, but the panel fitter and lvds port > > + * is hooked to plane B. Hence we want plane A feeding pipe B. > > + */ > > intel_crtc->pipe = pipe; > > intel_crtc->plane = pipe; > > - if (IS_MOBILE(dev) && IS_GEN3(dev)) { > > + if (IS_MOBILE(dev) && INTEL_INFO(dev)->gen < 4) { > > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); > > intel_crtc->plane = !pipe; > > } > > -- > > 1.9.1 >
On Thu, Feb 19, 2015 at 10:55:42AM -0800, Kamal Mostafa wrote: > On Thu, 2015-02-19 at 11:31 +0200, Ville Syrjälä wrote: > > On Wed, Feb 18, 2015 at 04:31:46PM -0800, Kamal Mostafa wrote: > > > 3.13.11-ckt16 -stable review patch. If anyone has any objections, please let me know. > > > > > > ------------------ > > > > > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com> > > > > > > commit 1f1c2e2468f937cefd6bcb645c959c7b5d9821df upstream. > > > > > > Only plane A is FBC capable on gen2 (like gen3), but the panel fitter > > > is hooked up to pipe B, so we want to prefer pipe B + plane A. > > > > FBC doesn't even work on gen2 (still missing my stolen mem patches) so > > this patch doesn't really seem like stable material to me. But I take > > it you backported it to get the prepare_page_flip() patch to apply cleanly? > > Yes, exactly (and I forgot to note that in my commit message). > > > If you really want to do that, then you should also backport > > > > commit 38af6096781e2ed3d83b970218dedfe98e2f8f9c > > Author: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Date: Tue Jan 7 16:15:37 2014 +0200 > > > > drm/i915: Don't swap planes on 830M > > I will pick up that one too. So this 3.13-stable release will include > all three of these, barring any further objections: > > 7d47559 drm/i915: Don't call intel_prepare_page_flip() multiple times on gen2-4 > 38af609 drm/i915: Don't swap planes on 830M > 1f1c2e2 drm/i915: Swap primary planes on gen2 for FBC > > > othwerwise you may have some angry 830 users knocking at the door, > > assuming there are more than two of us :) Although 830 support has > > been more or less broken since forever until 3.18 so maybe no one > > would notice anyway. > > Thanks very much for the guidance, Ville. No problem. When fixing a regression we do try to identify the bad commit in the commit message of the fix. So when considering backporting a commit, I think it would be a good idea to grep more recent commit messages for its hash, just in case it has caused some regression. > > -Kamal > > > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > > [danvet: Add the code comment Chris requested in his review.] > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > > > --- > > > drivers/gpu/drm/i915/i915_irq.c | 14 +++++++++----- > > > drivers/gpu/drm/i915/intel_display.c | 7 +++++-- > > > 2 files changed, 14 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > > index 3a10da2..5e0b5e4 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -3155,10 +3155,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev) > > > * Returns true when a page flip has completed. > > > */ > > > static bool i8xx_handle_vblank(struct drm_device *dev, > > > - int pipe, u16 iir) > > > + int plane, int pipe, u32 iir) > > > { > > > drm_i915_private_t *dev_priv = dev->dev_private; > > > - u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(pipe); > > > + u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane); > > > > > > if (!drm_handle_vblank(dev, pipe)) > > > return false; > > > @@ -3166,7 +3166,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev, > > > if ((iir & flip_pending) == 0) > > > return false; > > > > > > - intel_prepare_page_flip(dev, pipe); > > > + intel_prepare_page_flip(dev, plane); > > > > > > /* We detect FlipDone by looking for the change in PendingFlip from '1' > > > * to '0' on the following vblank, i.e. IIR has the Pendingflip > > > @@ -3235,9 +3235,13 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) > > > notify_ring(dev, &dev_priv->ring[RCS]); > > > > > > for_each_pipe(pipe) { > > > + int plane = pipe; > > > + if (IS_MOBILE(dev)) > > > + plane = !plane; > > > + > > > if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS && > > > - i8xx_handle_vblank(dev, pipe, iir)) > > > - flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe); > > > + i8xx_handle_vblank(dev, plane, pipe, iir)) > > > + flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane); > > > > > > if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS) > > > i9xx_pipe_crc_irq_handler(dev, pipe); > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 2d2c1e1..ae3bc77 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -9949,10 +9949,13 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > > intel_crtc->lut_b[i] = i; > > > } > > > > > > - /* Swap pipes & planes for FBC on pre-965 */ > > > + /* > > > + * On gen2/3 only plane A can do fbc, but the panel fitter and lvds port > > > + * is hooked to plane B. Hence we want plane A feeding pipe B. > > > + */ > > > intel_crtc->pipe = pipe; > > > intel_crtc->plane = pipe; > > > - if (IS_MOBILE(dev) && IS_GEN3(dev)) { > > > + if (IS_MOBILE(dev) && INTEL_INFO(dev)->gen < 4) { > > > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); > > > intel_crtc->plane = !pipe; > > > } > > > -- > > > 1.9.1 > > >
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 3a10da2..5e0b5e4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3155,10 +3155,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev) * Returns true when a page flip has completed. */ static bool i8xx_handle_vblank(struct drm_device *dev, - int pipe, u16 iir) + int plane, int pipe, u32 iir) { drm_i915_private_t *dev_priv = dev->dev_private; - u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(pipe); + u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane); if (!drm_handle_vblank(dev, pipe)) return false; @@ -3166,7 +3166,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev, if ((iir & flip_pending) == 0) return false; - intel_prepare_page_flip(dev, pipe); + intel_prepare_page_flip(dev, plane); /* We detect FlipDone by looking for the change in PendingFlip from '1' * to '0' on the following vblank, i.e. IIR has the Pendingflip @@ -3235,9 +3235,13 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) notify_ring(dev, &dev_priv->ring[RCS]); for_each_pipe(pipe) { + int plane = pipe; + if (IS_MOBILE(dev)) + plane = !plane; + if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS && - i8xx_handle_vblank(dev, pipe, iir)) - flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe); + i8xx_handle_vblank(dev, plane, pipe, iir)) + flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane); if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS) i9xx_pipe_crc_irq_handler(dev, pipe); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2d2c1e1..ae3bc77 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9949,10 +9949,13 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) intel_crtc->lut_b[i] = i; } - /* Swap pipes & planes for FBC on pre-965 */ + /* + * On gen2/3 only plane A can do fbc, but the panel fitter and lvds port + * is hooked to plane B. Hence we want plane A feeding pipe B. + */ intel_crtc->pipe = pipe; intel_crtc->plane = pipe; - if (IS_MOBILE(dev) && IS_GEN3(dev)) { + if (IS_MOBILE(dev) && INTEL_INFO(dev)->gen < 4) { DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); intel_crtc->plane = !pipe; }