Re: [PATCH v3 09/33] drm/i915: Move panel's pipe from backlight setup to init

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jun 03, 2016 at 03:36:52PM +0100, Chris Wilson wrote:
> In the very near future, we will perform the backlight setup
> consistently during connector registration - moving the setup further
> away from the intel_panel_init call and to where we no longer know the
> associated pipe. To pass that information along we need to store it
> during init.

This looks like the wrong approach. The setup hook should be called
at connector init time. In fact the code looks correcto to me already,
as in we have two phases already: setup and register.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c    |  6 ++++--
>  drivers/gpu/drm/i915/intel_drv.h   |  6 ++++--
>  drivers/gpu/drm/i915/intel_dsi.c   |  6 ++++--
>  drivers/gpu/drm/i915/intel_dvo.c   |  3 ++-
>  drivers/gpu/drm/i915/intel_lvds.c  |  6 ++++--
>  drivers/gpu/drm/i915/intel_panel.c | 11 +++++++----
>  6 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f97cd5305e4c..809680bc5d04 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5402,9 +5402,11 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  			      pipe_name(pipe));
>  	}
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +	intel_panel_init(&intel_connector->panel,
> +			 fixed_mode, downclock_mode,
> +			 pipe);
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
> -	intel_panel_setup_backlight(connector, pipe);
> +	intel_panel_setup_backlight(connector);
>  
>  	return true;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b3fffd805225..250b7c5d992e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -202,6 +202,7 @@ struct intel_panel {
>  	struct drm_display_mode *downclock_mode;
>  	int fitting_mode;
>  	bool is_panel;
> +	enum pipe pipe;
>  
>  	/* backlight */
>  	struct {
> @@ -1476,7 +1477,8 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
>  /* intel_panel.c */
>  int intel_panel_init(struct intel_panel *panel,
>  		     struct drm_display_mode *fixed_mode,
> -		     struct drm_display_mode *downclock_mode);
> +		     struct drm_display_mode *downclock_mode,
> +		     enum pipe pipe);
>  void intel_panel_fini(struct intel_panel *panel);
>  void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  			    struct drm_display_mode *adjusted_mode);
> @@ -1488,7 +1490,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *crtc,
>  			      int fitting_mode);
>  void intel_panel_set_backlight_acpi(struct intel_connector *connector,
>  				    u32 level, u32 max);
> -int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe);
> +int intel_panel_setup_backlight(struct drm_connector *connector);
>  void intel_panel_enable_backlight(struct intel_connector *connector);
>  void intel_panel_disable_backlight(struct intel_connector *connector);
>  void intel_panel_destroy_backlight(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index c70132aa91d5..64d6e397639f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1581,13 +1581,15 @@ void intel_dsi_init(struct drm_device *dev)
>  	connector->display_info.width_mm = fixed_mode->width_mm;
>  	connector->display_info.height_mm = fixed_mode->height_mm;
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> +	intel_panel_init(&intel_connector->panel,
> +			 fixed_mode, NULL,
> +			 INVALID_PIPE);
>  
>  	intel_dsi_add_properties(intel_connector);
>  
>  	drm_connector_register(connector);
>  
> -	intel_panel_setup_backlight(connector, INVALID_PIPE);
> +	intel_panel_setup_backlight(connector);
>  
>  	return;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index a456f2eb68b6..c1c8a1cf4420 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -547,7 +547,8 @@ void intel_dvo_init(struct drm_device *dev)
>  			 */
>  			intel_panel_init(&intel_connector->panel,
>  					 intel_dvo_get_current_mode(connector),
> -					 NULL);
> +					 NULL,
> +					 INVALID_PIPE);
>  			intel_dvo->panel_wants_dither = true;
>  		}
>  
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 62eaa895fe5b..1ceaf8db1545 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1118,7 +1118,9 @@ void intel_lvds_init(struct drm_device *dev)
>  out:
>  	mutex_unlock(&dev->mode_config.mutex);
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +	intel_panel_init(&intel_connector->panel,
> +			 fixed_mode, downclock_mode,
> +			 INVALID_PIPE);
>  
>  	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
>  	DRM_DEBUG_KMS("detected %s-link lvds configuration\n",
> @@ -1133,7 +1135,7 @@ out:
>  	}
>  	drm_connector_register(connector);
>  
> -	intel_panel_setup_backlight(connector, INVALID_PIPE);
> +	intel_panel_setup_backlight(connector);
>  
>  	return;
>  
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 8bd076b11af1..dfd388567fa7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1663,7 +1663,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
>  	return 0;
>  }
>  
> -int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
> +int intel_panel_setup_backlight(struct drm_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
> @@ -1688,7 +1688,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
>  
>  	/* set level and max in panel struct */
>  	mutex_lock(&dev_priv->backlight_lock);
> -	ret = panel->backlight.setup(intel_connector, pipe);
> +	ret = panel->backlight.setup(intel_connector, panel->pipe);
>  	mutex_unlock(&dev_priv->backlight_lock);
>  
>  	if (ret) {
> @@ -1796,13 +1796,16 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>  
>  int intel_panel_init(struct intel_panel *panel,
>  		     struct drm_display_mode *fixed_mode,
> -		     struct drm_display_mode *downclock_mode)
> +		     struct drm_display_mode *downclock_mode,
> +		     enum pipe pipe)
>  {
>  	intel_panel_init_backlight_funcs(panel);
>  
> +	panel->is_panel = true;
> +	panel->pipe = pipe;
> +
>  	panel->fixed_mode = fixed_mode;
>  	panel->downclock_mode = downclock_mode;
> -	panel->is_panel = true;
>  
>  	return 0;
>  }
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx