On Fri, Jul 05, 2019 at 02:10:05PM +0200, Daniel Vetter wrote:
> Properties are uapi like anything else, with all the usual rules
> regarding review, testcases, open source userspace ... Furthermore
> driver-private kms properties are highly discouraged, over the past
> few years we've realized we need to make a serious effort at better
> standardizing this stuff.
>
> Again this probably needs multiple pieces to solve this properly:
>
> - Instead of expecting userspace to compute this (and duplicating
> modeset code), the kernel driver should compute when it's necessary
> to enable layer_split mode to make a configuration possible. I.e. in
> komeda_plane_atomic_check() first try komeda_build_layer_data_flow()
> and if that fails, try komeda_build_layer_split_data_flow(), and set
> dflow.en_split accordingly. Assuming I understand somewhat correctly
> what this does.
>
> - If this is needed for validation then you want a debugfs file to
> force this one way or the other, or alternatively use
> ->atomic_print_state to dump such hidden driver-private state.
> Depends upon how you do your validation ofc.
>
> Fixes: a407a6509393 ("drm/komeda: Add layer split support")
> Cc: Lowry Li (Arm Technology China) <lowry...@arm.com>
> Cc: James Qian Wang (Arm Technology China) <james.qian.w...@arm.com>
> Cc: Liviu Dudau <liviu.du...@arm.com>
> Cc: Mali DP Maintainers <mal...@foss.arm.com>
> Cc: Brian Starkey <brian.star...@arm.com>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Hi Daniel:
Thank you for the patch!
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.w...@arm.com>
> ---
> .../gpu/drm/arm/display/komeda/komeda_kms.h | 3 -
> .../gpu/drm/arm/display/komeda/komeda_plane.c | 61 -------------------
> 2 files changed, 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index fb2adc233760..0c006576a25c 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -33,9 +33,6 @@ struct komeda_plane {
> * Layers with same capabilities.
> */
> struct komeda_layer *layer;
> -
> - /** @prop_layer_split: for on/off layer_split */
> - struct drm_property *prop_layer_split;
> };
>
> /**
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index 23db38650a46..5bb8553cc117 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -188,40 +188,6 @@ komeda_plane_atomic_destroy_state(struct drm_plane
> *plane,
> kfree(to_kplane_st(state));
> }
>
> -static int
> -komeda_plane_atomic_get_property(struct drm_plane *plane,
> - const struct drm_plane_state *state,
> - struct drm_property *property,
> - uint64_t *val)
> -{
> - struct komeda_plane *kplane = to_kplane(plane);
> - struct komeda_plane_state *st = to_kplane_st(state);
> -
> - if (property == kplane->prop_layer_split)
> - *val = st->layer_split;
> - else
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> -static int
> -komeda_plane_atomic_set_property(struct drm_plane *plane,
> - struct drm_plane_state *state,
> - struct drm_property *property,
> - uint64_t val)
> -{
> - struct komeda_plane *kplane = to_kplane(plane);
> - struct komeda_plane_state *st = to_kplane_st(state);
> -
> - if (property == kplane->prop_layer_split)
> - st->layer_split = !!val;
> - else
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> static bool
> komeda_plane_format_mod_supported(struct drm_plane *plane,
> u32 format, u64 modifier)
> @@ -241,32 +207,9 @@ static const struct drm_plane_funcs komeda_plane_funcs =
> {
> .reset = komeda_plane_reset,
> .atomic_duplicate_state = komeda_plane_atomic_duplicate_state,
> .atomic_destroy_state = komeda_plane_atomic_destroy_state,
> - .atomic_get_property = komeda_plane_atomic_get_property,
> - .atomic_set_property = komeda_plane_atomic_set_property,
> .format_mod_supported = komeda_plane_format_mod_supported,
> };
>
> -static int
> -komeda_plane_create_layer_properties(struct komeda_plane *kplane,
> - struct komeda_layer *layer)
> -{
> - struct drm_device *drm = kplane->base.dev;
> - struct drm_plane *plane = &kplane->base;
> - struct drm_property *prop = NULL;
> -
> - /* property: layer split */
> - if (layer->right) {
> - prop = drm_property_create_bool(drm, DRM_MODE_PROP_ATOMIC,
> - "layer_split");
> - if (!prop)
> - return -ENOMEM;
> - kplane->prop_layer_split = prop;
> - drm_object_attach_property(&plane->base, prop, 0);
> - }
> -
> - return 0;
> -}
> -
> /* for komeda, which is pipeline can be share between crtcs */
> static u32 get_possible_crtcs(struct komeda_kms_dev *kms,
> struct komeda_pipeline *pipe)
> @@ -360,10 +303,6 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
> if (err)
> goto cleanup;
>
> - err = komeda_plane_create_layer_properties(kplane, layer);
> - if (err)
> - goto cleanup;
> -
> err = drm_plane_create_color_properties(plane,
> BIT(DRM_COLOR_YCBCR_BT601) |
> BIT(DRM_COLOR_YCBCR_BT709) |
> --
> 2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
