Re: [PATCH] drm/i915/bxt: Added identifier for MIPI transcoder.

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 04 Feb 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 03 Feb 2016, Animesh Manna <animesh.manna@intel.com> wrote:
>> Changes done:
>> - Added identifier for Mipi transcoder A and C.
>> - Added power domain identifier for newly added mipi trancoder.
>> - Initialized transcoder for mipi during compute config.
>
> Please separate the power domain control from the addition of the
> transcoders. I think the former is safer to go in.

Hmm, that probably doesn't work either. But this would require a
thorough checking of *all* uses of cpu_transcoder in bxt code paths,
ensuring we don't accidentally index registers with DSI transcoders. And
you'll also notice the transcoders are not at uniformly spread register
offsets, so you can't just assume whatever value TRANSCODER_MIPI_A will
get will be the right offset when used as cpu_transcoder.

See for example what [1] does and imagine TRANSCODER_MIPI_A for HTOTAL
et al.

BR,
Jani.

[1] http://patchwork.freedesktop.org/patch/msgid/1454502456-10556-2-git-send-email-ramalingam.c@intel.com


>
> BR,
> Jani.
>
>
>>
>> v1: Initial RFC version.
>>
>> v2: Rebased on tot.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h         | 4 ++++
>>  drivers/gpu/drm/i915/intel_dsi.c        | 8 ++++++++
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 6 ++++++
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 65a2cd0..c484df8 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -122,6 +122,8 @@ enum transcoder {
>>  	TRANSCODER_B,
>>  	TRANSCODER_C,
>>  	TRANSCODER_EDP,
>> +	TRANSCODER_MIPI_A,
>> +	TRANSCODER_MIPI_C,
>>  	I915_MAX_TRANSCODERS
>>  };
>>  #define transcoder_name(t) ((t) + 'A')
>> @@ -176,6 +178,8 @@ enum intel_display_power_domain {
>>  	POWER_DOMAIN_TRANSCODER_B,
>>  	POWER_DOMAIN_TRANSCODER_C,
>>  	POWER_DOMAIN_TRANSCODER_EDP,
>> +	POWER_DOMAIN_TRANSCODER_MIPI_A,
>> +	POWER_DOMAIN_TRANSCODER_MIPI_C,
>>  	POWER_DOMAIN_PORT_DDI_A_LANES,
>>  	POWER_DOMAIN_PORT_DDI_B_LANES,
>>  	POWER_DOMAIN_PORT_DDI_C_LANES,
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 91cef35..4f2b513 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -273,6 +273,7 @@ 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 = &pipe_config->base.adjusted_mode;
>> +	struct drm_device *dev = intel_connector->base.dev;
>>  
>>  	DRM_DEBUG_KMS("\n");
>>  
>> @@ -284,6 +285,13 @@ 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 (IS_BROXTON(dev)) {
>> +		if (intel_dsi->ports & (1 << PORT_A))
>> +			pipe_config->cpu_transcoder = TRANSCODER_MIPI_A;
>> +		else
>> +			pipe_config->cpu_transcoder = TRANSCODER_MIPI_C;
>> +	}
>> +
>>  	return true;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index bbca527..611dc1e 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -87,6 +87,10 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>>  		return "TRANSCODER_B";
>>  	case POWER_DOMAIN_TRANSCODER_C:
>>  		return "TRANSCODER_C";
>> +	case POWER_DOMAIN_TRANSCODER_MIPI_A:
>> +		return "TRANSCODER_MIPI_A";
>> +	case POWER_DOMAIN_TRANSCODER_MIPI_C:
>> +		return "TRANSCODER_MIPI_C";
>>  	case POWER_DOMAIN_TRANSCODER_EDP:
>>  		return "TRANSCODER_EDP";
>>  	case POWER_DOMAIN_PORT_DDI_A_LANES:
>> @@ -403,6 +407,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>>  	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
>>  	BIT(POWER_DOMAIN_PIPE_A) |			\
>>  	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
>> +	BIT(POWER_DOMAIN_TRANSCODER_MIPI_A) |		\
>> +	BIT(POWER_DOMAIN_TRANSCODER_MIPI_C) |		\
>>  	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
>>  	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
>>  	BIT(POWER_DOMAIN_AUX_A) |			\

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx