[DPU PATCH] drm/msm: remove support for seamless modes

Sean Paul seanpaul at chromium.org
Thu Jun 14 15:26:25 UTC 2018


On Tue, Jun 12, 2018 at 05:49:03PM -0700, Abhinav Kumar wrote:
> Seamless modes are ones which do not require a display
> to be turned OFF/ON between mode switches.
> 
> Remove support for seamless modes from DPU for now.
> 
> This will be added later based on additional requirements.
> 
> This change depends on the DPU custom property removal series:
>  - https://patchwork.freedesktop.org/series/44592/

Pushed to dpu-staging/for-next, thanks

Sean

> 
> Signed-off-by: Abhinav Kumar <abhinavk at codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |  31 --------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 106 +---------------------------
>  drivers/gpu/drm/msm/msm_kms.h               |  44 ------------
>  include/uapi/drm/drm_mode.h                 |   1 -
>  4 files changed, 3 insertions(+), 179 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 4616a62..9ca8325 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -591,22 +591,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
>  	kfree(dpu_crtc);
>  }
>  
> -static bool dpu_crtc_mode_fixup(struct drm_crtc *crtc,
> -		const struct drm_display_mode *mode,
> -		struct drm_display_mode *adjusted_mode)
> -{
> -	DPU_DEBUG("\n");
> -
> -	if ((msm_is_mode_seamless(adjusted_mode) ||
> -			msm_is_mode_seamless_vrr(adjusted_mode)) &&
> -		(!crtc->enabled)) {
> -		DPU_ERROR("crtc state prevents seamless transition\n");
> -		return false;
> -	}
> -
> -	return true;
> -}
> -
>  static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
>  		struct dpu_plane_state *pstate)
>  {
> @@ -1728,12 +1712,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
>  	mode = &cstate->base.adjusted_mode;
>  	priv = crtc->dev->dev_private;
>  
> -	if (msm_is_mode_seamless(mode) || msm_is_mode_seamless_vrr(mode) ||
> -	    msm_is_mode_seamless_dms(mode)) {
> -		DPU_DEBUG("Seamless mode is being applied, skip disable\n");
> -		return;
> -	}
> -
>  	DPU_DEBUG("crtc%d\n", crtc->base.id);
>  
>  	if (dpu_kms_is_suspend_state(crtc->dev))
> @@ -1817,12 +1795,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
>  	DPU_EVT32_VERBOSE(DRMID(crtc));
>  	dpu_crtc = to_dpu_crtc(crtc);
>  
> -	if (msm_is_mode_seamless(&crtc->state->adjusted_mode) ||
> -	    msm_is_mode_seamless_vrr(&crtc->state->adjusted_mode)) {
> -		DPU_DEBUG("Skipping crtc enable, seamless mode\n");
> -		return;
> -	}
> -
>  	drm_for_each_encoder(encoder, crtc->dev) {
>  		if (encoder->crtc != crtc)
>  			continue;
> @@ -1857,8 +1829,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
>  		DPU_POWER_EVENT_PRE_DISABLE,
>  		dpu_crtc_handle_power_event, crtc, dpu_crtc->name);
>  
> -	if (msm_needs_vblank_pre_modeset(&crtc->state->adjusted_mode))
> -		drm_crtc_wait_one_vblank(crtc);
>  }
>  
>  struct plane_state {
> @@ -2497,7 +2467,6 @@ static void dpu_crtc_early_unregister(struct drm_crtc *crtc)
>  };
>  
>  static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = {
> -	.mode_fixup = dpu_crtc_mode_fixup,
>  	.disable = dpu_crtc_disable,
>  	.atomic_enable = dpu_crtc_enable,
>  	.atomic_check = dpu_crtc_atomic_check,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 7dd609c..11a1045 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -96,15 +96,6 @@
>   *	IDLE is expected when IDLE_PC has run, and PRE_OFF did nothing.
>   *	PRE_OFF is expected when PRE_STOP was executed during the ON state.
>   *	Resource state should be in OFF at the end of the event.
> - * @DPU_ENC_RC_EVENT_PRE_MODESET:
> - *	This event happens at NORMAL priority from a work item.
> - *	Event signals that there is a seamless mode switch is in prgoress. A
> - *	client needs to turn of only irq - leave clocks ON to reduce the mode
> - *	switch latency.
> - * @DPU_ENC_RC_EVENT_POST_MODESET:
> - *	This event happens at NORMAL priority from a work item.
> - *	Event signals that seamless mode switch is complete and resources are
> - *	acquired. Clients wants to turn on the irq again.
>   * @DPU_ENC_RC_EVENT_ENTER_IDLE:
>   *	This event happens at NORMAL priority from a work item.
>   *	Event signals that there were no frame updates for IDLE_TIMEOUT time.
> @@ -116,8 +107,6 @@ enum dpu_enc_rc_events {
>  	DPU_ENC_RC_EVENT_FRAME_DONE,
>  	DPU_ENC_RC_EVENT_PRE_STOP,
>  	DPU_ENC_RC_EVENT_STOP,
> -	DPU_ENC_RC_EVENT_PRE_MODESET,
> -	DPU_ENC_RC_EVENT_POST_MODESET,
>  	DPU_ENC_RC_EVENT_ENTER_IDLE
>  };
>  
> @@ -133,7 +122,6 @@ enum dpu_enc_rc_states {
>  	DPU_ENC_RC_STATE_OFF,
>  	DPU_ENC_RC_STATE_PRE_OFF,
>  	DPU_ENC_RC_STATE_ON,
> -	DPU_ENC_RC_STATE_MODESET,
>  	DPU_ENC_RC_STATE_IDLE
>  };
>  
> @@ -809,7 +797,6 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
>  	struct dpu_encoder_virt *dpu_enc;
>  	struct msm_drm_private *priv;
>  	struct msm_drm_thread *disp_thread;
> -	int ret;
>  	bool is_vid_mode = false;
>  
>  	if (!drm_enc || !drm_enc->dev || !drm_enc->dev->dev_private ||
> @@ -834,8 +821,6 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
>  	 */
>  	if (!dpu_enc->idle_pc_supported &&
>  			(sw_event != DPU_ENC_RC_EVENT_KICKOFF &&
> -			sw_event != DPU_ENC_RC_EVENT_PRE_MODESET &&
> -			sw_event != DPU_ENC_RC_EVENT_POST_MODESET &&
>  			sw_event != DPU_ENC_RC_EVENT_STOP &&
>  			sw_event != DPU_ENC_RC_EVENT_PRE_STOP))
>  		return 0;
> @@ -966,8 +951,7 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
>  				DPU_EVTLOG_FUNC_CASE4);
>  			mutex_unlock(&dpu_enc->rc_lock);
>  			return 0;
> -		} else if (dpu_enc->rc_state == DPU_ENC_RC_STATE_ON ||
> -			   dpu_enc->rc_state == DPU_ENC_RC_STATE_MODESET) {
> +		} else if (dpu_enc->rc_state == DPU_ENC_RC_STATE_ON) {
>  			DPU_ERROR_ENC(dpu_enc, "sw_event:%d, rc in state %d\n",
>  					sw_event, dpu_enc->rc_state);
>  			DPU_EVT32(DRMID(drm_enc), sw_event, dpu_enc->rc_state,
> @@ -991,69 +975,6 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
>  		mutex_unlock(&dpu_enc->rc_lock);
>  		break;
>  
> -	case DPU_ENC_RC_EVENT_PRE_MODESET:
> -		/* cancel delayed off work, if any */
> -		if (kthread_cancel_delayed_work_sync(
> -				&dpu_enc->delayed_off_work))
> -			DPU_DEBUG_ENC(dpu_enc, "sw_event:%d, work cancelled\n",
> -					sw_event);
> -
> -		mutex_lock(&dpu_enc->rc_lock);
> -
> -		/* return if the resource control is already in ON state */
> -		if (dpu_enc->rc_state != DPU_ENC_RC_STATE_ON) {
> -			/* enable all the clks and resources */
> -			_dpu_encoder_resource_control_helper(drm_enc, true);
> -
> -			DPU_EVT32(DRMID(drm_enc), sw_event, dpu_enc->rc_state,
> -				DPU_ENC_RC_STATE_ON, DPU_EVTLOG_FUNC_CASE5);
> -			dpu_enc->rc_state = DPU_ENC_RC_STATE_ON;
> -		}
> -
> -		ret = dpu_encoder_wait_for_event(drm_enc, MSM_ENC_TX_COMPLETE);
> -		if (ret && ret != -EWOULDBLOCK) {
> -			DPU_ERROR_ENC(dpu_enc,
> -					"wait for commit done returned %d\n",
> -					ret);
> -			DPU_EVT32(DRMID(drm_enc), sw_event, dpu_enc->rc_state,
> -					ret, DPU_EVTLOG_ERROR);
> -			mutex_unlock(&dpu_enc->rc_lock);
> -			return -EINVAL;
> -		}
> -
> -		_dpu_encoder_irq_control(drm_enc, false);
> -
> -		DPU_EVT32(DRMID(drm_enc), sw_event, dpu_enc->rc_state,
> -			DPU_ENC_RC_STATE_MODESET, DPU_EVTLOG_FUNC_CASE5);
> -
> -		dpu_enc->rc_state = DPU_ENC_RC_STATE_MODESET;
> -		mutex_unlock(&dpu_enc->rc_lock);
> -		break;
> -
> -	case DPU_ENC_RC_EVENT_POST_MODESET:
> -		mutex_lock(&dpu_enc->rc_lock);
> -
> -		/* return if the resource control is already in ON state */
> -		if (dpu_enc->rc_state != DPU_ENC_RC_STATE_MODESET) {
> -			DPU_ERROR_ENC(dpu_enc,
> -					"sw_event:%d, rc:%d !MODESET state\n",
> -					sw_event, dpu_enc->rc_state);
> -			DPU_EVT32(DRMID(drm_enc), sw_event, dpu_enc->rc_state,
> -					DPU_EVTLOG_ERROR);
> -			mutex_unlock(&dpu_enc->rc_lock);
> -			return -EINVAL;
> -		}
> -
> -		_dpu_encoder_irq_control(drm_enc, true);
> -
> -		DPU_EVT32(DRMID(drm_enc), sw_event, dpu_enc->rc_state,
> -				DPU_ENC_RC_STATE_ON, DPU_EVTLOG_FUNC_CASE6);
> -
> -		dpu_enc->rc_state = DPU_ENC_RC_STATE_ON;
> -
> -		mutex_unlock(&dpu_enc->rc_lock);
> -		break;
> -
>  	case DPU_ENC_RC_EVENT_ENTER_IDLE:
>  		mutex_lock(&dpu_enc->rc_lock);
>  
> @@ -1180,11 +1101,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  		}
>  	}
>  
> -	/* update resources after seamless mode change */
> -	if (msm_is_mode_seamless_dms(adj_mode))
> -		dpu_encoder_resource_control(&dpu_enc->base,
> -						DPU_ENC_RC_EVENT_POST_MODESET);
> -
>  	dpu_enc->mode_set_complete = true;
>  }
>  
> @@ -1297,15 +1213,7 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>  			continue;
>  
>  		if (phys != dpu_enc->cur_master) {
> -			/**
> -			 * on DMS request, the encoder will be enabled
> -			 * already. Invoke restore to reconfigure the
> -			 * new mode.
> -			 */
> -			if (msm_is_mode_seamless_dms(cur_mode) &&
> -					phys->ops.restore)
> -				phys->ops.restore(phys);
> -			else if (phys->ops.enable)
> +			if (phys->ops.enable)
>  				phys->ops.enable(phys);
>  		}
>  
> @@ -1315,10 +1223,7 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>  						dpu_enc->misr_frame_count);
>  	}
>  
> -	if (msm_is_mode_seamless_dms(cur_mode) &&
> -			dpu_enc->cur_master->ops.restore)
> -		dpu_enc->cur_master->ops.restore(dpu_enc->cur_master);
> -	else if (dpu_enc->cur_master->ops.enable)
> +	if (dpu_enc->cur_master->ops.enable)
>  		dpu_enc->cur_master->ops.enable(dpu_enc->cur_master);
>  
>  	_dpu_encoder_virt_enable_helper(drm_enc);
> @@ -1344,11 +1249,6 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
>  	}
>  
>  	mode = &drm_enc->crtc->state->adjusted_mode;
> -	if (msm_is_mode_seamless(mode) || msm_is_mode_seamless_vrr(mode) ||
> -	    msm_is_mode_seamless_dms(mode)) {
> -		DPU_DEBUG("Seamless mode is being applied, skip disable\n");
> -		return;
> -	}
>  
>  	dpu_enc = to_dpu_encoder_virt(drm_enc);
>  	DPU_DEBUG_ENC(dpu_enc, "\n");
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 5e1de85..4544d0a 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -26,19 +26,6 @@
>  
>  #define MAX_PLANE	4
>  
> -/**
> - * Device Private DRM Mode Flags
> - * drm_mode->private_flags
> - */
> -/* Connector has interpreted seamless transition request as dynamic fps */
> -#define MSM_MODE_FLAG_SEAMLESS_DYNAMIC_FPS	(1<<0)
> -/* Transition to new mode requires a wait-for-vblank before the modeset */
> -#define MSM_MODE_FLAG_VBLANK_PRE_MODESET	(1<<1)
> -/* Request to switch the connector mode */
> -#define MSM_MODE_FLAG_SEAMLESS_DMS			(1<<2)
> -/* Request to switch the fps */
> -#define MSM_MODE_FLAG_SEAMLESS_VRR			(1<<3)
> -
>  /* As there are different display controller blocks depending on the
>   * snapdragon version, the kms support is split out and the appropriate
>   * implementation is loaded at runtime.  The kms module is responsible
> @@ -146,35 +133,4 @@ struct msm_mdss {
>  int mdp5_mdss_init(struct drm_device *dev);
>  int dpu_mdss_init(struct drm_device *dev);
>  
> -/**
> - * Mode Set Utility Functions
> - */
> -static inline bool msm_is_mode_seamless(const struct drm_display_mode *mode)
> -{
> -	return (mode->flags & DRM_MODE_FLAG_SEAMLESS);
> -}
> -
> -static inline bool msm_is_mode_seamless_dms(const struct drm_display_mode *mode)
> -{
> -	return mode ? (mode->private_flags & MSM_MODE_FLAG_SEAMLESS_DMS)
> -		: false;
> -}
> -
> -static inline bool msm_is_mode_dynamic_fps(const struct drm_display_mode *mode)
> -{
> -	return ((mode->flags & DRM_MODE_FLAG_SEAMLESS) &&
> -		(mode->private_flags & MSM_MODE_FLAG_SEAMLESS_DYNAMIC_FPS));
> -}
> -
> -static inline bool msm_is_mode_seamless_vrr(const struct drm_display_mode *mode)
> -{
> -	return mode ? (mode->private_flags & MSM_MODE_FLAG_SEAMLESS_VRR)
> -		: false;
> -}
> -
> -static inline bool msm_needs_vblank_pre_modeset(
> -		const struct drm_display_mode *mode)
> -{
> -	return (mode->private_flags & MSM_MODE_FLAG_VBLANK_PRE_MODESET);
> -}
>  #endif /* __MSM_KMS_H__ */
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 4e1a3ff..50bcf42 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -88,7 +88,6 @@
>  #define  DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH	(6<<14)
>  #define  DRM_MODE_FLAG_3D_TOP_AND_BOTTOM	(7<<14)
>  #define  DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF	(8<<14)
> -#define  DRM_MODE_FLAG_SEAMLESS			(1<<19)
>  
>  /* Picture aspect ratio options */
>  #define DRM_MODE_PICTURE_ASPECT_NONE		0
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS


More information about the dri-devel mailing list