On 8/11/2014 5:12 PM, Daniel Vetter wrote:
On Mon, Aug 11, 2014 at 04:37:00PM +0530, Jindal, Sonika wrote:
Hi Daniel,
Are you taking this patch back in drm-intel?
We should still call the primary_plane->update hook directly instead of
open-coding it.
-Daniel
But we are already doing dev_priv->display.update_primary_plane. Can you
please elaborate? Last time we had a discussion on this same point, and
we thought for some platforms more work might be needed but for current
platforms this looks ok. Following was the discussion:
"> >Also Chris mentioned that on some platforms this won't work and it's
> >more future-proof to just do a full modeset until we have the proper
> >infrastructure.
> >
> Can you please elaborate on this? What needs to be done?
Apparently nothing, it turned out that on the platforms currently
supported everything is fine with your patch. Damien promised to follow
up with the details on internal mailing lists.
-Daniel
"
-Sonika
On 8/7/2014 5:41 PM, Daniel Vetter wrote:
On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:
On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jindal@xxxxxxxxx wrote:
+ /* FBC does not work on some platforms for rotated planes */
+ if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev)) {
+ if (dev_priv->fbc.plane == intel_crtc->plane &&
+ intel_plane->rotation != BIT(DRM_ROTATE_0))
+ intel_disable_fbc(dev);
+ /* If rotation was set earlier and new rotation is 0,
+ we might have disabled fbc earlier. So update it now */
+ else if (intel_plane->rotation == BIT(DRM_ROTATE_0)
+ && old_val != BIT(DRM_ROTATE_0)) {
+ mutex_lock(&dev->struct_mutex);
+ intel_update_fbc(dev);
+ mutex_unlock(&dev->struct_mutex);
+ }
+ }
Indentation is screwed up here. Also if we convert some of the checks into
early bails we could de-indent this by one level.
Also Chris mentioned that on some platforms this won't work and it's more
future-proof to just do a full modeset until we have the proper
infrastructure.
Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.
I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
to call the update_plane function (not the raw low-level one, the
high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
correctness with screen rotation. Probably best to reuse the existing
fbc testcase and add a bunch or rotated tests.
Ok, the check in update_fbc is there, I've been blind. Sorry about all the
confusion. So just amounts of calling the higher level function and we can
forgo the fbc testing.
-Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
