On Thu, Nov 06, 2014 at 09:04:14AM +0000, Chris Wilson wrote:
> On Wed, Nov 05, 2014 at 02:26:06PM -0800, Jesse Barnes wrote:
> > +static int intel_set_mode(struct drm_crtc *crtc,
> > + struct drm_display_mode *mode,
> > + int x, int y, struct drm_framebuffer *fb)
> > +{
> > + struct intel_crtc_config *pipe_config;
> > + unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > +
> > + pipe_config = intel_modeset_compute_config(crtc, mode, fb,
> > + &modeset_pipes,
> > + &prepare_pipes,
> > + &disable_pipes);
> > +
> > + if (IS_ERR(pipe_config))
> > + return PTR_ERR(pipe_config);
> > +
> > + return intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
> > + modeset_pipes, prepare_pipes,
> > + disable_pipes);
> > +}
>
> intel_set_mode() -> intel_set_mode_pipes() -> __intel_set_mode() wins
> this morning's prize for causing confusion.
>
> Does it make sense to wrap intel_crtc_config + pipes in a new struct to
> avoid passing 4 new parameters down the chain? Or will that just be
> extra churn to be rewritten by atomic?
Atomic has one big structure to track all the updated per-object states
(for crtcs, planes & connectors). Atm there's no provisions for
subclassing it, but would be trivial to add. Then we could throw all this
additional i915 state in there. I guess we might as well start with that.
One funny problem btw is all these ->new_ pointers we sprinkle all over
the place. Upstream atomic has completely free-standing new state objects,
with a bunch of helpers to fetch the new state for a given object from the
overall atomic update structure. So we have a bit of an impendance
mismatch. But I think we just need to handle that by setting (and
resetting when we don't actually commit the new state) these pointers when
entering the i915 atomic backend.
-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
