public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/6] drm/atomic: Unconditionally call prepare_fb.
Date: Wed, 14 Dec 2016 02:12:12 +0200	[thread overview]
Message-ID: <1509754.taGcj52Vdd@avalon> (raw)
In-Reply-To: <d75392f3-7359-7ad3-b9d6-60822b7b0c70@linux.intel.com>

Hi Maarten,

On Tuesday 13 Dec 2016 15:13:54 Maarten Lankhorst wrote:
> Op 09-12-16 om 09:25 schreef Daniel Vetter:
> > On Fri, Dec 09, 2016 at 12:42:19AM +0200, Laurent Pinchart wrote:
> >> On Thursday 08 Dec 2016 16:41:04 Daniel Vetter wrote:
> >>> On Thu, Dec 08, 2016 at 02:45:25PM +0100, Maarten Lankhorst wrote:
> >>>> Atomic drivers may set properties like rotation on the same fb, which
> >>>> may require a call to prepare_fb even when framebuffer stays identical.
> >>>> 
> >>>> Instead of handling all the special cases in the core, let the driver
> >>>> decide when prepare_fb and cleanup_fb are noops.
> >>>> 
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> 
> >>> I think this makes sense, but would be really good to get a pile of acks
> >>> from driver maintainers on this one. Rob, Eric, Laurent, others?
> >> 
> >> This is all very nice, but it will introduce at least a performance
> >> regression, and possibly worse, until drivers get updated. There are 7
> >> drivers implementing the .prepare_fb() callback (plus a bunch of drivers
> >> that probably should use drm_fb_cma_prepare_fb() but don't at the
> >> moment). I can't ack this patch before they get fixed.
> > 
> > Maarten's commit message is insufficient, since this is defacto a revert
> > of
> > 
> > commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba
> > Author: Keith Packard <keithp@keithp.com>
> > Date:   Sat Jun 4 01:16:22 2016 -0700
> > 
> >     drm: Don't prepare or cleanup unchanging frame buffers [v3]
> > 
> > because that breaks stuff. We're simply going back to where we've been a
> > few months ago. Since this is a regression fix, back to original
> > behaviour, can you ack (assuming Maarten updates the commit message to
> > reflect the nature of the commit here)?
> 
> Waiting on a reply, but what about this commit message for this patch?
> ---
> Atomic drivers may set properties like rotation on the same fb, which
> may require a call to prepare_fb even when framebuffer stays identical.
> 
> Instead of handling all the special cases in the core, let the driver
> decide when prepare_fb and cleanup_fb are noops.
> 
> This is a revert of:
> 
> commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba
> Author: Keith Packard <keithp@keithp.com>
> Date:   Sat Jun 4 01:16:22 2016 -0700
> 
>     drm: Don't prepare or cleanup unchanging frame buffers [v3]
> 
> The original commit mentions that this prevents waiting in i915 on all
> previous rendering during cursor updates, but there are better ways to
> fix this.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2016-12-14  0:12 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08 13:45 [PATCH 0/6] drm/atomic: Remove drm_atomic_helper_framebuffer_changed Maarten Lankhorst
2016-12-08 13:45 ` [PATCH 1/6] drm/atomic: Use active instead of enable in wait_for_vblanks Maarten Lankhorst
2016-12-08 15:36   ` Daniel Vetter
2016-12-08 13:45 ` [PATCH 2/6] drm/atomic: Unconditionally call prepare_fb Maarten Lankhorst
2016-12-08 15:41   ` Daniel Vetter
2016-12-08 22:42     ` [Intel-gfx] " Laurent Pinchart
2016-12-09  8:25       ` Daniel Vetter
2016-12-13 14:13         ` Maarten Lankhorst
2016-12-13 17:10           ` Daniel Vetter
2016-12-19 11:08             ` [PATCH v2 " Maarten Lankhorst
2016-12-19 12:07               ` Laurent Pinchart
2016-12-14  0:12           ` Laurent Pinchart [this message]
2016-12-08 13:45 ` [PATCH 3/6] drm/atomic: Clean up wait_for_vblanks Maarten Lankhorst
2016-12-08 15:38   ` Daniel Vetter
2016-12-15 11:51     ` [PATCH v2 3/6] drm/atomic: Clean up wait_for_vblanks, v2 Maarten Lankhorst
2016-12-15 15:55       ` Daniel Vetter
2016-12-15 16:14         ` Maarten Lankhorst
2016-12-08 13:45 ` [PATCH 4/6] drm/atomic: Wait for vblank whenever a plane is added to state Maarten Lankhorst
2016-12-08 15:43   ` Daniel Vetter
2016-12-12 10:27     ` Maarten Lankhorst
2016-12-08 13:45 ` [PATCH 5/6] drm/atomic: Remove drm_atomic_helper_framebuffer_changed Maarten Lankhorst
2016-12-08 13:45 ` [PATCH 6/6] drm/i915: Add a cursor hack to allow converting legacy page flip to atomic Maarten Lankhorst
2016-12-08 14:07   ` Matthew Auld
2016-12-12 10:34     ` [PATCH v2 6/6] drm/i915: Add a cursor hack to allow converting legacy page flip to atomic, v3 Maarten Lankhorst
2016-12-13 13:01       ` Archit Taneja
2016-12-13 13:52         ` Maarten Lankhorst
2016-12-14  3:19           ` Archit Taneja
2016-12-20  9:26       ` Daniel Vetter
2016-12-08 15:52 ` ✓ Fi.CI.BAT: success for drm/atomic: Remove drm_atomic_helper_framebuffer_changed Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1509754.taGcj52Vdd@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox