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
>
> -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
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
