[Intel-gfx] [PATCH 3/9] drm/i915: Add support for port enable/disable for dual link configuration
Singh, Gaurav K
gaurav.k.singh at intel.com
Mon Dec 1 10:05:41 PST 2014
On 12/1/2014 7:41 PM, Jani Nikula wrote:
> On Mon, 01 Dec 2014, Jani Nikula <jani.nikula at linux.intel.com> wrote:
>> On Sat, 29 Nov 2014, Gaurav K Singh <gaurav.k.singh at intel.com> wrote:
>>> For Dual Link MIPI Panels, both Port A and Port C should be enabled
>>> during the MIPI encoder enabling sequence. Similarly, during the
>>> disabling sequence, both ports needs to be disabled.
>>>
>>> v2: Used for_each_dsi_port macro instead of for loop
>>>
>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh at intel.com>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar at intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h | 1 +
>>> drivers/gpu/drm/i915/intel_dsi.c | 39 +++++++++++++++++++---------
>>> drivers/gpu/drm/i915/intel_dsi.h | 1 +
>>> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 7 +++++
>>> 4 files changed, 36 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index dc03fac..c981f5d 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -6664,6 +6664,7 @@ enum punit_power_well {
>>> #define DPI_ENABLE (1 << 31) /* A + C */
>>> #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
>>> #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
>>> +#define DUAL_LINK_MODE_SHIFT 26
>>> #define DUAL_LINK_MODE_MASK (1 << 26)
>>> #define DUAL_LINK_MODE_FRONT_BACK (0 << 26)
>>> #define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>> index 693736b..1163a5b 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>> @@ -108,28 +108,43 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> - enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>>> + enum port port;
>>> u32 temp;
>>>
>>> - /* assert ip_tg_enable signal */
>>> - temp = I915_READ(MIPI_PORT_CTRL(port)) & ~LANE_CONFIGURATION_MASK;
>>> - temp = temp | intel_dsi->port_bits;
>>> - I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
>>> - POSTING_READ(MIPI_PORT_CTRL(port));
>>> + for_each_dsi_port(port, intel_dsi->ports) {
>>> + temp = I915_READ(MIPI_PORT_CTRL(port));
>>> +
>>> + if (intel_dsi->dual_link) {
>>> + if (port == PORT_A)
>>> + intel_dsi->port_bits |= intel_crtc->pipe ?
>>> + LANE_CONFIGURATION_DUAL_LINK_B :
>>> + LANE_CONFIGURATION_DUAL_LINK_A;
>>> + else
>>> + intel_dsi->port_bits = 0;
>> It feels wrong to clobber intel_dsi->port_bits here; the old code didn't
>> do that either. I think you should either set port_bits somewhere else
>> (or remove it altogether), and then modify temp depending on port.
>>
>> Side note, it seems to me intel_dsi->dual_link is becoming redundant, as
>> intel_dsi->ports will have more than one bit set in the dual link
>> case. This can be a future cleanup though.
> Okay, so it's becoming redundant for checking whether we are using dual
> link or not, but it still has it's place for indicating which dual link
> mode to use.
>
> BR,
> Jani.
Hi Jani,
I preferred if (intel_dsi->dual_link) than if (intel_dsi->ports ==((1 <<
PORT_A) | (1 << PORT_C))), as it looks more cleaner than the second one.
Regarding port_bits, in the old code for single link DSI panels, this
variable was always 0, and it did not had any use. But with dual link
configuration, there are few more register bits which we need to
configure, since the function name was intel_dsi_port_enable, i thought
of setting the corresponding mipi port_bits variable in this function
itself.
With regards,
Gaurav
>
>> BR,
>> Jani.
>>
>>> + } else
>>> + temp &= ~LANE_CONFIGURATION_MASK;
>>> +
>>> + /* assert ip_tg_enable signal */
>>> + temp = temp | intel_dsi->port_bits;
>>> + I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
>>> + POSTING_READ(MIPI_PORT_CTRL(port));
>>> + }
>>> }
>>>
>>> static void intel_dsi_port_disable(struct intel_encoder *encoder)
>>> {
>>> struct drm_device *dev = encoder->base.dev;
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>> - enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>>> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> + enum port port;
>>> u32 temp;
>>>
>>> - /* de-assert ip_tg_enable signal */
>>> - temp = I915_READ(MIPI_PORT_CTRL(port));
>>> - I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
>>> - POSTING_READ(MIPI_PORT_CTRL(port));
>>> + for_each_dsi_port(port, intel_dsi->ports) {
>>> + /* de-assert ip_tg_enable signal */
>>> + temp = I915_READ(MIPI_PORT_CTRL(port));
>>> + I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
>>> + POSTING_READ(MIPI_PORT_CTRL(port));
>>> + }
>>> }
>>>
>>> static void intel_dsi_device_ready(struct intel_encoder *encoder)
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>>> index 7f5d028..f2cc2fc 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>>> @@ -104,6 +104,7 @@ struct intel_dsi {
>>> u8 clock_stop;
>>>
>>> u8 escape_clk_div;
>>> + u8 dual_link;
>>> u32 port_bits;
>>> u32 bw_timer;
>>> u32 dphy_reg;
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>>> index 612592f..7f1ba58 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>>> @@ -287,6 +287,13 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>> intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
>>> intel_dsi->lane_count = mipi_config->lane_cnt + 1;
>>> intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
>>> + intel_dsi->dual_link = mipi_config->dual_link;
>>> +
>>> + if (intel_dsi->dual_link) {
>>> + intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
>>> + intel_dsi->port_bits = (intel_dsi->dual_link - 1)
>>> + << DUAL_LINK_MODE_SHIFT;
>>> + }
>>>
>>> if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
>>> bits_per_pixel = 18;
>>> --
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> 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