On Thu, Sep 10, 2015 at 07:36:39PM +0300, Ville Syrjälä wrote: > On Thu, Sep 10, 2015 at 06:30:20PM +0200, Daniel Vetter wrote: > > On Thu, Sep 10, 2015 at 07:20:11PM +0300, Ville Syrjälä wrote: > > > On Thu, Sep 10, 2015 at 07:13:46PM +0300, Ville Syrjälä wrote: > > > > On Thu, Sep 10, 2015 at 06:07:34PM +0200, Daniel Vetter wrote: > > > > > On Thu, Sep 10, 2015 at 06:59:09PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > > > > > > > Add a .get_hw_state() method for planes, returning true or false > > > > > > depending on whether the plane is enabled. Use it to populate the > > > > > > plane state 'visible' during state readout. > > > > > > > > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > > > > > Cc: Patrik Jakobsson <patrik.jakobsson@xxxxxxxxxxxxxxx> > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > > > > > Do we really need this? The plane readout is fairly limited and we don't > > > > > really care about recovering anything but the initial primary plane on > > > > > driver load. Anything else can just be nuked if it misfits ... > > > > > > > > At least avoids calling ->disable_plane for an already disabled plane. > > > > We could even put a WARN there to catch broken code. > > > > > > Oh and we could also do something like > > > for_each_plane() > > > assert(visible == get_hw_state); > > > > > > somewhere to perhaps catch even more crap. > > > > Yeah maybe, but I think that should be done as part of the patch series > > with this patch here. As-is I don't think it adds a lot really ... > > It would at least fix SKL/BXT primary readout, which I forgot to mention > in the commit message naturally. Yeah, please hide the bugfix less ;-) Adding ->get_hw_state shouldn't be the subject either, it should be something like "Add plane hw state readouf for skl" and the commit message then explains why you added a new vfunc. And we should have some idea what to do about the get_initial_plane_config hook - having two very similar hooks is a bit confusing imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
