| Message ID | 20190705121006.26085-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] drm/komeda: Remove clock ratio property | expand |
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
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; };
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(-)