[Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits

2016-01-21 Thread Paulo Zanoni 
We unconditionally disable/update FBC even during the page flip
IOCTLs, and an unconditional disable/update at every atomic commit
touching the primary plane shouldn't impact PC state residency
noticeably. Besides, the code that checks for rotation is a good hint
that we may be forgetting something else, so let's leave all the
decisions to intel_fbc.c, making the code much safer.

Once we have the code to properly make FBC enable/update decisions
based on atomic states, with proper locking, then we'll be able to
evaluate whether it will be worth trying to optimize the cases where a
disable isn't needed.

v2: Upstream moved and now our patch needs to remove dev_priv.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_display.c | 23 ++-
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 4329ba1..d876887 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11838,7 +11838,6 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *plane = plane_state->plane;
struct drm_device *dev = crtc->dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_plane_state *old_plane_state =
to_intel_plane_state(plane->state);
int idx = intel_crtc->base.base.id, ret;
@@ -11906,6 +11905,8 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
case DRM_PLANE_TYPE_PRIMARY:
intel_crtc->atomic.pre_disable_primary = turn_off;
intel_crtc->atomic.post_enable_primary = turn_on;
+   intel_crtc->atomic.disable_fbc = true;
+   intel_crtc->atomic.update_fbc = true;
 
if (turn_off) {
/*
@@ -11917,28 +11918,9 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
 * disable.
 */
intel_crtc->atomic.disable_ips = true;
-
-   intel_crtc->atomic.disable_fbc = true;
}
 
/*
-* FBC does not work on some platforms for rotated
-* planes, so disable it when rotation is not 0 and
-* update it when rotation is set back to 0.
-*
-* FIXME: This is redundant with the fbc update done in
-* the primary plane enable function except that that
-* one is done too late. We eventually need to unify
-* this.
-*/
-
-   if (visible &&
-   INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
-   dev_priv->fbc.crtc == intel_crtc &&
-   plane_state->rotation != BIT(DRM_ROTATE_0))
-   intel_crtc->atomic.disable_fbc = true;
-
-   /*
 * BDW signals flip done immediately if the plane
 * is disabled, even if the plane enable is already
 * armed to occur at the next vblank :(
@@ -11946,7 +11928,6 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
if (turn_on && IS_BROADWELL(dev))
intel_crtc->atomic.wait_vblank = true;
 
-   intel_crtc->atomic.update_fbc |= visible || mode_changed;
break;
case DRM_PLANE_TYPE_CURSOR:
break;
-- 
2.6.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits

2016-01-21 Thread Maarten Lankhorst 
Op 21-01-16 om 14:27 schreef Zanoni, Paulo R:
> Em Qui, 2016-01-21 às 14:04 +0100, Maarten Lankhorst escreveu:
>> Op 19-01-16 om 14:35 schreef Paulo Zanoni:
>>> We unconditionally disable/update FBC even during the page flip
>>> IOCTLs, and an unconditional disable/update at every atomic commit
>>> touching the primary plane shouldn't impact PC state residency
>>> noticeably. Besides, the code that checks for rotation is a good
>>> hint
>>> that we may be forgetting something else, so let's leave all the
>>> decisions to intel_fbc.c, making the code much safer.
>>>
>>> Once we have the code to properly make FBC enable/update decisions
>>> based on atomic states, with proper locking, then we'll be able to
>>> evaluate whether it will be worth trying to optimize the cases
>>> where a
>>> disable isn't needed.
>>>
>>> Signed-off-by: Paulo Zanoni 
>> I would rather have this patch remove those 2 members entirely, but I
>> can work with this for now.
> And what would be the new way to know whether a given atomic commit
> touches the primary plane of a given crtc?
if (drm_atomic_get_existing_plane_state(old_crtc_state->state, crtc->primary))
>> Could nuke at least disable_fbc though, being redundant with
>> update_fbc. :)
> Check patch 11 :)
>
>> ~Maarten

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits

2016-01-21 Thread Zanoni, Paulo R 
Em Qui, 2016-01-21 às 14:04 +0100, Maarten Lankhorst escreveu:
> Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> > We unconditionally disable/update FBC even during the page flip
> > IOCTLs, and an unconditional disable/update at every atomic commit
> > touching the primary plane shouldn't impact PC state residency
> > noticeably. Besides, the code that checks for rotation is a good
> > hint
> > that we may be forgetting something else, so let's leave all the
> > decisions to intel_fbc.c, making the code much safer.
> > 
> > Once we have the code to properly make FBC enable/update decisions
> > based on atomic states, with proper locking, then we'll be able to
> > evaluate whether it will be worth trying to optimize the cases
> > where a
> > disable isn't needed.
> > 
> > Signed-off-by: Paulo Zanoni 
> I would rather have this patch remove those 2 members entirely, but I
> can work with this for now.

And what would be the new way to know whether a given atomic commit
touches the primary plane of a given crtc?

> 
> Could nuke at least disable_fbc though, being redundant with
> update_fbc. :)

Check patch 11 :)

> 
> ~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits

2016-01-21 Thread Maarten Lankhorst 
Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> We unconditionally disable/update FBC even during the page flip
> IOCTLs, and an unconditional disable/update at every atomic commit
> touching the primary plane shouldn't impact PC state residency
> noticeably. Besides, the code that checks for rotation is a good hint
> that we may be forgetting something else, so let's leave all the
> decisions to intel_fbc.c, making the code much safer.
>
> Once we have the code to properly make FBC enable/update decisions
> based on atomic states, with proper locking, then we'll be able to
> evaluate whether it will be worth trying to optimize the cases where a
> disable isn't needed.
>
> Signed-off-by: Paulo Zanoni 
I would rather have this patch remove those 2 members entirely, but I can work 
with this for now.

Could nuke at least disable_fbc though, being redundant with update_fbc. :)

~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits

2016-01-19 Thread kbuild test robot 
Hi Paulo,

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20160119]
[cannot apply to v4.4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Paulo-Zanoni/FBC-crtc-fb-locking-smaller-fixes/20160119-214108
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x011-01180513 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_display.c: In function 
'intel_plane_atomic_calc_changes':
>> drivers/gpu/drm/i915/intel_display.c:11811:27: warning: unused variable 
>> 'dev_priv' [-Wunused-variable]
 struct drm_i915_private *dev_priv = dev->dev_private;
  ^

vim +/dev_priv +11811 drivers/gpu/drm/i915/intel_display.c

d21fbe87 Matt Roper2015-09-24  11795int src_w = 
drm_rect_width(&state->src) >> 16;
d21fbe87 Matt Roper2015-09-24  11796int src_h = 
drm_rect_height(&state->src) >> 16;
d21fbe87 Matt Roper2015-09-24  11797int dst_w = 
drm_rect_width(&state->dst);
d21fbe87 Matt Roper2015-09-24  11798int dst_h = 
drm_rect_height(&state->dst);
d21fbe87 Matt Roper2015-09-24  11799  
d21fbe87 Matt Roper2015-09-24  11800return (src_w != dst_w || src_h 
!= dst_h);
d21fbe87 Matt Roper2015-09-24  11801  }
d21fbe87 Matt Roper2015-09-24  11802  
da20eabd Maarten Lankhorst 2015-06-15  11803  int 
intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
da20eabd Maarten Lankhorst 2015-06-15  11804
struct drm_plane_state *plane_state)
da20eabd Maarten Lankhorst 2015-06-15  11805  {
ab1d3a0e Maarten Lankhorst 2015-11-19  11806struct intel_crtc_state 
*pipe_config = to_intel_crtc_state(crtc_state);
da20eabd Maarten Lankhorst 2015-06-15  11807struct drm_crtc *crtc = 
crtc_state->crtc;
da20eabd Maarten Lankhorst 2015-06-15  11808struct intel_crtc *intel_crtc = 
to_intel_crtc(crtc);
da20eabd Maarten Lankhorst 2015-06-15  11809struct drm_plane *plane = 
plane_state->plane;
da20eabd Maarten Lankhorst 2015-06-15  11810struct drm_device *dev = 
crtc->dev;
da20eabd Maarten Lankhorst 2015-06-15 @11811struct drm_i915_private 
*dev_priv = dev->dev_private;
da20eabd Maarten Lankhorst 2015-06-15  11812struct intel_plane_state 
*old_plane_state =
da20eabd Maarten Lankhorst 2015-06-15  11813
to_intel_plane_state(plane->state);
da20eabd Maarten Lankhorst 2015-06-15  11814int idx = 
intel_crtc->base.base.id, ret;
da20eabd Maarten Lankhorst 2015-06-15  11815int i = drm_plane_index(plane);
da20eabd Maarten Lankhorst 2015-06-15  11816bool mode_changed = 
needs_modeset(crtc_state);
da20eabd Maarten Lankhorst 2015-06-15  11817bool was_crtc_enabled = 
crtc->state->active;
da20eabd Maarten Lankhorst 2015-06-15  11818bool is_crtc_enabled = 
crtc_state->active;
da20eabd Maarten Lankhorst 2015-06-15  11819bool turn_off, turn_on, 
visible, was_visible;

:: The code at line 11811 was first introduced by commit
:: da20eabd2c69761f9dfd849985eb299e3335531f drm/i915: Split plane updates 
of crtc->atomic into a helper, v2.

:: TO: Maarten Lankhorst 
:: CC: Daniel Vetter 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits

2016-01-19 Thread Paulo Zanoni 
We unconditionally disable/update FBC even during the page flip
IOCTLs, and an unconditional disable/update at every atomic commit
touching the primary plane shouldn't impact PC state residency
noticeably. Besides, the code that checks for rotation is a good hint
that we may be forgetting something else, so let's leave all the
decisions to intel_fbc.c, making the code much safer.

Once we have the code to properly make FBC enable/update decisions
based on atomic states, with proper locking, then we'll be able to
evaluate whether it will be worth trying to optimize the cases where a
disable isn't needed.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f026ade..baab41046 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11928,6 +11928,8 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
case DRM_PLANE_TYPE_PRIMARY:
intel_crtc->atomic.pre_disable_primary = turn_off;
intel_crtc->atomic.post_enable_primary = turn_on;
+   intel_crtc->atomic.disable_fbc = true;
+   intel_crtc->atomic.update_fbc = true;
 
if (turn_off) {
/*
@@ -11939,28 +11941,9 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
 * disable.
 */
intel_crtc->atomic.disable_ips = true;
-
-   intel_crtc->atomic.disable_fbc = true;
}
 
/*
-* FBC does not work on some platforms for rotated
-* planes, so disable it when rotation is not 0 and
-* update it when rotation is set back to 0.
-*
-* FIXME: This is redundant with the fbc update done in
-* the primary plane enable function except that that
-* one is done too late. We eventually need to unify
-* this.
-*/
-
-   if (visible &&
-   INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
-   dev_priv->fbc.crtc == intel_crtc &&
-   plane_state->rotation != BIT(DRM_ROTATE_0))
-   intel_crtc->atomic.disable_fbc = true;
-
-   /*
 * BDW signals flip done immediately if the plane
 * is disabled, even if the plane enable is already
 * armed to occur at the next vblank :(
@@ -11968,7 +11951,6 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
if (turn_on && IS_BROADWELL(dev))
intel_crtc->atomic.wait_vblank = true;
 
-   intel_crtc->atomic.update_fbc |= visible || mode_changed;
break;
case DRM_PLANE_TYPE_CURSOR:
break;
-- 
2.6.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx