Reviewed-by: Mika Kahola <mika.kah...@intel.com>
On Thu, 2017-10-12 at 13:54 +0200, Maarten Lankhorst wrote:
> With the replace_prop_blob functions we can safely replace the blob
> for
> any property, without having to care about error handling ourselves.
>
> This will for example allow overriding color management blobs, or for
> kms_atomic set invalid mode blobs.
>
> The color management blob functions are removed, they can now be
> replaced by direct calls to replace the properties.
>
> Changes since v1:
> - Fix chamelium tests.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> ---
> lib/igt_kms.c | 117 +++++++++++++++++++++++++++++++++++++++-----
> ----------
> lib/igt_kms.h | 19 +++++++--
> tests/chamelium.c | 6 +--
> tests/kms_color.c | 12 +++---
> 4 files changed, 110 insertions(+), 44 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e7b9fbaba709..5f54021350ac 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2443,8 +2443,88 @@ static int igt_output_commit(igt_output_t
> *output,
> return 0;
> }
>
> -static void
> -igt_pipe_replace_blob(igt_pipe_t *pipe, enum
> igt_atomic_crtc_properties prop, void *ptr, size_t length)
> +/**
> + * igt_plane_replace_prop_blob:
> + * @plane: plane to set property on.
> + * @prop: property for which the blob will be replaced.
> + * @ptr: Pointer to contents for the property.
> + * @length: Length of contents.
> + *
> + * This function will destroy the old property blob for the given
> property,
> + * and will create a new property blob with the values passed to
> this function.
> + *
> + * The new property blob will be committed when you call
> igt_display_commit(),
> + * igt_display_commit2() or igt_display_commit_atomic().
> + */
> +void
> +igt_plane_replace_prop_blob(igt_plane_t *plane, enum
> igt_atomic_plane_properties prop, const void *ptr, size_t length)
> +{
> + igt_display_t *display = plane->pipe->display;
> + uint64_t *blob = &plane->values[prop];
> + uint32_t blob_id = 0;
> +
> + if (*blob != 0)
> + igt_assert(drmModeDestroyPropertyBlob(display-
> >drm_fd,
> + *blob) == 0);
> +
> + if (length > 0)
> + igt_assert(drmModeCreatePropertyBlob(display-
> >drm_fd,
> + ptr, length,
> &blob_id) == 0);
> +
> + *blob = blob_id;
> + igt_plane_set_prop_changed(plane, prop);
> +}
> +
> +/**
> + * igt_output_replace_prop_blob:
> + * @output: output to set property on.
> + * @prop: property for which the blob will be replaced.
> + * @ptr: Pointer to contents for the property.
> + * @length: Length of contents.
> + *
> + * This function will destroy the old property blob for the given
> property,
> + * and will create a new property blob with the values passed to
> this function.
> + *
> + * The new property blob will be committed when you call
> igt_display_commit(),
> + * igt_display_commit2() or igt_display_commit_atomic().
> + */
> +void
> +igt_output_replace_prop_blob(igt_output_t *output, enum
> igt_atomic_connector_properties prop, const void *ptr, size_t length)
> +{
> + igt_display_t *display = output->display;
> + uint64_t *blob = &output->values[prop];
> + uint32_t blob_id = 0;
> +
> + if (*blob != 0)
> + igt_assert(drmModeDestroyPropertyBlob(display-
> >drm_fd,
> + *blob) == 0);
> +
> + if (length > 0)
> + igt_assert(drmModeCreatePropertyBlob(display-
> >drm_fd,
> + ptr, length,
> &blob_id) == 0);
> +
> + *blob = blob_id;
> + igt_output_set_prop_changed(output, prop);
> +}
> +
> +/**
> + * igt_pipe_obj_replace_prop_blob:
> + * @pipe: pipe to set property on.
> + * @prop: property for which the blob will be replaced.
> + * @ptr: Pointer to contents for the property.
> + * @length: Length of contents.
> + *
> + * This function will destroy the old property blob for the given
> property,
> + * and will create a new property blob with the values passed to
> this function.
> + *
> + * The new property blob will be committed when you call
> igt_display_commit(),
> + * igt_display_commit2() or igt_display_commit_atomic().
> + *
> + * Please use igt_output_override_mode() if you want to set
> #IGT_CRTC_MODE_ID,
> + * it works better with legacy commit.
> + */
> +void
> +igt_pipe_obj_replace_prop_blob(igt_pipe_t *pipe, enum
> igt_atomic_crtc_properties prop, const void *ptr, size_t length)
> {
> igt_display_t *display = pipe->display;
> uint64_t *blob = &pipe->values[prop];
> @@ -2836,7 +2916,7 @@ void igt_output_override_mode(igt_output_t
> *output, drmModeModeInfo *mode)
>
> if (pipe) {
> if (output->display->is_atomic)
> - igt_pipe_replace_blob(pipe,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
> + igt_pipe_obj_replace_prop_blob(pipe,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
> else
> igt_pipe_obj_set_prop_changed(pipe,
> IGT_CRTC_MODE_ID);
> }
> @@ -2865,7 +2945,7 @@ void igt_output_set_pipe(igt_output_t *output,
> enum pipe pipe)
> old_output = igt_pipe_get_output(old_pipe);
> if (!old_output) {
> if (display->is_atomic)
> - igt_pipe_replace_blob(old_pipe,
> IGT_CRTC_MODE_ID, NULL, 0);
> + igt_pipe_obj_replace_prop_blob(old_p
> ipe, IGT_CRTC_MODE_ID, NULL, 0);
> else
> igt_pipe_obj_set_prop_changed(old_pi
> pe, IGT_CRTC_MODE_ID);
>
> @@ -2879,7 +2959,7 @@ void igt_output_set_pipe(igt_output_t *output,
> enum pipe pipe)
>
> if (pipe_obj) {
> if (display->is_atomic)
> - igt_pipe_replace_blob(pipe_obj,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output),
> sizeof(drmModeModeInfo));
> + igt_pipe_obj_replace_prop_blob(pipe_obj,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output),
> sizeof(drmModeModeInfo));
> else
> igt_pipe_obj_set_prop_changed(pipe_obj,
> IGT_CRTC_MODE_ID);
>
> @@ -2908,18 +2988,11 @@ void igt_pipe_refresh(igt_display_t *display,
> enum pipe pipe, bool force)
>
> pipe_obj->values[IGT_CRTC_MODE_ID] = 0;
> if (output)
> - igt_pipe_replace_blob(pipe_obj,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output),
> sizeof(drmModeModeInfo));
> + igt_pipe_obj_replace_prop_blob(pipe_obj,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output),
> sizeof(drmModeModeInfo));
> } else
> igt_pipe_obj_set_prop_changed(pipe_obj,
> IGT_CRTC_MODE_ID);
> }
>
> -void igt_output_set_scaling_mode(igt_output_t *output, uint64_t
> scaling_mode)
> -{
> - igt_output_set_prop_value(output,
> IGT_CONNECTOR_SCALING_MODE, scaling_mode);
> -
> - igt_require(output->props[IGT_CONNECTOR_SCALING_MODE]);
> -}
> -
> igt_plane_t *igt_output_get_plane(igt_output_t *output, int
> plane_idx)
> {
> igt_pipe_t *pipe;
> @@ -3119,24 +3192,6 @@ void igt_pipe_request_out_fence(igt_pipe_t
> *pipe)
> igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR,
> (ptrdiff_t)&pipe->out_fence_fd);
> }
>
> -void
> -igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> -{
> - igt_pipe_replace_blob(pipe, IGT_CRTC_DEGAMMA_LUT, ptr,
> length);
> -}
> -
> -void
> -igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length)
> -{
> - igt_pipe_replace_blob(pipe, IGT_CRTC_CTM, ptr, length);
> -}
> -
> -void
> -igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> -{
> - igt_pipe_replace_blob(pipe, IGT_CRTC_GAMMA_LUT, ptr,
> length);
> -}
> -
> /**
> * igt_crtc_set_background:
> * @pipe: pipe pointer to which background color to be set
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index b53127ffef5f..2e4c23437f4e 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -366,7 +366,6 @@ const char *igt_output_name(igt_output_t
> *output);
> drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
> void igt_output_override_mode(igt_output_t *output, drmModeModeInfo
> *mode);
> void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
> -void igt_output_set_scaling_mode(igt_output_t *output, uint64_t
> scaling_mode);
> igt_plane_t *igt_output_get_plane(igt_output_t *output, int
> plane_idx);
> igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int
> plane_type);
> igt_output_t *igt_output_from_connector(igt_display_t *display,
> @@ -381,9 +380,6 @@ static inline bool
> igt_plane_supports_rotation(igt_plane_t *plane)
> return plane->props[IGT_PLANE_ROTATION] != 0;
> }
> void igt_pipe_request_out_fence(igt_pipe_t *pipe);
> -void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t
> length);
> -void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t
> length);
> -void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t
> length);
>
> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
> void igt_plane_set_fence_fd(igt_plane_t *plane, int fence_fd);
> @@ -513,6 +509,10 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
> igt_plane_set_prop_changed(plane, prop); \
> } while (0)
>
> +extern void igt_plane_replace_prop_blob(igt_plane_t *plane,
> + enum
> igt_atomic_plane_properties prop,
> + const void *ptr, size_t
> length);
> +
> #define igt_output_is_prop_changed(output, prop) \
> (!!((output)->changed & (1 << (prop))))
> #define igt_output_set_prop_changed(output, prop) \
> @@ -527,6 +527,10 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
> igt_output_set_prop_changed(output, prop); \
> } while (0)
>
> +extern void igt_output_replace_prop_blob(igt_output_t *output,
> + enum
> igt_atomic_connector_properties prop,
> + const void *ptr, size_t
> length);
> +
> #define igt_pipe_obj_is_prop_changed(pipe_obj, prop) \
> (!!((pipe_obj)->changed & (1 << (prop))))
>
> @@ -554,6 +558,13 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
> #define igt_pipe_set_prop_value(display, pipe, prop, value) \
> igt_pipe_obj_set_prop_value(&(display)->pipes[(pipe)], prop,
> value)
>
> +extern void igt_pipe_obj_replace_prop_blob(igt_pipe_t *pipe_obj,
> + enum
> igt_atomic_crtc_properties prop,
> + const void *ptr, size_t
> length);
> +
> +#define igt_pipe_replace_prop_blob(display, pipe, prop, ptr, length)
> \
> + igt_pipe_obj_replace_prop_blob(&(display)->pipes[(pipe)],
> prop, ptr, length)
> +
> void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool
> force);
>
> void igt_enable_connectors(void);
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index e3d81357b32b..ae164920e86e 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -462,9 +462,9 @@ enable_output(data_t *data,
> igt_output_set_pipe(output, output->config.pipe);
>
> /* Clear any color correction values that might be enabled
> */
> - igt_pipe_set_degamma_lut(primary->pipe, NULL, 0);
> - igt_pipe_set_gamma_lut(primary->pipe, NULL, 0);
> - igt_pipe_set_ctm_matrix(primary->pipe, NULL, 0);
> + igt_pipe_obj_replace_prop_blob(primary->pipe,
> IGT_CRTC_DEGAMMA_LUT, NULL, 0);
> + igt_pipe_obj_replace_prop_blob(primary->pipe,
> IGT_CRTC_GAMMA_LUT, NULL, 0);
> + igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_CTM,
> NULL, 0);
>
> kmstest_set_connector_broadcast_rgb(display->drm_fd,
> connector,
> BROADCAST_RGB_FULL);
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index bcd48d89875c..e30e6bf4e409 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -189,7 +189,7 @@ static void set_degamma(data_t *data,
> data-
> >degamma_lut_size,
> data-
> >color_depth, 0);
>
> - igt_pipe_set_degamma_lut(pipe, lut, size);
> + igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_DEGAMMA_LUT,
> lut, size);
>
> free(lut);
> }
> @@ -204,7 +204,7 @@ static void set_gamma(data_t *data,
> data-
> >gamma_lut_size,
> data-
> >color_depth, 0);
>
> - igt_pipe_set_gamma_lut(pipe, lut, size);
> + igt_pipe_obj_replace_prop_blob(pipe,
> IGT_CRTC_GAMMA_LUT, lut, size);
>
> free(lut);
> }
> @@ -224,12 +224,12 @@ static void set_ctm(igt_pipe_t *pipe, const
> double *coefficients)
> (int64_t) (coefficients[i] *
> ((int64_t) 1L << 32));
> }
>
> - igt_pipe_set_ctm_matrix(pipe, &ctm, sizeof(ctm));
> + igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, &ctm,
> sizeof(ctm));
> }
>
> -#define disable_degamma(pipe) igt_pipe_set_degamma_lut(pipe, NULL,
> 0)
> -#define disable_gamma(pipe) igt_pipe_set_gamma_lut(pipe, NULL, 0)
> -#define disable_ctm(pipe) igt_pipe_set_ctm_matrix(pipe, NULL, 0)
> +#define disable_degamma(pipe) igt_pipe_obj_replace_prop_blob(pipe,
> IGT_CRTC_DEGAMMA_LUT, NULL, 0)
> +#define disable_gamma(pipe) igt_pipe_obj_replace_prop_blob(pipe,
> IGT_CRTC_GAMMA_LUT, NULL, 0)
> +#define disable_ctm(pipe) igt_pipe_obj_replace_prop_blob(pipe,
> IGT_CRTC_CTM, NULL, 0)
>
> static void output_set_property_enum(igt_output_t *output,
> const char *property,
--
Mika Kahola - Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
