Re: [v3 6/7] drm/mediatek: change the dsi phytiming calculate method

Hi Jitao,

On Sun, May 19, 2019 at 2:27 AM Jitao Shi jitao.shi@mediatek.com wrote:

Change the method of frame rate calc which can get more accurate frame rate.

data rate = pixel_clock * bit_per_pixel / lanes Adjust hfp_wc to adapt the additional phy_data

if MIPI_DSI_MODE_VIDEO_BURST hfp_wc = hfp * bpp - data_phy_cycles * lanes - 12 - 6; else hfp_wc = hfp * bpp - data_phy_cycles * lanes - 12;

Note: //(2: 1 for sync, 1 for phy idle) data_phy_cycles = T_hs_exit + T_lpx + T_hs_prepare + T_hs_zero + 2;

bpp: bit per pixel

Signed-off-by: Jitao Shi jitao.shi@mediatek.com

drivers/gpu/drm/mediatek/mtk_dsi.c | 119 +++++++++++++++++++++-------- 1 file changed, 86 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 1165ff944889..3f51b2000c68 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -158,6 +158,25 @@ (type == MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM) || \ (type == MIPI_DSI_DCS_READ))

+struct mtk_phy_timing {

•   u32 lpx;
  

•   u32 da_hs_prepare;
  

•   u32 da_hs_zero;
  

•   u32 da_hs_trail;
  

•   u32 ta_go;
  

•   u32 ta_sure;
  

•   u32 ta_get;
  

•   u32 da_hs_exit;
  

•   u32 clk_hs_zero;
  

•   u32 clk_hs_trail;
  

•   u32 clk_hs_prepare;
  

•   u32 clk_hs_post;
  

•   u32 clk_hs_exit;
  

+};

struct phy;

struct mtk_dsi_driver_data { @@ -182,12 +201,13 @@ struct mtk_dsi { struct clk *digital_clk; struct clk *hs_clk;

•   u32 data_rate;
  

•   u64 data_rate;
  
    unsigned long mode_flags;
    enum mipi_dsi_pixel_format format;
    unsigned int lanes;
    struct videomode vm;
  

•   struct mtk_phy_timing phy_timing;
    int refcount;
    bool enabled;
    u32 irq_data;
  

@@ -221,17 +241,39 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi) { u32 timcon0, timcon1, timcon2, timcon3; u32 ui, cycle_time;

•   struct mtk_phy_timing *timing = &dsi->phy_timing;
  

•   ui = 1000000000 / dsi->data_rate;
  

•   cycle_time = 8000000000 / dsi->data_rate;
  

•   timing->lpx = NS_TO_CYCLE(60, cycle_time);
  

•   timing->da_hs_prepare = NS_TO_CYCLE((40 + 5 * ui), cycle_time);
  

•   timing->da_hs_zero = NS_TO_CYCLE((110 + 6 * ui), cycle_time);
  

•   timing->da_hs_trail = NS_TO_CYCLE(((0x4 * ui) + 80), cycle_time);
  

•   if (timing->da_hs_zero > timing->da_hs_prepare)
  

•           timing->da_hs_zero -= timing->da_hs_prepare;
  

I don't follow why the above comparison and subtraction is necessary when the values are being explicitly set immediately prior and it seems to introduce a bug. Leftover from an early revision?

It looks like you've tuned the values such that hs_prepare+hs_zero are just above the minimum requirements for that sum, however due to this comparison and subtraction we wind up with a value of hs_prepare+hs_zero-hs_prepare and fall below spec. Either boosting the initial value set for hs_zero or removing the comparison makes display happy again. Since I don't see any reason for the compare and subtract I'd just drop that.

•   timing->ta_go = 4 * timing->lpx;
  

•   timing->ta_sure = 3 * timing->lpx / 2;
  

•   timing->ta_get = 5 * timing->lpx;
  

•   timing->da_hs_exit = 2 * timing->lpx;
  

•   timing->clk_hs_zero = NS_TO_CYCLE(0x150, cycle_time);
  

•   timing->clk_hs_trail = NS_TO_CYCLE(0x64, cycle_time) + 0xa;
  
  

•   ui = 1000 / dsi->data_rate + 0x01;
  

•   cycle_time = 8000 / dsi->data_rate + 0x01;
  

•   timing->clk_hs_prepare = NS_TO_CYCLE(0x40, cycle_time);
  

•   timing->clk_hs_post = NS_TO_CYCLE(80 + 52 * ui, cycle_time);
  

•   timing->clk_hs_exit = 2 * timing->lpx;
  

There is a lot of alternating between hex and decimal values in this function which makes it a little hard to follow. Would be nice to stick to one or the other.

•   timcon0 = T_LPX | T_HS_PREP << 8 | T_HS_ZERO << 16 | T_HS_TRAIL << 24;
  

•   timcon1 = 4 * T_LPX | (3 * T_LPX / 2) << 8 | 5 * T_LPX << 16 |
  

•             T_HS_EXIT << 24;
  

•   timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
  

•             (NS_TO_CYCLE(0x150, cycle_time) << 16);
  

•   timcon3 = NS_TO_CYCLE(0x40, cycle_time) | (2 * T_LPX) << 16 |
  

•             NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8;
  

•   timcon0 = timing->lpx | timing->da_hs_prepare << 8 |
  

•             timing->da_hs_zero << 16 | timing->da_hs_trail << 24;
  

•   timcon1 = timing->ta_go | timing->ta_sure << 8 |
  

•             timing->ta_get << 16 | timing->da_hs_exit << 24;
  

•   timcon2 = 1 << 8 | timing->clk_hs_zero << 16 |
  

•             timing->clk_hs_trail << 24;
  

•   timcon3 = timing->clk_hs_prepare | timing->clk_hs_post << 8 |
  

•             timing->clk_hs_exit << 16;
  
    writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
    writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
  

@@ -418,7 +460,8 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) u32 horizontal_sync_active_byte; u32 horizontal_backporch_byte; u32 horizontal_frontporch_byte;

•   u32 dsi_tmp_buf_bpp;
  

•   u32 dsi_tmp_buf_bpp, data_phy_cycles;
  

•   struct mtk_phy_timing *timing = &dsi->phy_timing;
  
    struct videomode *vm = &dsi->vm;
  
  

@@ -433,7 +476,8 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) writel(vm->vactive, dsi->regs + DSI_VACT_NL);

    if (dsi->driver_data->has_size_ctl)

•           writel(vm->vactive << 16 | vm->hactive, dsi->regs + DSI_SIZE_CON);
  

•           writel(vm->vactive << 16 | vm->hactive,
  

•                  dsi->regs + DSI_SIZE_CON);
  
    horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
  
  

@@ -444,7 +488,34 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) horizontal_backporch_byte = ((vm->hback_porch + vm->hsync_len) * dsi_tmp_buf_bpp - 10);

•   horizontal_frontporch_byte = (vm->hfront_porch * dsi_tmp_buf_bpp - 12);
  

•   data_phy_cycles = timing->lpx + timing->da_hs_prepare +
  

•                             timing->da_hs_zero + timing->da_hs_exit + 2;
  

•   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
  

•           if (vm->hfront_porch * dsi_tmp_buf_bpp >
  

•               data_phy_cycles * dsi->lanes + 18) {
  

•                   horizontal_frontporch_byte = vm->hfront_porch *
  

•                                                dsi_tmp_buf_bpp -
  

•                                                data_phy_cycles *
  

•                                                dsi->lanes - 18;
  

•           } else {
  

•                   DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n");
  

•                   horizontal_frontporch_byte = vm->hfront_porch *
  

•                                                dsi_tmp_buf_bpp;
  

•           }
  

•   } else {
  

•           if (vm->hfront_porch * dsi_tmp_buf_bpp >
  

•               data_phy_cycles * dsi->lanes + 12) {
  

•                   horizontal_frontporch_byte = vm->hfront_porch *
  

•                                                dsi_tmp_buf_bpp -
  

•                                                data_phy_cycles *
  

•                                                dsi->lanes - 12;
  

•           } else {
  

•                   DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n");
  

•                   horizontal_frontporch_byte = vm->hfront_porch *
  

•                                                dsi_tmp_buf_bpp;
  

•           }
  

•   }
  
    writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
    writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
  

@@ -544,8 +615,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) { struct device *dev = dsi->dev; int ret;

•   u64 pixel_clock, total_bits;
  

•   u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
  

•   u32 bit_per_pixel;
  
    if (++dsi->refcount != 1)
            return 0;
  

@@ -564,24 +634,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) break; }

•   /**
  

•    * htotal_time = htotal * byte_per_pixel / num_lanes
  

•    * overhead_time = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
  

•    * mipi_ratio = (htotal_time + overhead_time) / htotal_time
  

•    * data_rate = pixel_clock * bit_per_pixel * mipi_ratio / num_lanes;
  

•    */
  

•   pixel_clock = dsi->vm.pixelclock;
  

•   htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
  

•                   dsi->vm.hsync_len;
  

•   htotal_bits = htotal * bit_per_pixel;
  

•   overhead_cycles = T_LPX + T_HS_PREP + T_HS_ZERO + T_HS_TRAIL +
  

•                   T_HS_EXIT;
  

•   overhead_bits = overhead_cycles * dsi->lanes * 8;
  

•   total_bits = htotal_bits + overhead_bits;
  

•   dsi->data_rate = DIV_ROUND_UP_ULL(pixel_clock * total_bits,
  

•                                     htotal * dsi->lanes);
  

•   dsi->data_rate = dsi->vm.pixelclock * bit_per_pixel / dsi->lanes;
  
    ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
    if (ret < 0) {
  

With the earlier fix feel free to add to the next revision Tested-by: Ryan Case ryandcase@chromium.org