[Intel-gfx] [BXT MIPI PATCH v5 05/14] drm/i915/bxt: DSI encoder support in CRTC modeset
Shankar, Uma
uma.shankar at intel.com
Mon Oct 5 09:06:17 PDT 2015
>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Friday, October 2, 2015 6:05 PM
>To: Shankar, Uma
>Cc: intel-gfx at lists.freedesktop.org; Kumar, Shobhit
>Subject: Re: [Intel-gfx] [BXT MIPI PATCH v5 05/14] drm/i915/bxt: DSI encoder
>support in CRTC modeset
>
>On Thu, Oct 01, 2015 at 10:23:49PM +0530, Uma Shankar wrote:
>> From: Shashank Sharma <shashank.sharma at intel.com>
>>
>> SKL and BXT qualifies the HAS_DDI() check, and hence haswell modeset
>> functions are re-used for modeset sequence. But DDI interface doesn't
>> include support for DSI.
>> This patch adds:
>> 1. cases for DSI encoder, in those modeset functions and allows
>> a CRTC modeset
>> 2. Adds call to pre_pll enabled from CRTC modeset function. Nothing
>> needs to be done as such in CRTC for DSI encoder, as PLL, clock
>> and and transcoder programming will be taken care in encoder's
>> pre_enable and pre_pll_enable function.
>>
>> v2: Fixed Jani's review comments. Added INVALID_PORT for non DDI
>> encoder like DSI for platforms having HAS_DDI as true.
>>
>> v3: Rebased on latest drm-nightly branch. Added a WARN_ON for invalid
>> encoder.
>>
>> v4: WARN_ON for invalid encoder is refactored as per Jani's suggestion.
>> Fixed the sequence for pre_pll_enable.
>>
>> v5: Protected DDI code paths in case of DSI encoder calls.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>
>Ok, after this patch we get stuff like this:
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> if (encoder->pre_pll_enable)
> encoder->pre_pll_enable(encoder);
> if (encoder->pre_enable)
> encoder->pre_enable(encoder);
> }
>
> if (intel_crtc->config->has_pch_encoder) {
> intel_set_pch_fifo_underrun_reporting(dev_priv,
>TRANSCODER_A,
> true);
> dev_priv->display.fdi_link_train(crtc);
> }
>
> if (!is_dsi)
> intel_ddi_enable_pipe_clock(intel_crtc);
>
>1. Please remove pre_pll_enable again, we don't need 2 callbacks in exactly the
>same spot. Yes this might mean that you need special bxt_ versions of that in the
>dsi encoder, we have that everywhere.
>
>2. the has_pch_encoder is already something encoder-specific (it's exclusively
>used by the HSW LPT CRT encoder). Now we have another one of those for the
>!is_dsi case. These special-cases should be moved into the
>encoder->pre_enable callbacks, that's what they're for.
>
>I'm not going to block these patches are (18months is already ridiculous), but I
>want this cleanup done. Uma, can you pls own this? If you can't do it yourself
>please escalate to Indranil so he can find someone.
>
>Thanks, Daniel
>
Hi Daniel,
I will discuss with Indranil and will get this done.
Thanks for your support in getting the video mode patches merged.
Regards,
Uma Shankar
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++--
>> drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++------
>> drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++--
>> 3 files changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index cacb07b..7b7f544 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -390,8 +390,10 @@ void intel_prepare_ddi(struct drm_device *dev)
>> enum port port;
>> bool supports_hdmi;
>>
>> - ddi_get_encoder_port(intel_encoder, &intel_dig_port, &port);
>> + if (intel_encoder->type == INTEL_OUTPUT_DSI)
>> + continue;
>>
>> + ddi_get_encoder_port(intel_encoder, &intel_dig_port, &port);
>> if (visited[port])
>> continue;
>>
>> @@ -1779,7 +1781,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder
>> *encoder, void intel_ddi_enable_pipe_clock(struct intel_crtc
>> *intel_crtc) {
>> struct drm_crtc *crtc = &intel_crtc->base;
>> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>> + struct drm_device *dev = crtc->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
>> enum port port = intel_ddi_get_encoder_port(intel_encoder);
>> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index b8e0310..ea0f533 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4991,6 +4991,7 @@ static void haswell_crtc_enable(struct drm_crtc
>*crtc)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> struct intel_encoder *encoder;
>> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>> int pipe = intel_crtc->pipe;
>>
>> WARN_ON(!crtc->state->enable);
>> @@ -5023,9 +5024,12 @@ static void haswell_crtc_enable(struct drm_crtc
>*crtc)
>> intel_crtc->active = true;
>>
>> intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>> - for_each_encoder_on_crtc(dev, crtc, encoder)
>> + for_each_encoder_on_crtc(dev, crtc, encoder) {
>> + if (encoder->pre_pll_enable)
>> + encoder->pre_pll_enable(encoder);
>> if (encoder->pre_enable)
>> encoder->pre_enable(encoder);
>> + }
>>
>> if (intel_crtc->config->has_pch_encoder) {
>> intel_set_pch_fifo_underrun_reporting(dev_priv,
>TRANSCODER_A, @@
>> -5033,7 +5037,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>> dev_priv->display.fdi_link_train(crtc);
>> }
>>
>> - intel_ddi_enable_pipe_clock(intel_crtc);
>> + if (!is_dsi)
>> + intel_ddi_enable_pipe_clock(intel_crtc);
>>
>> if (INTEL_INFO(dev)->gen == 9)
>> skylake_pfit_update(intel_crtc, 1); @@ -5049,7 +5054,8 @@
>static
>> void haswell_crtc_enable(struct drm_crtc *crtc)
>> intel_crtc_load_lut(crtc);
>>
>> intel_ddi_set_pipe_settings(crtc);
>> - intel_ddi_enable_transcoder_func(crtc);
>> + if (!is_dsi)
>> + intel_ddi_enable_transcoder_func(crtc);
>>
>> intel_update_watermarks(crtc);
>> intel_enable_pipe(intel_crtc);
>> @@ -5057,7 +5063,7 @@ static void haswell_crtc_enable(struct drm_crtc
>*crtc)
>> if (intel_crtc->config->has_pch_encoder)
>> lpt_pch_enable(crtc);
>>
>> - if (intel_crtc->config->dp_encoder_is_mst)
>> + if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
>> intel_ddi_set_vc_payload_alloc(crtc, true);
>>
>> assert_vblank_disabled(crtc);
>> @@ -5159,6 +5165,7 @@ static void haswell_crtc_disable(struct drm_crtc
>*crtc)
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> struct intel_encoder *encoder;
>> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>>
>> if (!intel_crtc->active)
>> return;
>> @@ -5179,7 +5186,8 @@ static void haswell_crtc_disable(struct drm_crtc
>*crtc)
>> if (intel_crtc->config->dp_encoder_is_mst)
>> intel_ddi_set_vc_payload_alloc(crtc, false);
>>
>> - intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
>> + if (!is_dsi)
>> + intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
>>
>> if (INTEL_INFO(dev)->gen == 9)
>> skylake_pfit_update(intel_crtc, 0); @@ -5188,7 +5196,8 @@
>static
>> void haswell_crtc_disable(struct drm_crtc *crtc)
>> else
>> MISSING_CASE(INTEL_INFO(dev)->gen);
>>
>> - intel_ddi_disable_pipe_clock(intel_crtc);
>> + if (!is_dsi)
>> + intel_ddi_disable_pipe_clock(intel_crtc);
>>
>> if (intel_crtc->config->has_pch_encoder) {
>> lpt_disable_pch_transcoder(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
>> b/drivers/gpu/drm/i915/intel_opregion.c
>> index 4813374..db518ef 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -334,8 +334,12 @@ int intel_opregion_notify_encoder(struct
>intel_encoder *intel_encoder,
>> if (!HAS_DDI(dev))
>> return 0;
>>
>> - port = intel_ddi_get_encoder_port(intel_encoder);
>> - if (port == PORT_E) {
>> + if (intel_encoder->type == INTEL_OUTPUT_DSI)
>> + port = 0;
>> + else
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> +
>> + if (port == PORT_E) {
>> port = 0;
>> } else {
>> parm |= 1 << port;
>> @@ -356,6 +360,7 @@ int intel_opregion_notify_encoder(struct
>intel_encoder *intel_encoder,
>> type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
>> break;
>> case INTEL_OUTPUT_EDP:
>> + case INTEL_OUTPUT_DSI:
>> type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
>> break;
>> default:
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
More information about the Intel-gfx
mailing list