Re: [PATCH v3 09/10] drm: rcar-du: Perform group setup from the atomic tail handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On June 17, 2019 at 11:09 PM Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> wrote:
> 
> 
> From: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> 
> Create rcar_du_group_atomic_check() and rcar_du_group_atomic_setup()
> functions to track and apply group state through the DRM atomic state.
> The use_count field is moved from the rcar_du_group structure to an
> enabled field in the rcar_du_group_state structure.
> 
> This allows separating group setup from the configuration of the CRTCs
> that are part of the group, simplifying the CRTC code and improving
> overall readability. The existing rcar_du_group_{get,put}() functions
> are now redundant and removed.
> 
> The groups share clocking with the CRTCs within the group, and so are
> accessible only when one of its CRTCs has been powered through
> rcar_du_crtc_atomic_exit_standby().
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> Changes since v2:
> 
> - Simplify error handling in rcar_du_crtc_enable()
> - Rename rcar_du_group_atomic_pre_commit() to
>   rcar_du_group_atomic_setup() and turn it into a void function
> - Remove rcar_du_group_atomic_post_commit()
> - Replace group state use_count field by enabled
> - Rename group state variable from rstate to gstate
> 
> Changes since v1:
> 
> - All register sequences now maintained.
> - Clock management is no longer handled by the group
>   (_crtc_{exit,enter}_standby handles this for us)
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 18 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 91 ++++++++++++++++---------
>  drivers/gpu/drm/rcar-du/rcar_du_group.h | 12 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  5 ++
>  4 files changed, 76 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index d11a474f6f72..ab5c288f9d09 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -487,12 +487,10 @@ static int rcar_du_crtc_exit_standby(struct rcar_du_crtc *rcrtc)
>  		return ret;
>  
>  	ret = clk_prepare_enable(rcrtc->extclock);
> -	if (ret < 0)
> -		goto error_clock;
> -
> -	ret = rcar_du_group_get(rcrtc->group);
> -	if (ret < 0)
> -		goto error_group;
> +	if (ret < 0) {
> +		clk_disable_unprepare(rcrtc->clock);
> +		return ret;
> +	}
>  
>  	/* Set display off and background to black. */
>  	rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> @@ -502,18 +500,10 @@ static int rcar_du_crtc_exit_standby(struct rcar_du_crtc *rcrtc)
>  	rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>  
>  	return 0;
> -
> -error_group:
> -	clk_disable_unprepare(rcrtc->extclock);
> -error_clock:
> -	clk_disable_unprepare(rcrtc->clock);
> -	return ret;
>  }
>  
>  static void rcar_du_crtc_enter_standby(struct rcar_du_crtc *rcrtc)
>  {
> -	rcar_du_group_put(rcrtc->group);
> -
>  	clk_disable_unprepare(rcrtc->extclock);
>  	clk_disable_unprepare(rcrtc->clock);
>  }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 8e12bd42890e..7c9145778567 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -24,6 +24,7 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/err.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  
> @@ -173,38 +174,6 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  	mutex_unlock(&rgrp->lock);
>  }
>  
> -/*
> - * rcar_du_group_get - Acquire a reference to the DU channels group
> - *
> - * Acquiring the first reference setups core registers. A reference must be held
> - * before accessing any hardware registers.
> - *
> - * This function must be called with the DRM mode_config lock held.
> - *
> - * Return 0 in case of success or a negative error code otherwise.
> - */
> -int rcar_du_group_get(struct rcar_du_group *rgrp)
> -{
> -	if (rgrp->use_count)
> -		goto done;
> -
> -	rcar_du_group_setup(rgrp);
> -
> -done:
> -	rgrp->use_count++;
> -	return 0;
> -}
> -
> -/*
> - * rcar_du_group_put - Release a reference to the DU
> - *
> - * This function must be called with the DRM mode_config lock held.
> - */
> -void rcar_du_group_put(struct rcar_du_group *rgrp)
> -{
> -	--rgrp->use_count;
> -}
> -
>  static void __rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start)
>  {
>  	struct rcar_du_device *rcdu = rgrp->dev;
> @@ -389,6 +358,23 @@ static const struct drm_private_state_funcs rcar_du_group_state_funcs = {
>  	.atomic_destroy_state = rcar_du_group_atomic_destroy_state,
>  };
>  
> +#define for_each_oldnew_group_in_state(__state, __obj, __old_state, __new_state, __i) \
> +	for_each_oldnew_private_obj_in_state((__state), (__obj), (__old_state), (__new_state), (__i)) \
> +		for_each_if((__obj)->funcs == &rcar_du_group_state_funcs)
> +
> +static struct rcar_du_group_state *
> +rcar_du_get_group_state(struct drm_atomic_state *state,
> +			struct rcar_du_group *rgrp)
> +{
> +	struct drm_private_state *pstate;
> +
> +	pstate = drm_atomic_get_private_obj_state(state, &rgrp->private);
> +	if (IS_ERR(pstate))
> +		return ERR_CAST(pstate);
> +
> +	return to_rcar_group_state(pstate);
> +}
> +
>  /**
>   * rcar_du_get_old_group_state - get old group state, if it exists
>   * @state: global atomic state object
> @@ -441,6 +427,47 @@ rcar_du_get_new_group_state(struct drm_atomic_state *state,
>  	return NULL;
>  }
>  
> +int rcar_du_group_atomic_check(struct drm_device *dev,
> +			       struct drm_atomic_state *state)
> +{
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc *crtc;
> +	unsigned int i;
> +
> +	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> +		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +		struct rcar_du_group_state *gstate;
> +
> +		gstate = rcar_du_get_group_state(state, rcrtc->group);
> +		if (IS_ERR(gstate))
> +			return PTR_ERR(gstate);
> +
> +		if (crtc_state->active)
> +			gstate->enabled = true;
> +	}
> +
> +	return 0;
> +}
> +
> +void rcar_du_group_atomic_setup(struct drm_device *dev,
> +				struct drm_atomic_state *state)
> +{
> +	struct drm_private_state *old_pstate, *new_pstate;
> +	struct drm_private_obj *obj;
> +	unsigned int i;
> +
> +	for_each_oldnew_group_in_state(state, obj, old_pstate, new_pstate, i) {
> +		struct rcar_du_group *rgrp = to_rcar_group(obj);
> +		struct rcar_du_group_state *old_state, *new_state;
> +
> +		old_state = to_rcar_group_state(old_pstate);
> +		new_state = to_rcar_group_state(new_pstate);
> +
> +		if (!old_state->enabled && new_state->enabled)
> +			rcar_du_group_setup(rgrp);
> +	}
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Init and Cleanup
>   */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> index f9961f89fd97..20efd2251ec4 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> @@ -26,7 +26,6 @@ struct rcar_du_device;
>   * @index: group index
>   * @channels_mask: bitmask of populated DU channels in this group
>   * @num_crtcs: number of CRTCs in this group (1 or 2)
> - * @use_count: number of users of the group (rcar_du_group_(get|put))
>   * @used_crtcs: number of CRTCs currently in use
>   * @lock: protects the dptsr_planes field and the DPTSR register
>   * @dptsr_planes: bitmask of planes driven by dot-clock and timing generator 1
> @@ -43,7 +42,6 @@ struct rcar_du_group {
>  
>  	unsigned int channels_mask;
>  	unsigned int num_crtcs;
> -	unsigned int use_count;
>  	unsigned int used_crtcs;
>  
>  	struct mutex lock;
> @@ -59,9 +57,12 @@ struct rcar_du_group {
>  /**
>   * struct rcar_du_group_state - Driver-specific group state
>   * @state: base DRM private state
> + * @enabled: true if at least one CRTC in the group is enabled
>   */
>  struct rcar_du_group_state {
>  	struct drm_private_state state;
> +
> +	bool enabled;
>  };
>  
>  #define to_rcar_group_state(s) \
> @@ -70,8 +71,6 @@ struct rcar_du_group_state {
>  u32 rcar_du_group_read(struct rcar_du_group *rgrp, u32 reg);
>  void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data);
>  
> -int rcar_du_group_get(struct rcar_du_group *rgrp);
> -void rcar_du_group_put(struct rcar_du_group *rgrp);
>  void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start);
>  void rcar_du_group_restart(struct rcar_du_group *rgrp);
>  int rcar_du_group_set_routing(struct rcar_du_group *rgrp);
> @@ -85,6 +84,11 @@ struct rcar_du_group_state *
>  rcar_du_get_new_group_state(struct drm_atomic_state *state,
>  			    struct rcar_du_group *rgrp);
>  
> +int rcar_du_group_atomic_check(struct drm_device *dev,
> +			       struct drm_atomic_state *state);
> +void rcar_du_group_atomic_setup(struct drm_device *dev,
> +				struct drm_atomic_state *state);
> +
>  int rcar_du_group_init(struct rcar_du_device *rcdu, struct rcar_du_group *rgrp,
>  		       unsigned int index);
>  void rcar_du_group_cleanup(struct rcar_du_group *rgrp);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index f57a035a94ee..65396134fba1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -377,6 +377,10 @@ static int rcar_du_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		return ret;
>  
> +	ret = rcar_du_group_atomic_check(dev, state);
> +	if (ret)
> +		return ret;
> +
>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		return 0;
>  
> @@ -411,6 +415,7 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
>  
>  	/* Apply the atomic update. */
>  	rcar_du_crtc_atomic_exit_standby(dev, old_state);
> +	rcar_du_group_atomic_setup(dev, old_state);
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>  	rcar_du_crtc_atomic_modeset(dev, old_state);
> -- 
> Regards,
> 
> Laurent Pinchart
>

Reviewed-by: Ulrich Hecht <uli+renesas@xxxxxxxx>

CU
Uli



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux