| Message ID | 1410273801-20300-1-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 09, 2014 at 11:43:19AM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@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.padovan@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.syrjala@linux.intel.com> > > /* > -- > 1.9.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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.padovan@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.padovan@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.syrjala@linux.intel.com> Queued for -next, thanks for the patch. -Daniel
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); /*