> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>
> Sent: Tuesday, January 28, 2025 9:21 PM
> To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-
> devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; dmitry.baryshkov@xxxxxxxxxx;
> Murthy, Arun R <arun.r.murthy@xxxxxxxxx>
> Subject: [PATCH v8 04/14] drm/crtc: Expose API to create drm crtc property for
> IET LUT
>
> Add drm-crtc property for IET 1DLUT and for the properties added add
Nit: Fix Grammar * added. Add
Do the same in previous patch as well
> corresponding get/set_property.
>
> Signed-off-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic_state_helper.c | 9 ++++++
> drivers/gpu/drm/drm_atomic_uapi.c | 13 ++++++++
> drivers/gpu/drm/drm_crtc.c | 54
> +++++++++++++++++++++++++++++++
> include/drm/drm_crtc.h | 36 +++++++++++++++++++++
> 4 files changed, 112 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index
> dfe6293f7a42d034da3de593094019ca15014a02..ceab90cec57cc580afcf334e27
> 5982827e9b0e0d 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -149,6 +149,10 @@ void
> __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> drm_property_blob_get(state->histogram_enable);
> if (state->histogram_data)
> drm_property_blob_get(state->histogram_data);
> + if (state->iet_lut_caps)
> + drm_property_blob_get(state->iet_lut_caps);
> + if (state->iet_lut)
> + drm_property_blob_get(state->iet_lut);
> state->mode_changed = false;
> state->active_changed = false;
> state->planes_changed = false;
> @@ -164,6 +168,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct
> drm_crtc *crtc,
> state->self_refresh_active = false;
>
> state->histogram_updated = false;
> + state->iet_lut_updated = false;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
>
> @@ -229,6 +234,10 @@ void __drm_atomic_helper_crtc_destroy_state(struct
> drm_crtc_state *state)
> drm_property_blob_put(state->histogram_enable);
> if (state->histogram_data)
> drm_property_blob_put(state->histogram_data);
> + if (state->iet_lut_caps)
> + drm_property_blob_put(state->iet_lut_caps);
> + if (state->iet_lut)
> + drm_property_blob_put(state->iet_lut);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index
> 459d30898196c94392a7f916b1fa9ca3a334eea8..f31d24d80cc082b38c611b12f3
> 6f281fa7404869 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -424,6 +424,15 @@ static int drm_atomic_crtc_set_property(struct
> drm_crtc *crtc,
> &replaced);
> state->histogram_updated |= replaced;
> return ret;
> + } else if (property == crtc->iet_lut_property) {
> + ret = drm_property_replace_blob_from_id(dev,
> + &state->iet_lut,
> + val,
> + -1,
> + sizeof(struct
> drm_iet_1dlut_sample),
> + &replaced);
> + state->iet_lut_updated |= replaced;
> + return ret;
> } else if (property == crtc->scaling_filter_property) {
> state->scaling_filter = val;
> } else if (crtc->funcs->atomic_set_property) { @@ -467,6 +476,10 @@
> drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->histogram_enable) ? state->histogram_enable-
> >base.id : 0;
> else if (property == crtc->histogram_data_property)
> *val = (state->histogram_data) ? state->histogram_data-
> >base.id : 0;
> + else if (property == crtc->iet_lut_caps_property)
> + *val = (state->iet_lut_caps) ? state->iet_lut_caps->base.id : 0;
> + else if (property == crtc->iet_lut_property)
> + *val = (state->iet_lut) ? state->iet_lut->base.id : 0;
> else if (property == crtc->scaling_filter_property)
> *val = state->scaling_filter;
> else if (crtc->funcs->atomic_get_property)
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index
> d10b29aff725e40bdb93e6bd0828347db40fa3e8..850d98d7f9c8965c7a5e9ac55
> 05e355042041449 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1032,3 +1032,57 @@ int drm_crtc_create_histogram_property(struct
> drm_crtc *crtc,
> return 0;
> }
> EXPORT_SYMBOL(drm_crtc_create_histogram_property);
> +
> +/**
> + * drm_crtc_create_iet_lut_property
> + *
> + * @crtc: pointer to the struct drm_crtc.
> + * @caps: pointer to the struct drm_iet_caps, holds the
> + * image enhancement LUT hardware capabilities.
> + *
> + * This 1DLUT is used by the hardware to enahance the image. Hardware
> + * interpolates this LUT value to generate the enhanced output image.
> + *
> + * The blob property IET_LUT_CAPS points to the struct drm_iet_lut_caps
> + * The blob property IET_LUT points to the struct drm_iet_1dlut_sample
> + * Description of the structure is in include/uapi/drm/drm_mode.h
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_crtc_create_iet_lut_property(struct drm_crtc *crtc,
> + struct drm_iet_caps *caps)
> +{
> + struct drm_property *prop;
> + struct drm_iet_caps *blob_data;
> + struct drm_property_blob *blob;
> +
> + blob = drm_property_create_blob(crtc->dev,
> + sizeof(struct drm_iet_caps),
> + NULL);
> + if (IS_ERR(blob))
> + return -1;
Use PTR_ERR(blob) to return error instead of directly returning -1
And add new line after the if.
> + blob_data = blob->data;
> + blob_data->iet_mode = caps->iet_mode;
Shouldn't we be Oring this
> + blob_data->nr_iet_sample_formats = caps->nr_iet_sample_formats;
> + blob_data->nr_iet_lut_entries = caps->nr_iet_lut_entries;
> + blob_data->iet_sample_format = caps->iet_sample_format;
> +
> + prop = drm_property_create(crtc->dev, DRM_MODE_PROP_ATOMIC |
> + DRM_MODE_PROP_IMMUTABLE |
> DRM_MODE_PROP_BLOB,
> + "IET_LUT_CAPS", blob->base.id);
> + if (!prop)
> + return -ENOMEM;
Ditto
> + drm_object_attach_property(&crtc->base, prop, 0);
> + crtc->iet_lut_caps_property = prop;
> +
> + prop = drm_property_create(crtc->dev, DRM_MODE_PROP_ATOMIC |
> + DRM_MODE_PROP_BLOB, "IET_LUT", 0);
> + if (!prop)
> + return -ENOMEM;
Here too.
> + drm_object_attach_property(&crtc->base, prop, 0);
> + crtc->iet_lut_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_create_iet_lut_property);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> 2da803749bdf03c07268be4e075793ef4e4eb99a..bc85ab16d5c817773a1d8b415
> eb256d08c13c709 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -300,6 +300,29 @@ struct drm_crtc_state {
> struct drm_property_blob *histogram_data;
> bool histogram_updated;
>
> + /**
> + * @iet_lut_caps:
> + *
> + * The blob points to the structure drm_iet_lut_caps.
> + * For more info on the elements of the struct drm_iet_lut_caps
> + * see include/uapi/drm/drm_mode.h
Don’t see the need of this
> + */
> + struct drm_property_blob *iet_lut_caps;
> + /**
> + * @iet_lut:
> + *
> + * The blob points to the struct drm_lut_sample
> + * For more information on the elements of struct drm_lut_sample
> + * see include/uapi/drm/drm_mode.h
Same here
Regards,
Suraj Kandpal
> + */
> + struct drm_property_blob *iet_lut;
> + /**
> + * @iet_lut_updates:
> + *
> + * Convey that the image enhanced data has been updated by the user
> + */
> + bool iet_lut_updated;
> +
> /**
> * @target_vblank:
> *
> @@ -1130,6 +1153,17 @@ struct drm_crtc {
> */
> struct drm_property *histogram_data_property;
>
> + /**
> + * @iet_lut_caps_property: Optional CRTC property for getting the
> + * iet LUT hardware capability.
> + */
> + struct drm_property *iet_lut_caps_property;
> + /**
> + * @iet_lut_proeprty: Optional CRTC property for writing the
> + * image enhanced LUT
> + */
> + struct drm_property *iet_lut_property;
> +
> /**
> * @state:
> *
> @@ -1368,4 +1402,6 @@ int drm_crtc_create_scaling_filter_property(struct
> drm_crtc *crtc, bool drm_crtc_in_clone_mode(struct drm_crtc_state
> *crtc_state); int drm_crtc_create_histogram_property(struct drm_crtc *crtc,
> struct drm_histogram_caps *caps);
> +int drm_crtc_create_iet_lut_property(struct drm_crtc *crtc,
> + struct drm_iet_caps *caps);
> #endif /* __DRM_CRTC_H__ */
>
> --
> 2.25.1
