Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1395215

Re: [PATCH 2/4] drm: Make drm_encoder_helper_funcs optional

From Daniel Vetter <daniel@ffwll.ch>
Newsgroups linux.kernel
Subject Re: [PATCH 2/4] drm: Make drm_encoder_helper_funcs optional
Date 2016-05-05 18:30 +0200
Message-ID <rvuk2-3e2-15@gated-at.bofh.it> (permalink)
References <rvrvQ-MH-21@gated-at.bofh.it> <rvrvR-MH-33@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, May 05, 2016 at 03:24:32PM +0200, Noralf Trønnes wrote:
> Make drm_encoder_helper_funcs and it's functions optional to avoid
> having dummy functions.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Please also update the kerneldoc and mention there that the enable/disable
hooks are optional. You can build the hmtl docs using

$ make htmldocs

to check the result. The vtables are documented in
include/drm/drm_helper_vtables.h

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++++--
>  drivers/gpu/drm/drm_crtc_helper.c   | 41 +++++++++++++++++++++++++++++--------

Hm, tbh I wouldn't bother at all with crtc helpers and just drop that
part. Old drivers should converted to atomic, new drivers should just use
atomic. No need to touch that code at all.
-Daniel

>  2 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 92e11a2..03ea049 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -696,6 +696,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  			continue;
>  
>  		funcs = encoder->helper_private;
> +		if (!funcs)
> +			continue;
>  
>  		DRM_DEBUG_ATOMIC("disabling [ENCODER:%d:%s]\n",
>  				 encoder->base.id, encoder->name);
> @@ -711,7 +713,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  			funcs->prepare(encoder);
>  		else if (funcs->disable)
>  			funcs->disable(encoder);
> -		else
> +		else if (funcs->dpms)
>  			funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
>  
>  		drm_bridge_post_disable(encoder->bridge);
> @@ -859,6 +861,9 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
>  
>  		encoder = connector->state->best_encoder;
>  		funcs = encoder->helper_private;
> +		if (!funcs)
> +			continue;
> +
>  		new_crtc_state = connector->state->crtc->state;
>  		mode = &new_crtc_state->mode;
>  		adjusted_mode = &new_crtc_state->adjusted_mode;
> @@ -964,6 +969,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>  
>  		encoder = connector->state->best_encoder;
>  		funcs = encoder->helper_private;
> +		if (!funcs)
> +			continue;
>  
>  		DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n",
>  				 encoder->base.id, encoder->name);
> @@ -976,7 +983,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>  
>  		if (funcs->enable)
>  			funcs->enable(encoder);
> -		else
> +		else if (funcs->commit)
>  			funcs->commit(encoder);
>  
>  		drm_bridge_enable(encoder->bridge);
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 66ca313..6e6ab38 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -170,11 +170,14 @@ drm_encoder_disable(struct drm_encoder *encoder)
>  {
>  	const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;
>  
> +	if (!encoder_funcs)
> +		return;
> +
>  	drm_bridge_disable(encoder->bridge);
>  
>  	if (encoder_funcs->disable)
>  		(*encoder_funcs->disable)(encoder);
> -	else
> +	else if (encoder_funcs->dpms)
>  		(*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
>  
>  	drm_bridge_post_disable(encoder->bridge);
> @@ -248,6 +251,9 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
>  
>  	drm_for_each_encoder(encoder, dev) {
>  		encoder_funcs = encoder->helper_private;
> +		if (!encoder_funcs)
> +			continue;
> +
>  		/* Disable unused encoders */
>  		if (encoder->crtc == NULL)
>  			drm_encoder_disable(encoder);
> @@ -326,6 +332,10 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  		if (encoder->crtc != crtc)
>  			continue;
>  
> +		encoder_funcs = encoder->helper_private;
> +		if (!encoder_funcs)
> +			continue;
> +
>  		ret = drm_bridge_mode_fixup(encoder->bridge,
>  			mode, adjusted_mode);
>  		if (!ret) {
> @@ -360,11 +370,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  		if (encoder->crtc != crtc)
>  			continue;
>  
> +		encoder_funcs = encoder->helper_private;
> +		if (!encoder_funcs)
> +			continue;
> +
>  		drm_bridge_disable(encoder->bridge);
>  
> -		encoder_funcs = encoder->helper_private;
>  		/* Disable the encoders as the first thing we do. */
> -		encoder_funcs->prepare(encoder);
> +		if (encoder_funcs->prepare)
> +			encoder_funcs->prepare(encoder);
>  
>  		drm_bridge_post_disable(encoder->bridge);
>  	}
> @@ -385,11 +399,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  		if (encoder->crtc != crtc)
>  			continue;
>  
> +		encoder_funcs = encoder->helper_private;
> +		if (!encoder_funcs)
> +			continue;
> +
>  		DRM_DEBUG_KMS("[ENCODER:%d:%s] set [MODE:%d:%s]\n",
>  			encoder->base.id, encoder->name,
>  			mode->base.id, mode->name);
> -		encoder_funcs = encoder->helper_private;
> -		encoder_funcs->mode_set(encoder, mode, adjusted_mode);
> +		if (encoder_funcs->mode_set)
> +			encoder_funcs->mode_set(encoder, mode, adjusted_mode);
>  
>  		drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
>  	}
> @@ -402,10 +420,14 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  		if (encoder->crtc != crtc)
>  			continue;
>  
> +		encoder_funcs = encoder->helper_private;
> +		if (!encoder_funcs)
> +			continue;
> +
>  		drm_bridge_pre_enable(encoder->bridge);
>  
> -		encoder_funcs = encoder->helper_private;
> -		encoder_funcs->commit(encoder);
> +		if (encoder_funcs->commit)
> +			encoder_funcs->commit(encoder);
>  
>  		drm_bridge_enable(encoder->bridge);
>  	}
> @@ -771,12 +793,15 @@ static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode)
>  	struct drm_bridge *bridge = encoder->bridge;
>  	const struct drm_encoder_helper_funcs *encoder_funcs;
>  
> +	encoder_funcs = encoder->helper_private;
> +	if (!encoder_funcs)
> +		return;
> +
>  	if (mode == DRM_MODE_DPMS_ON)
>  		drm_bridge_pre_enable(bridge);
>  	else
>  		drm_bridge_disable(bridge);
>  
> -	encoder_funcs = encoder->helper_private;
>  	if (encoder_funcs->dpms)
>  		encoder_funcs->dpms(encoder, mode);
>  
> -- 
> 2.2.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/4] drm: Add various helpers for simple drivers Noralf Trønnes <noralf@tronnes.org> - 2016-05-05 15:30 +0200
  [PATCH 2/4] drm: Make drm_encoder_helper_funcs optional Noralf Trønnes <noralf@tronnes.org> - 2016-05-05 15:30 +0200
    Re: [PATCH 2/4] drm: Make drm_encoder_helper_funcs optional Daniel Vetter <daniel@ffwll.ch> - 2016-05-05 18:30 +0200
      Re: [PATCH 2/4] drm: Make drm_encoder_helper_funcs optional Noralf Trønnes <noralf@tronnes.org> - 2016-05-09 21:20 +0200
        Re: [PATCH 2/4] drm: Make drm_encoder_helper_funcs optional Daniel Vetter <daniel@ffwll.ch> - 2016-05-10 09:00 +0200
  [PATCH 3/4] drm: Add helper for simple display pipeline Noralf Trønnes <noralf@tronnes.org> - 2016-05-05 15:30 +0200
    Re: [PATCH 3/4] drm: Add helper for simple display pipeline Daniel Vetter <daniel@ffwll.ch> - 2016-05-05 18:50 +0200
    Re: [PATCH 3/4] drm: Add helper for simple display pipeline Daniel Vetter <daniel@ffwll.ch> - 2016-05-09 16:50 +0200
      Re: [PATCH 3/4] drm: Add helper for simple display pipeline Noralf Trønnes <noralf@tronnes.org> - 2016-05-09 20:40 +0200
        Re: [PATCH 3/4] drm: Add helper for simple display pipeline Daniel Vetter <daniel@ffwll.ch> - 2016-05-10 09:00 +0200
          Re: [PATCH 3/4] drm: Add helper for simple display pipeline Daniel Vetter <daniel@ffwll.ch> - 2016-05-11 00:40 +0200
  [PATCH 1/4] drm/fb-cma-helper: Add function drm_fb_cma_create_with_funcs() Noralf Trønnes <noralf@tronnes.org> - 2016-05-05 15:30 +0200
    Re: [PATCH 1/4] drm/fb-cma-helper: Add function  drm_fb_cma_create_with_funcs() Daniel Vetter <daniel@ffwll.ch> - 2016-05-05 18:30 +0200
      Re: [PATCH 1/4] drm/fb-cma-helper: Add function  drm_fb_cma_create_with_funcs() Noralf Trønnes <noralf@tronnes.org> - 2016-05-06 15:10 +0200
        Re: [PATCH 1/4] drm/fb-cma-helper: Add function  drm_fb_cma_create_with_funcs() Daniel Vetter <daniel@ffwll.ch> - 2016-05-06 15:20 +0200

csiph-web