[1/5] drm/komeda: Remove clock ratio property
diff mbox series

Message ID 20190705121006.26085-1-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • [1/5] drm/komeda: Remove clock ratio property
Related show

Commit Message

Daniel Vetter July 5, 2019, 12:10 p.m. UTC
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.

From the discussion with Liviu the solution for these here needs
multiple pieces:

- For being able to reliably read the memory clock we need a DT
  property, plus maybe DT override snippets to fix it if it's wrong.

- For exposing plane limitations to userspace there's TEST_ONLY. There
  is a bit a gap in telling userspace better that scaling doesn't work
  due to limits (atm a good strategy is to retry again without scaling
  when adding a plane didn't work the first time around). But that
  needs a more generic solution, not exposing something extremely
  komeda specific.

- If this is needed by validation tools, you can still expose it in
  debugfs. We have an entire nice infrastructure for debug printing of
  kms objects already, see the various atomic_print_state callbacks
  and infrastructure around them.

Fixes: 1f7f9ab7900e ("drm/komeda: Add engine clock requirement check for the downscaling")
Cc: Lowry Li (Arm Technology China) <lowry.li@arm.com>
Cc: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Mali DP Maintainers <malidp@foss.arm.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 39 -------------------
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |  3 --
 2 files changed, 42 deletions(-)

Comments

james qian wang (Arm Technology China) July 9, 2019, 8:06 a.m. UTC | #1
On Fri, Jul 05, 2019 at 02:10:02PM +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.
> 
> >From the discussion with Liviu the solution for these here needs
> multiple pieces:
> 
> - For being able to reliably read the memory clock we need a DT
>   property, plus maybe DT override snippets to fix it if it's wrong.
> 
> - For exposing plane limitations to userspace there's TEST_ONLY. There
>   is a bit a gap in telling userspace better that scaling doesn't work
>   due to limits (atm a good strategy is to retry again without scaling
>   when adding a plane didn't work the first time around). But that
>   needs a more generic solution, not exposing something extremely
>   komeda specific.
> 
> - If this is needed by validation tools, you can still expose it in
>   debugfs. We have an entire nice infrastructure for debug printing of
>   kms objects already, see the various atomic_print_state callbacks
>   and infrastructure around them.
> 
> Fixes: 1f7f9ab7900e ("drm/komeda: Add engine clock requirement check for the downscaling")
> Cc: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> Cc: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Mali DP Maintainers <malidp@foss.arm.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Hi Daniel:
Thank you for the patch!

Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>

> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 39 -------------------
>  .../gpu/drm/arm/display/komeda/komeda_kms.h   |  3 --
>  2 files changed, 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 3f222f464eb2..e852dc27f1b8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -454,24 +454,6 @@ static void komeda_crtc_vblank_disable(struct drm_crtc *crtc)
>  	mdev->funcs->on_off_vblank(mdev, kcrtc->master->id, false);
>  }
>  
> -static int
> -komeda_crtc_atomic_get_property(struct drm_crtc *crtc,
> -				const struct drm_crtc_state *state,
> -				struct drm_property *property, uint64_t *val)
> -{
> -	struct komeda_crtc *kcrtc = to_kcrtc(crtc);
> -	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(state);
> -
> -	if (property == kcrtc->clock_ratio_property) {
> -		*val = kcrtc_st->clock_ratio;
> -	} else {
> -		DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
>  static const struct drm_crtc_funcs komeda_crtc_funcs = {
>  	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
>  	.destroy		= drm_crtc_cleanup,
> @@ -482,7 +464,6 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
>  	.atomic_destroy_state	= komeda_crtc_atomic_destroy_state,
>  	.enable_vblank		= komeda_crtc_vblank_enable,
>  	.disable_vblank		= komeda_crtc_vblank_disable,
> -	.atomic_get_property	= komeda_crtc_atomic_get_property,
>  };
>  
>  int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> @@ -518,22 +499,6 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>  	return 0;
>  }
>  
> -static int komeda_crtc_create_clock_ratio_property(struct komeda_crtc *kcrtc)
> -{
> -	struct drm_crtc *crtc = &kcrtc->base;
> -	struct drm_property *prop;
> -
> -	prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_ATOMIC,
> -					 "CLOCK_RATIO", 0, U64_MAX);
> -	if (!prop)
> -		return -ENOMEM;
> -
> -	drm_object_attach_property(&crtc->base, prop, 0);
> -	kcrtc->clock_ratio_property = prop;
> -
> -	return 0;
> -}
> -
>  static int komeda_crtc_create_slave_planes_property(struct komeda_crtc *kcrtc)
>  {
>  	struct drm_crtc *crtc = &kcrtc->base;
> @@ -590,10 +555,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>  
>  	crtc->port = kcrtc->master->of_output_port;
>  
> -	err = komeda_crtc_create_clock_ratio_property(kcrtc);
> -	if (err)
> -		return err;
> -
>  	err = komeda_crtc_create_slave_planes_property(kcrtc);
>  	if (err)
>  		return err;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index 219fa3f0c336..2775f34bf4ab 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -95,9 +95,6 @@ struct komeda_crtc {
>  	/** @disable_done: this flip_done is for tracing the disable */
>  	struct completion *disable_done;
>  
> -	/** @clock_ratio_property: property for ratio of (aclk << 32)/pxlclk */
> -	struct drm_property *clock_ratio_property;
> -
>  	/** @slave_planes_property: property for slaves of the planes */
>  	struct drm_property *slave_planes_property;
>  };
> -- 
> 2.20.1

Patch
diff mbox series

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 3f222f464eb2..e852dc27f1b8 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -454,24 +454,6 @@  static void komeda_crtc_vblank_disable(struct drm_crtc *crtc)
 	mdev->funcs->on_off_vblank(mdev, kcrtc->master->id, false);
 }
 
-static int
-komeda_crtc_atomic_get_property(struct drm_crtc *crtc,
-				const struct drm_crtc_state *state,
-				struct drm_property *property, uint64_t *val)
-{
-	struct komeda_crtc *kcrtc = to_kcrtc(crtc);
-	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(state);
-
-	if (property == kcrtc->clock_ratio_property) {
-		*val = kcrtc_st->clock_ratio;
-	} else {
-		DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static const struct drm_crtc_funcs komeda_crtc_funcs = {
 	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
 	.destroy		= drm_crtc_cleanup,
@@ -482,7 +464,6 @@  static const struct drm_crtc_funcs komeda_crtc_funcs = {
 	.atomic_destroy_state	= komeda_crtc_atomic_destroy_state,
 	.enable_vblank		= komeda_crtc_vblank_enable,
 	.disable_vblank		= komeda_crtc_vblank_disable,
-	.atomic_get_property	= komeda_crtc_atomic_get_property,
 };
 
 int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
@@ -518,22 +499,6 @@  int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
 	return 0;
 }
 
-static int komeda_crtc_create_clock_ratio_property(struct komeda_crtc *kcrtc)
-{
-	struct drm_crtc *crtc = &kcrtc->base;
-	struct drm_property *prop;
-
-	prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_ATOMIC,
-					 "CLOCK_RATIO", 0, U64_MAX);
-	if (!prop)
-		return -ENOMEM;
-
-	drm_object_attach_property(&crtc->base, prop, 0);
-	kcrtc->clock_ratio_property = prop;
-
-	return 0;
-}
-
 static int komeda_crtc_create_slave_planes_property(struct komeda_crtc *kcrtc)
 {
 	struct drm_crtc *crtc = &kcrtc->base;
@@ -590,10 +555,6 @@  static int komeda_crtc_add(struct komeda_kms_dev *kms,
 
 	crtc->port = kcrtc->master->of_output_port;
 
-	err = komeda_crtc_create_clock_ratio_property(kcrtc);
-	if (err)
-		return err;
-
 	err = komeda_crtc_create_slave_planes_property(kcrtc);
 	if (err)
 		return err;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 219fa3f0c336..2775f34bf4ab 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -95,9 +95,6 @@  struct komeda_crtc {
 	/** @disable_done: this flip_done is for tracing the disable */
 	struct completion *disable_done;
 
-	/** @clock_ratio_property: property for ratio of (aclk << 32)/pxlclk */
-	struct drm_property *clock_ratio_property;
-
 	/** @slave_planes_property: property for slaves of the planes */
 	struct drm_property *slave_planes_property;
 };