Re: [Intel-gfx] [RFC 1/3] drm/i915: remove !enabled handling from commit primary plane step

2014-09-10 Thread Daniel Vetter
On Tue, Sep 09, 2014 at 06:58:47PM +0300, Ville Syrjälä wrote:
 On Tue, Sep 09, 2014 at 11:43:19AM -0300, Gustavo Padovan wrote:
  From: Gustavo Padovan gustavo.pado...@collabora.co.uk
  
  The !crtc-enabled case will now be handled by the !visible code,
  since the handling is basically the same.
  
  Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
  ---
   drivers/gpu/drm/i915/intel_display.c | 26 --
   1 file changed, 26 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 5279b99..2ccf7c0 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -11837,32 +11837,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
  struct drm_rect *src = state-src;
  int ret;
   
  -   /*
  -* If the CRTC isn't enabled, we're just pinning the framebuffer,
  -* updating the fb pointer, and returning without touching the
  -* hardware.  This allows us to later do a drmModeSetCrtc with fb=-1 to
  -* turn on the display with all planes setup as desired.
  -*/
  -   if (!crtc-enabled) {
  -   mutex_lock(dev-struct_mutex);
  -
  -   /*
  -* If we already called setplane while the crtc was disabled,
  -* we may have an fb pinned; unpin it.
  -*/
  -   if (plane-fb)
  -   intel_unpin_fb_obj(old_obj);
  -
  -   i915_gem_track_fb(old_obj, obj,
  - INTEL_FRONTBUFFER_PRIMARY(intel_crtc-pipe));
  -
  -   /* Pin and return without programming hardware */
  -   ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
  -   mutex_unlock(dev-struct_mutex);
  -
  -   return ret;
  -   }
  -
  intel_crtc_wait_for_pending_flips(crtc);
 
 Yeah this should work just fine.
 
 One difference between the code paths is the 
 intel_crtc_wait_for_pending_flips()
 call, but since the crtc isn't enabled there can't be any pending flip.
 
 The other difference is the pin vs. unpin order, but that shouldn't matter
 unless there's tons of other stuff pinned as well. It's not worth optimizing
 setplane calls on disabled CRTCs too much IMO.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [RFC 1/3] drm/i915: remove !enabled handling from commit primary plane step

2014-09-09 Thread Gustavo Padovan
From: Gustavo Padovan gustavo.pado...@collabora.co.uk

The !crtc-enabled case will now be handled by the !visible code,
since the handling is basically the same.

Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
 drivers/gpu/drm/i915/intel_display.c | 26 --
 1 file changed, 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5279b99..2ccf7c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11837,32 +11837,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_rect *src = state-src;
int ret;
 
-   /*
-* If the CRTC isn't enabled, we're just pinning the framebuffer,
-* updating the fb pointer, and returning without touching the
-* hardware.  This allows us to later do a drmModeSetCrtc with fb=-1 to
-* turn on the display with all planes setup as desired.
-*/
-   if (!crtc-enabled) {
-   mutex_lock(dev-struct_mutex);
-
-   /*
-* If we already called setplane while the crtc was disabled,
-* we may have an fb pinned; unpin it.
-*/
-   if (plane-fb)
-   intel_unpin_fb_obj(old_obj);
-
-   i915_gem_track_fb(old_obj, obj,
- INTEL_FRONTBUFFER_PRIMARY(intel_crtc-pipe));
-
-   /* Pin and return without programming hardware */
-   ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
-   mutex_unlock(dev-struct_mutex);
-
-   return ret;
-   }
-
intel_crtc_wait_for_pending_flips(crtc);
 
/*
-- 
1.9.3

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


Re: [Intel-gfx] [RFC 1/3] drm/i915: remove !enabled handling from commit primary plane step

2014-09-09 Thread Ville Syrjälä
On Tue, Sep 09, 2014 at 11:43:19AM -0300, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
 The !crtc-enabled case will now be handled by the !visible code,
 since the handling is basically the same.
 
 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 ---
  drivers/gpu/drm/i915/intel_display.c | 26 --
  1 file changed, 26 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 5279b99..2ccf7c0 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -11837,32 +11837,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
   struct drm_rect *src = state-src;
   int ret;
  
 - /*
 -  * If the CRTC isn't enabled, we're just pinning the framebuffer,
 -  * updating the fb pointer, and returning without touching the
 -  * hardware.  This allows us to later do a drmModeSetCrtc with fb=-1 to
 -  * turn on the display with all planes setup as desired.
 -  */
 - if (!crtc-enabled) {
 - mutex_lock(dev-struct_mutex);
 -
 - /*
 -  * If we already called setplane while the crtc was disabled,
 -  * we may have an fb pinned; unpin it.
 -  */
 - if (plane-fb)
 - intel_unpin_fb_obj(old_obj);
 -
 - i915_gem_track_fb(old_obj, obj,
 -   INTEL_FRONTBUFFER_PRIMARY(intel_crtc-pipe));
 -
 - /* Pin and return without programming hardware */
 - ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
 - mutex_unlock(dev-struct_mutex);
 -
 - return ret;
 - }
 -
   intel_crtc_wait_for_pending_flips(crtc);

Yeah this should work just fine.

One difference between the code paths is the intel_crtc_wait_for_pending_flips()
call, but since the crtc isn't enabled there can't be any pending flip.

The other difference is the pin vs. unpin order, but that shouldn't matter
unless there's tons of other stuff pinned as well. It's not worth optimizing
setplane calls on disabled CRTCs too much IMO.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

  
   /*
 -- 
 1.9.3
 
 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx