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

["Singh, Gaurav K" <gaurav.k.singh@intel.com>]

On 12/5/2014 10:24 PM, Siluvery, Arun 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.
>
> regards
> Arun

In Front-Back video mode for MIPI Dual Link the first half of columns of 
pixel is always transmitted by MIPI Port A  and second half in MIPI Port C.
Pixel Overlap Count sets the number of Pixels to be overlapped per half 
of Scanline in Front-Back video mode. In dual link front-back mode,
pixel overlap count needs to be added to HActive to account for the 
additional pixels added due to overlap.
>
>>>>
>>>> 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
>>
>
>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx