Re: [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link

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

On Fri, 05 Dec 2014, "Siluvery, Arun" <arun.siluvery@linux.intel.com> wrote:
> On 05/12/2014 16:33, Singh, Gaurav K wrote:
>>
>> On 12/4/2014 2:57 PM, Jani Nikula wrote:
>>> On Thu, 04 Dec 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>>>> For dual link MIPI Panels, each port needs half of pixel clock. Pixel overlap
>>>> can be enabled if needed by panel, then in that case, pixel clock will be
>>>> increased for extra pixels.
>
> just a question, why do we need pixel overlap?
> I couldn't find more details from spec other than that when overlap is 
> set some extra pixels are sent.

From the host perspective a dual link (or dual channel) DSI device is
two independent peripheral devices. On the peripheral side the display
has to combine the input from the two links (which may be two
independent DSI blocks on the peripheral as well) into one contiguous
display. I don't know the details, but I'm guessing pixel overlap just
makes it easier for the peripheral implementation to get it all
together.

BR,
Jani.

>
> regards
> Arun
>
>>>>
>>>> v2 : Address review comments by Jani
>>>>        - Removed the bit mask used for ->dual_link
>>>>        - Used DSI instead of MIPI for #define variables
>>>>
>>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_reg.h            |    4 ++++
>>>>    drivers/gpu/drm/i915/intel_bios.h          |    3 ++-
>>>>    drivers/gpu/drm/i915/intel_dsi.c           |    8 ++++++++
>>>>    drivers/gpu/drm/i915/intel_dsi.h           |    6 ++++++
>>>>    drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   21 +++++++++++++++++++++
>>>>    5 files changed, 41 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>> index c981f5d..87149ba 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -6029,6 +6029,10 @@ enum punit_power_well {
>>>>    #define GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
>>>>    #define VLV_PWRDWNUPCTL				0xA294
>>>>
>>>> +#define VLV_CHICKEN_3				0x7040C
>>>> +#define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
>>>> +#define  PIXEL_OVERLAP_CNT_SHIFT		30
>>> I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
>>>
>>> Given that I can't find the register my review is pretty shallow, but I
>>> don't spot anything obviously wrong either. With these caveats,
>>>
>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> This reg is available in BSpec though the bit definitions have not been updated in the BSpec. Also, it was communicated by the BIOS team.
>>>> +
>>>>    #define GEN6_PMISR				0x44020
>>>>    #define GEN6_PMIMR				0x44024 /* rps_lock */
>>>>    #define GEN6_PMIIR				0x44028
>>>> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
>>>> index de01167..a6a8710 100644
>>>> --- a/drivers/gpu/drm/i915/intel_bios.h
>>>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>>>> @@ -818,7 +818,8 @@ struct mipi_config {
>>>>    #define DUAL_LINK_PIXEL_ALT	2
>>>>    	u16 dual_link:2;
>>>>    	u16 lane_cnt:2;
>>>> -	u16 rsvd3:12;
>>>> +	u16 pixel_overlap:3;
>>>> +	u16 rsvd3:9;
>>>>
>>>>    	u16 rsvd4;
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>>> index dbe52e9..4e18abd 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>>> @@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>>>>    	enum port port;
>>>>    	u32 temp;
>>>>
>>>> +	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>>>> +		temp = I915_READ(VLV_CHICKEN_3);
>>>> +		temp &= ~PIXEL_OVERLAP_CNT_MASK |
>>>> +					intel_dsi->pixel_overlap <<
>>>> +					PIXEL_OVERLAP_CNT_SHIFT;
>>>> +		I915_WRITE(VLV_CHICKEN_3, temp);
>>>> +	}
>>>> +
>>>>    	for_each_dsi_port(port, intel_dsi->ports) {
>>>>    		temp = I915_READ(MIPI_PORT_CTRL(port));
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>>>> index f2cc2fc..8fe2064 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>>>> @@ -28,6 +28,11 @@
>>>>    #include <drm/drm_crtc.h>
>>>>    #include "intel_drv.h"
>>>>
>>>> +/* Dual Link support */
>>>> +#define DSI_DUAL_LINK_NONE		0
>>>> +#define DSI_DUAL_LINK_FRONT_BACK	1
>>>> +#define DSI_DUAL_LINK_PIXEL_ALT		2
>>>> +
>>>>    struct intel_dsi_device {
>>>>    	unsigned int panel_id;
>>>>    	const char *name;
>>>> @@ -105,6 +110,7 @@ struct intel_dsi {
>>>>
>>>>    	u8 escape_clk_div;
>>>>    	u8 dual_link;
>>>> +	u8 pixel_overlap;
>>>>    	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 f60146f..f8c2269 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>>>> @@ -288,6 +288,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>>>    	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;
>>>> +	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>>>>
>>>>    	if (intel_dsi->dual_link)
>>>>    		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
>>>> @@ -310,6 +311,20 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>>>
>>>>    	pclk = mode->clock;
>>>>
>>>> +	/* In dual link mode each port needs half of pixel clock */
>>>> +	if (intel_dsi->dual_link) {
>>>> +		pclk = pclk / 2;
>>>> +
>>>> +		/* we can enable pixel_overlap if needed by panel. In this
>>>> +		 * case we need to increase the pixelclock for extra pixels
>>>> +		 */
>>>> +		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>>>> +			pclk += DIV_ROUND_UP(mode->vtotal *
>>>> +						intel_dsi->pixel_overlap *
>>>> +						60, 1000);
>>>> +		}
>>>> +	}
>>>> +
>>>>    	/* Burst Mode Ratio
>>>>    	 * Target ddr frequency from VBT / non burst ddr freq
>>>>    	 * multiply by 100 to preserve remainder
>>>> @@ -504,6 +519,12 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>>>    	DRM_DEBUG_KMS("Clockstop %s\n", intel_dsi->clock_stop ?
>>>>    						"disabled" : "enabled");
>>>>    	DRM_DEBUG_KMS("Mode %s\n", intel_dsi->operation_mode ? "command" : "video");
>>>> +	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
>>>> +		DRM_DEBUG_KMS("Dual link: DSI_DUAL_LINK_FRONT_BACK\n");
>>>> +	else if (intel_dsi->dual_link == DSI_DUAL_LINK_PIXEL_ALT)
>>>> +		DRM_DEBUG_KMS("Dual link: DSI_DUAL_LINK_PIXEL_ALT\n");
>>>> +	else
>>>> +		DRM_DEBUG_KMS("Dual link: NONE\n");
>>>>    	DRM_DEBUG_KMS("Pixel Format %d\n", intel_dsi->pixel_format);
>>>>    	DRM_DEBUG_KMS("TLPX %d\n", intel_dsi->escape_clk_div);
>>>>    	DRM_DEBUG_KMS("LP RX Timeout 0x%x\n", intel_dsi->lp_rx_timeout);
>>>> --
>>>> 1.7.9.5
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>

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