[Intel-gfx] [RFC PATCH 05/12] drm/i915/dsi: remove unnecessary dsi device callbacks

Jani Nikula jani.nikula at intel.com
Thu Jan 22 05:23:45 PST 2015


On Thu, 22 Jan 2015, Shobhit Kumar <shobhit.kumar at linux.intel.com> wrote:
> On 01/16/2015 05:57 PM, Jani Nikula wrote:
>> Remove all the trivial and/or dummy callbacks from intel dsi device
>> ops. Merge send_otp_cmds into panel_reset as they're called back to
>> back.
>>
>> This will be helpful for switching to use drm_panel for the
>> callbacks. If we ever need the additional callbacks, we should add them
>> to drm_panel funcs.
>>
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dsi.c           | 32 ++-------------------
>>   drivers/gpu/drm/i915/intel_dsi.h           | 20 -------------
>>   drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 45 ++----------------------------
>>   3 files changed, 5 insertions(+), 92 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 9b0eaa9db845..fc218b7754b3 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -70,12 +70,6 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
>>   	mutex_unlock(&dev_priv->dpio_lock);
>>   }
>>
>> -static struct intel_dsi *intel_attached_dsi(struct drm_connector *connector)
>> -{
>> -	return container_of(intel_attached_encoder(connector),
>> -			    struct intel_dsi, base);
>> -}
>> -
>>   static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
>>   {
>>   	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
>> @@ -99,7 +93,6 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>>   	struct intel_connector *intel_connector = intel_dsi->attached_connector;
>>   	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>>   	struct drm_display_mode *adjusted_mode = &config->adjusted_mode;
>> -	struct drm_display_mode *mode = &config->requested_mode;
>>
>>   	DRM_DEBUG_KMS("\n");
>>
>> @@ -109,10 +102,6 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>>   	/* DSI uses short packets for sync events, so clear mode flags for DSI */
>>   	adjusted_mode->flags = 0;
>>
>> -	if (intel_dsi->dev.dev_ops->mode_fixup)
>> -		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
>> -							  mode, adjusted_mode);
>> -
>
> There had been a instance where we had to drive different resolution 
> (lower) than the native one. Also in VBT there is a field to make this 
> generic at least from driver perspective to give the needed target 
> resolution. In case target resolution is same as native, nothing gets 
> changed, else mode_fixup function adjusts the mode accordingly keeping 
> timing as same and enabling scalar. Might not be useful in general, but 
> did find a use internally.

Can we just have the driver return the desired mode from .get_modes in
that case?

BR,
Jani.


