> > > +static int > > > +intel_atomic_replace_property_blob_from_id(struct drm_device *dev, > > > + struct drm_property_blob **blob, > > > + u64 blob_id, > > > + ssize_t expected_size, > > > + ssize_t expected_elem_size, > > > + bool *replaced) > > > +{ > > > + struct drm_property_blob *new_blob = NULL; > > > + > > > + if (blob_id != 0) { > > > + new_blob = drm_property_lookup_blob(dev, blob_id); > > > + if (!new_blob) > > > + return -EINVAL; > > > + > > > + if (expected_size > 0 && > > > + new_blob->length != expected_size) { > > > + drm_property_blob_put(new_blob); > > > + return -EINVAL; > > > + } > > > + if (expected_elem_size > 0 && > > > + new_blob->length % expected_elem_size != 0) { > > > + drm_property_blob_put(new_blob); > > > + return -EINVAL; > > > + } > > > + } > > > + > > > + *replaced |= drm_property_replace_blob(blob, new_blob); > > > + drm_property_blob_put(new_blob); > > > + > > > + return 0; > > > +} > > > + > > Can we align this design to what we have for get property with similar > > pattern of if else logic and return 0. Done! > > > +static int intel_crtc_set_property(struct drm_crtc *crtc, > > > + struct drm_crtc_state *state, > > > + struct drm_property *property, > > > + u64 val) > > > +{ > > > + struct drm_i915_private *i915 = to_i915(crtc->dev); > > > + struct intel_crtc_state *intel_crtc_state = > > > + to_intel_crtc_state(state); > > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > > + bool replaced = false; > > > + > > > + if (property == intel_crtc->histogram_en_property) { > > > + intel_crtc_state->histogram_en = val; > > > + intel_crtc_state->histogram_en_changed = true; > > > + return 0; > > > + } > > > + > > > + if (property == intel_crtc->global_iet_property) { > > > + intel_atomic_replace_property_blob_from_id(crtc->dev, > > > + &intel_crtc_state- > > > >global_iet, > > > + val, > > > + sizeof(uint32_t) * > > > HISTOGRAM_IET_LENGTH, > > > + -1, &replaced); > > > + if (replaced) > > > + intel_crtc_state->global_iet_changed = true; > > > + return 0; > > > + } > > > + > > > + drm_dbg_atomic(&i915->drm, "Unknown property > > > [PROP:%d:%s]\n", > > > + property->base.id, property->name); > > > + return -EINVAL; > > > +} > > > + > > > #define INTEL_CRTC_FUNCS \ > > > .set_config = drm_atomic_helper_set_config, \ > > > .destroy = intel_crtc_destroy, \ > > > @@ -229,7 +326,9 @@ static int intel_crtc_late_register(struct > > > drm_crtc > > > *crtc) > > > .set_crc_source = intel_crtc_set_crc_source, \ > > > .verify_crc_source = intel_crtc_verify_crc_source, \ > > > .get_crc_sources = intel_crtc_get_crc_sources, \ > > > - .late_register = intel_crtc_late_register > > > + .late_register = intel_crtc_late_register, \ > > > + .atomic_set_property = intel_crtc_set_property, \ > > > + .atomic_get_property = intel_crtc_get_property > > > > +dri-devel > > > > Can this be made drm crtc property as histogram is generic? > > If there are other drivers using this property then this can be made a drm_crtc property. Thanks and Regards, Arun R Murthy --------------------