On Fri, Feb 21, 2020 at 05:40:31PM +0200, Ville Syrjälä wrote:
On Fri, Feb 21, 2020 at 03:42:56PM +0100, Daniel Vetter wrote:
On Fri, Feb 21, 2020 at 12:43 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Feb 21, 2020 at 01:32:29PM +0200, Jani Nikula wrote:
On Thu, 20 Feb 2020, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
Looks like getting rid of private_flags is going to be pretty straightforward. I'll post patches for that once this first series lands.
Going all in on crtc state? I suppose migrating away from private_flags could easily start in i915 before that. Seems rather independent.
I guess it's __intel_get_crtc_scanline() and:
vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)]; mode = &vblank->hwmode; if (mode->private_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)that gives me the creeps in reviewing all that.
There's also [1] adding new uses for private_flags; I think there were issues in getting at the right crtc state on some of those paths, but I forget the exact details. Ideas?
I'm just going to move them to the crtc_state and put a copy into the crtc itself for the vblank code. Pretty much a 1:1 replacement. Saves me from having to think ;)
I've looked through the patches, and didn't spot any place where we couldn't just get at the full crtc state. Might need some crtc->state dereferencing and upcasting and making sure stuff is ordered correctly with enable/disable paths of crtc, but nothing to jump over.
swap_state() could easily race with the irq handler. I guess practically unlikely the old crtc state would disappear before the irq handler is done, but still seems somewhat dubious.
And I guess the bigger problem is that swap_state() happens way too early. So crtc->state would be pointing to bogus stuff while we're disabling the crtc.