>
> Either way
>
> Reviewed-By: Shobhit Kumar <shobhit.kumar at intel.com>
>
>>   	return true;
>>   }
>>
>> @@ -269,9 +258,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>>   	if (intel_dsi->dev.dev_ops->panel_reset)
>>   		intel_dsi->dev.dev_ops->panel_reset(&intel_dsi->dev);
>>
>> -	if (intel_dsi->dev.dev_ops->send_otp_cmds)
>> -		intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev);
>> -
>>   	for_each_dsi_port(port, intel_dsi->ports)
>>   		wait_for_dsi_fifo_empty(intel_dsi, port);
>>
>> @@ -484,7 +470,6 @@ intel_dsi_mode_valid(struct drm_connector *connector,
>>   {
>>   	struct intel_connector *intel_connector = to_intel_connector(connector);
>>   	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>> -	struct intel_dsi *intel_dsi = intel_attached_dsi(connector);
>>
>>   	DRM_DEBUG_KMS("\n");
>>
>> @@ -500,7 +485,7 @@ intel_dsi_mode_valid(struct drm_connector *connector,
>>   			return MODE_PANEL;
>>   	}
>>
>> -	return intel_dsi->dev.dev_ops->mode_valid(&intel_dsi->dev, mode);
>> +	return MODE_OK;
>>   }
>>
>>   /* return txclkesc cycles in terms of divider and duration in us */
>> @@ -749,20 +734,7 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
>>   static enum drm_connector_status
>>   intel_dsi_detect(struct drm_connector *connector, bool force)
>>   {
>> -	struct intel_dsi *intel_dsi = intel_attached_dsi(connector);
>> -	struct intel_encoder *intel_encoder = &intel_dsi->base;
>> -	enum intel_display_power_domain power_domain;
>> -	enum drm_connector_status connector_status;
>> -	struct drm_i915_private *dev_priv = intel_encoder->base.dev->dev_private;
>> -
>> -	DRM_DEBUG_KMS("\n");
>> -	power_domain = intel_display_port_power_domain(intel_encoder);
>> -
>> -	intel_display_power_get(dev_priv, power_domain);
>> -	connector_status = intel_dsi->dev.dev_ops->detect(&intel_dsi->dev);
>> -	intel_display_power_put(dev_priv, power_domain);
>> -
>> -	return connector_status;
>> +	return connector_status_connected;
>>   }
>>
>>   static int intel_dsi_get_modes(struct drm_connector *connector)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index 2bb8c46c7889..22f87036a256 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -47,33 +47,13 @@ struct intel_dsi_dev_ops {
>>
>>   	void (*disable_panel_power)(struct intel_dsi_device *dsi);
>>
>> -	/* one time programmable commands if needed */
>> -	void (*send_otp_cmds)(struct intel_dsi_device *dsi);
>> -
>>   	/* This callback must be able to assume DSI commands can be sent */
>>   	void (*enable)(struct intel_dsi_device *dsi);
>>
>>   	/* This callback must be able to assume DSI commands can be sent */
>>   	void (*disable)(struct intel_dsi_device *dsi);
>>
>> -	int (*mode_valid)(struct intel_dsi_device *dsi,
>> -			  struct drm_display_mode *mode);
>> -
>> -	bool (*mode_fixup)(struct intel_dsi_device *dsi,
>> -			   const struct drm_display_mode *mode,
>> -			   struct drm_display_mode *adjusted_mode);
>> -
>> -	void (*mode_set)(struct intel_dsi_device *dsi,
>> -			 struct drm_display_mode *mode,
>> -			 struct drm_display_mode *adjusted_mode);
>> -
>> -	enum drm_connector_status (*detect)(struct intel_dsi_device *dsi);
>> -
>> -	bool (*get_hw_state)(struct intel_dsi_device *dev);
>> -
>>   	struct drm_display_mode *(*get_modes)(struct intel_dsi_device *dsi);
>> -
>> -	void (*destroy) (struct intel_dsi_device *dsi);
>>   };
>>
>>   struct intel_dsi {
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index 5493aef5a6a3..b0e7327a485f 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -559,18 +559,6 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>   	return true;
>>   }
>>
>> -static int generic_mode_valid(struct intel_dsi_device *dsi,
>> -		   struct drm_display_mode *mode)
>> -{
>> -	return MODE_OK;
>> -}
>> -
>> -static bool generic_mode_fixup(struct intel_dsi_device *dsi,
>> -		    const struct drm_display_mode *mode,
>> -		    struct drm_display_mode *adjusted_mode) {
>> -	return true;
>> -}
>> -
>>   static void generic_panel_reset(struct intel_dsi_device *dsi)
>>   {
>>   	struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev);
>> @@ -580,26 +568,18 @@ static void generic_panel_reset(struct intel_dsi_device *dsi)
>>   	char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET];
>>
>>   	generic_exec_sequence(intel_dsi, sequence);
>> -}
>> -
>> -static void generic_disable_panel_power(struct intel_dsi_device *dsi)
>> -{
>> -	struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev);
>> -	struct drm_device *dev = intel_dsi->base.base.dev;
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -
>> -	char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET];
>>
>> +	sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
>>   	generic_exec_sequence(intel_dsi, sequence);
>>   }
>>
>> -static void generic_send_otp_cmds(struct intel_dsi_device *dsi)
>> +static void generic_disable_panel_power(struct intel_dsi_device *dsi)
>>   {
>>   	struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev);
>>   	struct drm_device *dev = intel_dsi->base.base.dev;
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> -	char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
>> +	char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET];
>>
>>   	generic_exec_sequence(intel_dsi, sequence);
>>   }
>> @@ -626,16 +606,6 @@ static void generic_disable(struct intel_dsi_device *dsi)
>>   	generic_exec_sequence(intel_dsi, sequence);
>>   }
>>
>> -static enum drm_connector_status generic_detect(struct intel_dsi_device *dsi)
>> -{
>> -	return connector_status_connected;
>> -}
>> -
>> -static bool generic_get_hw_state(struct intel_dsi_device *dev)
>> -{
>> -	return true;
>> -}
>> -
>>   static struct drm_display_mode *generic_get_modes(struct intel_dsi_device *dsi)
>>   {
>>   	struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev);
>> @@ -646,20 +616,11 @@ static struct drm_display_mode *generic_get_modes(struct intel_dsi_device *dsi)
>>   	return dev_priv->vbt.lfp_lvds_vbt_mode;
>>   }
>>
>> -static void generic_destroy(struct intel_dsi_device *dsi) { }
>> -
>> -/* Callbacks. We might not need them all. */
>>   struct intel_dsi_dev_ops vbt_generic_dsi_display_ops = {
>>   	.init = generic_init,
>> -	.mode_valid = generic_mode_valid,
>> -	.mode_fixup = generic_mode_fixup,
>>   	.panel_reset = generic_panel_reset,
>>   	.disable_panel_power = generic_disable_panel_power,
>> -	.send_otp_cmds = generic_send_otp_cmds,
>>   	.enable = generic_enable,
>>   	.disable = generic_disable,
>> -	.detect = generic_detect,
>> -	.get_hw_state = generic_get_hw_state,
>>   	.get_modes = generic_get_modes,
>> -	.destroy = generic_destroy,
>>   };
>>
>
> Regards
> Shobhit
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center


More information about the Intel-gfx mailing list