On 7 November 2017 at 12:06, Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
> Add Plane Degamma as a blob property and plane
> degamma size as a range property.
>
> v2: Rebase
>
Hi Uma, seems like something has gone wrong during the rebase.
> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic.c | 12 ++++++++++++
> drivers/gpu/drm/drm_atomic_helper.c | 6 ++++++
> drivers/gpu/drm/drm_mode_config.c | 14 ++++++++++++++
> include/drm/drm_mode_config.h | 11 +++++++++++
> include/drm/drm_plane.h | 10 ++++++++++
> 5 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7e0e7be..30853c7 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -713,6 +713,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> {
> struct drm_device *dev = plane->dev;
> struct drm_mode_config *config = &dev->mode_config;
> + bool replaced = false;
> + int ret;
>
> if (property == config->prop_fb_id) {
> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -758,6 +760,12 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> } else if (plane->funcs->atomic_set_property) {
> return plane->funcs->atomic_set_property(plane, state,
> property, val);
> + } else if (property == config->plane_degamma_lut_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->degamma_lut,
> + val, -1, &replaced);
> + state->color_mgmt_changed |= replaced;
> + return ret;
Namely: the driver specific atomic_set_property will be called and the
newly added code will not be reached.
I think we should keep the atomic_set_property call last in the
if/else chain. Converting the lot to a switch statement might make
things a bit more obvious.
> } else {
> return -EINVAL;
> }
> @@ -816,6 +824,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> *val = state->zpos;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state, property, val);
> + } else if (property == config->plane_degamma_lut_property) {
> + *val = (state->degamma_lut) ?
> + state->degamma_lut->base.id : 0;
Analogous thing happens here.
Did you test the updated series through IGT - it should have caught
the above (considering we have tests, and I'm not loosing my marbles).
Same comments apply for CTM and gamma, patches 2 and 3 respectively.
HTH
Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
