On Wed, Feb 01, 2012 at 11:55:44AM +0000, Chris Wilson wrote: > On Wed, 1 Feb 2012 11:38:35 +0100, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote: > > The drivers currently check in set_property whether the property is > > unchanged. move this check into the core and do not bother the drivers > > with checking for unchanged properties. > > This patch seems to have functional side-effects beyond the description. > > For example, > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index db3b461..0024b59 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2223,9 +2223,6 @@ intel_dp_set_property(struct drm_connector *connector, > > int i = val; > > bool has_audio; > > > > - if (i == intel_dp->force_audio) > > - return 0; > > - > > Here we are checking against the current value of the intel_dp, which > may in theory be modified elsewhere as well, and avoiding the modeswitch > if it is unnecessary. In case of force_audio I just checked that the current value is not changed elsewhere, but I must admit that I haven't checked this for all other properties. Would the patch be ok if I review for side effects? BTW if someone changes the underlying variable of a property outside of the .set_property callback there are likely problems elsewhere because without calling drm_connector_property_set_value the values of the variable and the property backing store will be inconsistent. The calls to drm_connector_property_set_value in turn are quite easy reviewable, those are very few. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
