| Message ID | 20250327115616.1271635-1-xji@analogixsemi.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] drm/bridge:anx7625: Enable DSC feature | expand |
On Thu, Mar 27, 2025 at 07:56:15PM +0800, Xin Ji wrote: > 4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits RGB > pixel data format, total transport bandwidth need 297M*24(at least > 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, > 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot > receive 4K30 video timing. > > When display pixel clock exceed 250M, driver will enable DSC feature, > and the compression ratio is 3:1, eg: 4K30's pixel clock around 297M, > bandwidth 7.2G will be compressed to 7.2G/3 = 2.4G. Then anx7625 > can receive 4K30 video timing and do decompress, then package video > data and send to sink device through DP link. > > Anx7625 is bridge IC, sink monitor only receive normal DP signal from > anx7625, sink device didn't know DSC information between SOC and anx7625 > > v2: > 1. Add more commit message > 2. Remove unused code > 3. Add more comment This part is useless, it adds no information. It is as good as 'changed it'. > 4. Remove dsc_en flag > > Signed-off-by: Xin Ji <xji@analogixsemi.com> > --- > drivers/gpu/drm/bridge/analogix/anx7625.c | 299 ++++++++++++++++++---- > drivers/gpu/drm/bridge/analogix/anx7625.h | 31 +++ > 2 files changed, 274 insertions(+), 56 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c > index 4be34d5c7a3b..bae76d9a665f 100644 > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > @@ -9,6 +9,7 @@ > #include <linux/interrupt.h> > #include <linux/iopoll.h> > #include <linux/kernel.h> > +#include <linux/math64.h> > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/pm_runtime.h> > @@ -22,6 +23,7 @@ > > #include <drm/display/drm_dp_aux_bus.h> > #include <drm/display/drm_dp_helper.h> > +#include <drm/display/drm_dsc_helper.h> > #include <drm/display/drm_hdcp_helper.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_bridge.h> > @@ -476,11 +478,159 @@ static int anx7625_set_k_value(struct anx7625_data *ctx) > MIPI_DIGITAL_ADJ_1, 0x3D); > } > > +static bool anx7625_dsc_check(struct anx7625_data *ctx) > +{ > + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) > + return true; So, now DSC is enabled unconditionally just because the clock is too high. Let's see... > + > + return false; > +} > + > +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, > + struct i2c_client *client, > + u8 reg_addr, u16 val, > + bool dsc_check) > +{ > + int ret; > + > + if (dsc_check && anx7625_dsc_check(ctx)) > + val = dsc_div(val); > + > + ret = anx7625_reg_write(ctx, client, reg_addr, val); > + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> 8); > + > + return ret; > +} > + > +static int anx7625_h_timing_write(struct anx7625_data *ctx, > + struct i2c_client *client, > + bool rx_h_timing) > +{ > + u16 htotal; > + int ret; > + > + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > + /* Htotal */ > + ret = anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_TOTAL_PIXELS_L, > + htotal, rx_h_timing); > + /* Hactive */ > + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_ACTIVE_PIXELS_L, > + ctx->dt.hactive.min, rx_h_timing); > + /* HFP */ > + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_FRONT_PORCH_L, > + ctx->dt.hfront_porch.min, rx_h_timing); > + /* HWS */ > + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_SYNC_WIDTH_L, > + ctx->dt.hsync_len.min, rx_h_timing); > + /* HBP */ > + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_BACK_PORCH_L, > + ctx->dt.hback_porch.min, rx_h_timing); > + > + return ret; > +} > + > +static int anx7625_v_timing_write(struct anx7625_data *ctx, > + struct i2c_client *client) > +{ > + int ret; > + > + /* Vactive */ > + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, > + ctx->dt.vactive.min); > + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, > + ctx->dt.vactive.min >> 8); > + /* VFP */ > + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, > + ctx->dt.vfront_porch.min); > + /* VWS */ > + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, > + ctx->dt.vsync_len.min); > + /* VBP */ > + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, > + ctx->dt.vback_porch.min); > + > + return ret; > +} > + > +static int anx7625_set_dsc_params(struct anx7625_data *ctx) > +{ > + int ret, i; > + u16 htotal, vtotal; > + > + if (!anx7625_dsc_check(ctx)) > + return 0; > + > + /* Video Horizontal timing */ > + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, false); > + > + /* Video Vertical timing */ > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); > + > + /* Vtotal */ > + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + > + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, > + vtotal); > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, > + vtotal >> 8); > + /* Htotal */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_L_7E, > + htotal); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_H_7E, > + htotal >> 8); > + /* Hactive */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); > + /* HFP */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + HORIZON_FRONT_PORCH_L_7E, > + ctx->dt.hfront_porch.min); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + HORIZON_FRONT_PORCH_H_7E, > + ctx->dt.hfront_porch.min >> 8); > + /* HWS */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + HORIZON_SYNC_WIDTH_L_7E, > + ctx->dt.hsync_len.min); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + HORIZON_SYNC_WIDTH_H_7E, > + ctx->dt.hsync_len.min >> 8); > + /* HBP */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + HORIZON_BACK_PORCH_L_7E, > + ctx->dt.hback_porch.min); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + HORIZON_BACK_PORCH_H_7E, > + ctx->dt.hback_porch.min >> 8); > + > + /* Config DSC decoder internal blank timing for decoder to start */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > + H_BLANK_L, > + dsc_div(htotal - ctx->dt.hactive.min)); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > + H_BLANK_H, > + dsc_div(htotal - ctx->dt.hactive.min) >> 8); > + > + /* Compress ratio RATIO bit[7:6] */ > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, > + (5 - DSC_COMPRESS_RATIO) << 6); > + /*PPS table*/ > + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, > + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, > + &ctx->pps_table[i]); > + > + return ret; > +} > + > static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) > { > struct device *dev = ctx->dev; > unsigned long m, n; > - u16 htotal; > int ret; > u8 post_divider = 0; > > @@ -506,48 +656,12 @@ static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, > MIPI_LANE_CTRL_0, ctx->pdata.mipi_lanes - 1); > > - /* Htotal */ > - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); > - /* Hactive */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); > - /* HFP */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_FRONT_PORCH_H, > - ctx->dt.hfront_porch.min >> 8); > - /* HWS */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); > - /* HBP */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> 8); > - /* Vactive */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, > - ctx->dt.vactive.min); > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, > - ctx->dt.vactive.min >> 8); > - /* VFP */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); > - /* VWS */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); > - /* VBP */ > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); > + /* Video Horizontal timing */ > + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, true); > + > + /* Video Vertical timing */ > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); > + Please split this part into two commits: one refactoring timing programming into two functions and another one introducing DSC support. It is hard to review timing programming otherwise. > /* M value */ > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); > @@ -663,9 +777,15 @@ static int anx7625_dsi_config(struct anx7625_data *ctx) > > DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); > > - /* DSC disable */ > - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > - R_DSC_CTRL_0, ~DSC_EN); > + ret = anx7625_set_dsc_params(ctx); > + if (anx7625_dsc_check(ctx)) > + /* DSC enable */ > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > + R_DSC_CTRL_0, DSC_EN); > + else > + /* DSC disable */ > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > + R_DSC_CTRL_0, ~DSC_EN); > > ret |= anx7625_api_dsi_config(ctx); > > @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct anx7625_data *ctx) > MIPI_DSI_MODE_VIDEO_HSE | > MIPI_DSI_HS_PKT_END_ALIGNED; > > + dsi->dsc = &ctx->dsc; > ctx->dsi = dsi; > > return 0; > @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct drm_bridge *bridge, > struct device *dev = ctx->dev; > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); > + if (mode->clock > SUPPORT_PIXEL_CLOCK) > + return MODE_CLOCK_HIGH; > + > + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) > + return MODE_CLOCK_LOW; > > - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ > - if (mode->clock > SUPPORT_PIXEL_CLOCK) { > - DRM_DEV_DEBUG_DRIVER(dev, > - "drm mode invalid, pixelclock too high.\n"); Any reason for dropping debug message? > + /* > + * If hdisplay cannot be divided by DSC compress ratio, then display > + * will have overlap/shift issue > + */ > + if (mode->clock > DSC_PIXEL_CLOCK && > + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) ... and there still no check that the DSI host supports generating DSC data. Nor there is an atomic_check() that will check if the mode can be enabled. > return MODE_CLOCK_HIGH; > - } > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode valid.\n"); > > return MODE_OK; > } > > +static void anx7625_dsc_enable(struct anx7625_data *ctx, bool en) > +{ > + int ret; > + struct device *dev = ctx->dev; > + > + if (en) { > + ctx->dsc.dsc_version_major = 1; > + ctx->dsc.dsc_version_minor = 1; > + ctx->dsc.slice_height = 8; > + ctx->dsc.slice_width = ctx->dt.hactive.min / DSC_SLICE_NUM; > + ctx->dsc.slice_count = DSC_SLICE_NUM; > + ctx->dsc.bits_per_component = 8; > + ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */ > + ctx->dsc.block_pred_enable = true; > + ctx->dsc.native_420 = false; > + ctx->dsc.native_422 = false; > + ctx->dsc.simple_422 = false; > + ctx->dsc.vbr_enable = false; > + ctx->dsc.convert_rgb = 1; > + ctx->dsc.vbr_enable = 0; Aren't those 'false' and '0' defaults? If so, you don't need to write those fields. > + > + drm_dsc_set_rc_buf_thresh(&ctx->dsc); > + drm_dsc_set_const_params(&ctx->dsc); > + > + ctx->dsc.initial_scale_value = drm_dsc_initial_scale_value(&ctx->dsc); > + ctx->dsc.line_buf_depth = ctx->dsc.bits_per_component + 1; > + ret = drm_dsc_setup_rc_params(&ctx->dsc, DRM_DSC_1_2_444); > + if (ret < 0) > + dev_warn(dev, "drm_dsc_setup_rc_params ret %d\n", ret); > + > + ret = drm_dsc_compute_rc_parameters(&ctx->dsc); > + if (ret) > + dev_warn(dev, "drm dsc compute rc parameters failed ret %d\n", ret); > + > + drm_dsc_pps_payload_pack((struct drm_dsc_picture_parameter_set *)&ctx->pps_table, > + &ctx->dsc); > + dev_dbg(dev, "anx7625 enable dsc\n"); > + } else { > + ctx->dsc.dsc_version_major = 0; > + ctx->dsc.dsc_version_minor = 0; > + dev_dbg(dev, "anx7625 disable dsc\n"); > + } > +} > + > static void anx7625_bridge_mode_set(struct drm_bridge *bridge, > const struct drm_display_mode *old_mode, > const struct drm_display_mode *mode) > @@ -2244,6 +2415,8 @@ static void anx7625_bridge_mode_set(struct drm_bridge *bridge, > DRM_DEV_DEBUG_DRIVER(dev, "vsync_end(%d),vtotal(%d).\n", > mode->vsync_end, > mode->vtotal); > + > + anx7625_dsc_enable(ctx, anx7625_dsc_check(ctx)); > } > > static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > @@ -2258,10 +2431,6 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n"); > > - /* No need fixup for external monitor */ > - if (!ctx->pdata.panel_bridge) > - return true; > - > hsync = mode->hsync_end - mode->hsync_start; > hfp = mode->hsync_start - mode->hdisplay; > hbp = mode->htotal - mode->hsync_end; > @@ -2272,12 +2441,24 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > hsync, hfp, hbp, adj->clock); > DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d), hsync_end(%d), htot(%d)\n", > adj->hsync_start, adj->hsync_end, adj->htotal); > - > adj_hfp = hfp; > adj_hsync = hsync; > adj_hbp = hbp; > adj_hblanking = hblanking; > > + if (mode->clock > DSC_PIXEL_CLOCK) { > + adj_hsync = DSC_HSYNC_LEN; > + adj_hfp = DSC_HFP_LEN; > + adj_hbp = DSC_HBP_LEN; > + vref = (u32)div_u64((u64)adj->clock * 1000 * 1000, > + adj->htotal * adj->vtotal); > + goto calculate_timing; > + } > + > + /* No need fixup for external monitor */ > + if (!ctx->pdata.panel_bridge) > + return true; > + > /* HFP needs to be even */ > if (hfp & 0x1) { > adj_hfp += 1; > @@ -2349,6 +2530,8 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > adj_hfp -= delta_adj; > } > > +calculate_timing: > + > DRM_DEV_DEBUG_DRIVER(dev, "after mode fixup\n"); > DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d), hfp(%d), hbp(%d), clock(%d)\n", > adj_hsync, adj_hfp, adj_hbp, adj->clock); > @@ -2357,6 +2540,10 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > adj->hsync_start = adj->hdisplay + adj_hfp; > adj->hsync_end = adj->hsync_start + adj_hsync; > adj->htotal = adj->hsync_end + adj_hbp; > + if (mode->clock > DSC_PIXEL_CLOCK) > + adj->clock = (u32)div_u64((u64)vref * adj->htotal * adj->vtotal, > + 1000 * 1000); > + > DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d), hsync_end(%d), htot(%d)\n", > adj->hsync_start, adj->hsync_end, adj->htotal); >
> > 4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits RGB > > pixel data format, total transport bandwidth need 297M*24(at least > > 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, > > 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot > > receive 4K30 video timing. > > > > When display pixel clock exceed 250M, driver will enable DSC feature, > > and the compression ratio is 3:1, eg: 4K30's pixel clock around 297M, > > bandwidth 7.2G will be compressed to 7.2G/3 = 2.4G. Then anx7625 can > > receive 4K30 video timing and do decompress, then package video data > > and send to sink device through DP link. > > > > Anx7625 is bridge IC, sink monitor only receive normal DP signal from > > anx7625, sink device didn't know DSC information between SOC and > > anx7625 > > > > v2: > > 1. Add more commit message > > 2. Remove unused code > > 3. Add more comment > > This part is useless, it adds no information. It is as good as 'changed it'. OK, I'll remove it > > > 4. Remove dsc_en flag > > > > Signed-off-by: Xin Ji <xji@analogixsemi.com> > > --- > > drivers/gpu/drm/bridge/analogix/anx7625.c | 299 > > ++++++++++++++++++---- drivers/gpu/drm/bridge/analogix/anx7625.h | > > 31 +++ > > 2 files changed, 274 insertions(+), 56 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > index 4be34d5c7a3b..bae76d9a665f 100644 > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > @@ -9,6 +9,7 @@ > > #include <linux/interrupt.h> > > #include <linux/iopoll.h> > > #include <linux/kernel.h> > > +#include <linux/math64.h> > > #include <linux/module.h> > > #include <linux/mutex.h> > > #include <linux/pm_runtime.h> > > @@ -22,6 +23,7 @@ > > > > #include <drm/display/drm_dp_aux_bus.h> #include > > <drm/display/drm_dp_helper.h> > > +#include <drm/display/drm_dsc_helper.h> > > #include <drm/display/drm_hdcp_helper.h> #include > > <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ -476,11 > > +478,159 @@ static int anx7625_set_k_value(struct anx7625_data *ctx) > > MIPI_DIGITAL_ADJ_1, 0x3D); } > > > > +static bool anx7625_dsc_check(struct anx7625_data *ctx) { > > + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) > > + return true; > > So, now DSC is enabled unconditionally just because the clock is too high. Let's > see... Yes > > > + > > + return false; > > +} > > + > > +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, > > + struct i2c_client *client, > > + u8 reg_addr, u16 val, > > + bool dsc_check) { > > + int ret; > > + > > + if (dsc_check && anx7625_dsc_check(ctx)) > > + val = dsc_div(val); > > + > > + ret = anx7625_reg_write(ctx, client, reg_addr, val); > > + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> 8); > > + > > + return ret; > > +} > > + > > +static int anx7625_h_timing_write(struct anx7625_data *ctx, > > + struct i2c_client *client, > > + bool rx_h_timing) { > > + u16 htotal; > > + int ret; > > + > > + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > + /* Htotal */ > > + ret = anx7625_h_timing_reg_write(ctx, client, > HORIZONTAL_TOTAL_PIXELS_L, > > + htotal, rx_h_timing); > > + /* Hactive */ > > + ret |= anx7625_h_timing_reg_write(ctx, client, > HORIZONTAL_ACTIVE_PIXELS_L, > > + ctx->dt.hactive.min, rx_h_timing); > > + /* HFP */ > > + ret |= anx7625_h_timing_reg_write(ctx, client, > HORIZONTAL_FRONT_PORCH_L, > > + ctx->dt.hfront_porch.min, rx_h_timing); > > + /* HWS */ > > + ret |= anx7625_h_timing_reg_write(ctx, client, > HORIZONTAL_SYNC_WIDTH_L, > > + ctx->dt.hsync_len.min, rx_h_timing); > > + /* HBP */ > > + ret |= anx7625_h_timing_reg_write(ctx, client, > HORIZONTAL_BACK_PORCH_L, > > + ctx->dt.hback_porch.min, > > + rx_h_timing); > > + > > + return ret; > > +} > > + > > +static int anx7625_v_timing_write(struct anx7625_data *ctx, > > + struct i2c_client *client) { > > + int ret; > > + > > + /* Vactive */ > > + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, > > + ctx->dt.vactive.min); > > + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, > > + ctx->dt.vactive.min >> 8); > > + /* VFP */ > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, > > + ctx->dt.vfront_porch.min); > > + /* VWS */ > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, > > + ctx->dt.vsync_len.min); > > + /* VBP */ > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, > > + ctx->dt.vback_porch.min); > > + > > + return ret; > > +} > > + > > +static int anx7625_set_dsc_params(struct anx7625_data *ctx) { > > + int ret, i; > > + u16 htotal, vtotal; > > + > > + if (!anx7625_dsc_check(ctx)) > > + return 0; > > + > > + /* Video Horizontal timing */ > > + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, false); > > + > > + /* Video Vertical timing */ > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); > > + > > + /* Vtotal */ > > + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + > > + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, > > + vtotal); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, > > + vtotal >> 8); > > + /* Htotal */ > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_L_7E, > > + htotal); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_H_7E, > > + htotal >> 8); > > + /* Hactive */ > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); > > + /* HFP */ > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + HORIZON_FRONT_PORCH_L_7E, > > + ctx->dt.hfront_porch.min); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + HORIZON_FRONT_PORCH_H_7E, > > + ctx->dt.hfront_porch.min >> 8); > > + /* HWS */ > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + HORIZON_SYNC_WIDTH_L_7E, > > + ctx->dt.hsync_len.min); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + HORIZON_SYNC_WIDTH_H_7E, > > + ctx->dt.hsync_len.min >> 8); > > + /* HBP */ > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + HORIZON_BACK_PORCH_L_7E, > > + ctx->dt.hback_porch.min); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > + HORIZON_BACK_PORCH_H_7E, > > + ctx->dt.hback_porch.min >> 8); > > + > > + /* Config DSC decoder internal blank timing for decoder to start */ > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > + H_BLANK_L, > > + dsc_div(htotal - ctx->dt.hactive.min)); > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > + H_BLANK_H, > > + dsc_div(htotal - ctx->dt.hactive.min) > > + >> 8); > > + > > + /* Compress ratio RATIO bit[7:6] */ > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, > > + (5 - DSC_COMPRESS_RATIO) << 6); > > + /*PPS table*/ > > + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) > > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, > > + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, > > + &ctx->pps_table[i]); > > + > > + return ret; > > +} > > + > > static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) > > { > > struct device *dev = ctx->dev; > > unsigned long m, n; > > - u16 htotal; > > int ret; > > u8 post_divider = 0; > > > > @@ -506,48 +656,12 @@ static int anx7625_dsi_video_timing_config(struct > anx7625_data *ctx) > > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, > > MIPI_LANE_CTRL_0, ctx->pdata.mipi_lanes > > - 1); > > > > - /* Htotal */ > > - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); > > - /* Hactive */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); > > - /* HFP */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_FRONT_PORCH_H, > > - ctx->dt.hfront_porch.min >> 8); > > - /* HWS */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); > > - /* HBP */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> 8); > > - /* Vactive */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, > > - ctx->dt.vactive.min); > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, > > - ctx->dt.vactive.min >> 8); > > - /* VFP */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); > > - /* VWS */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); > > - /* VBP */ > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); > > + /* Video Horizontal timing */ > > + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, true); > > + > > + /* Video Vertical timing */ > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); > > + > > Please split this part into two commits: one refactoring timing programming into > two functions and another one introducing DSC support. > It is hard to review timing programming otherwise. OK > > > /* M value */ > > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); @@ > > -663,9 +777,15 @@ static int anx7625_dsi_config(struct anx7625_data > > *ctx) > > > > DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); > > > > - /* DSC disable */ > > - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > - R_DSC_CTRL_0, ~DSC_EN); > > + ret = anx7625_set_dsc_params(ctx); > > + if (anx7625_dsc_check(ctx)) > > + /* DSC enable */ > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > > + R_DSC_CTRL_0, DSC_EN); > > + else > > + /* DSC disable */ > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > + R_DSC_CTRL_0, ~DSC_EN); > > > > ret |= anx7625_api_dsi_config(ctx); > > > > @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct > anx7625_data *ctx) > > MIPI_DSI_MODE_VIDEO_HSE | > > MIPI_DSI_HS_PKT_END_ALIGNED; > > > > + dsi->dsc = &ctx->dsc; > > ctx->dsi = dsi; > > > > return 0; > > @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct drm_bridge > *bridge, > > struct device *dev = ctx->dev; > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); > > + if (mode->clock > SUPPORT_PIXEL_CLOCK) > > + return MODE_CLOCK_HIGH; > > + > > + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) > > + return MODE_CLOCK_LOW; > > > > - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ > > - if (mode->clock > SUPPORT_PIXEL_CLOCK) { > > - DRM_DEV_DEBUG_DRIVER(dev, > > - "drm mode invalid, pixelclock too high.\n"); > > Any reason for dropping debug message? I'll keep this message. > > > + /* > > + * If hdisplay cannot be divided by DSC compress ratio, then display > > + * will have overlap/shift issue > > + */ > > + if (mode->clock > DSC_PIXEL_CLOCK && > > + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) > > > ... and there still no check that the DSI host supports generating DSC data. Nor > there is an atomic_check() that will check if the mode can be enabled. I cannot know whether the DSI host supports DSC or not. Anx7625 driver force enable DSC feature if pixel clock higher than DSC_PIXEL_CLOCK. > > > return MODE_CLOCK_HIGH; > > - } > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode valid.\n"); > > > > return MODE_OK; > > } > > > > +static void anx7625_dsc_enable(struct anx7625_data *ctx, bool en) { > > + int ret; > > + struct device *dev = ctx->dev; > > + > > + if (en) { > > + ctx->dsc.dsc_version_major = 1; > > + ctx->dsc.dsc_version_minor = 1; > > + ctx->dsc.slice_height = 8; > > + ctx->dsc.slice_width = ctx->dt.hactive.min / DSC_SLICE_NUM; > > + ctx->dsc.slice_count = DSC_SLICE_NUM; > > + ctx->dsc.bits_per_component = 8; > > + ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */ > > + ctx->dsc.block_pred_enable = true; > > + ctx->dsc.native_420 = false; > > + ctx->dsc.native_422 = false; > > + ctx->dsc.simple_422 = false; > > + ctx->dsc.vbr_enable = false; > > + ctx->dsc.convert_rgb = 1; > > + ctx->dsc.vbr_enable = 0; > > Aren't those 'false' and '0' defaults? If so, you don't need to write those fields. Ok > > > + > > + drm_dsc_set_rc_buf_thresh(&ctx->dsc); > > + drm_dsc_set_const_params(&ctx->dsc); > > + > > + ctx->dsc.initial_scale_value = drm_dsc_initial_scale_value(&ctx->dsc); > > + ctx->dsc.line_buf_depth = ctx->dsc.bits_per_component + 1; > > + ret = drm_dsc_setup_rc_params(&ctx->dsc, DRM_DSC_1_2_444); > > + if (ret < 0) > > + dev_warn(dev, "drm_dsc_setup_rc_params ret > > + %d\n", ret); > > + > > + ret = drm_dsc_compute_rc_parameters(&ctx->dsc); > > + if (ret) > > + dev_warn(dev, "drm dsc compute rc parameters > > + failed ret %d\n", ret); > > + > > + drm_dsc_pps_payload_pack((struct drm_dsc_picture_parameter_set > *)&ctx->pps_table, > > + &ctx->dsc); > > + dev_dbg(dev, "anx7625 enable dsc\n"); > > + } else { > > + ctx->dsc.dsc_version_major = 0; > > + ctx->dsc.dsc_version_minor = 0; > > + dev_dbg(dev, "anx7625 disable dsc\n"); > > + } > > +} > > + > > static void anx7625_bridge_mode_set(struct drm_bridge *bridge, > > const struct drm_display_mode *old_mode, > > const struct drm_display_mode *mode) > > @@ -2244,6 +2415,8 @@ static void anx7625_bridge_mode_set(struct > drm_bridge *bridge, > > DRM_DEV_DEBUG_DRIVER(dev, "vsync_end(%d),vtotal(%d).\n", > > mode->vsync_end, > > mode->vtotal); > > + > > + anx7625_dsc_enable(ctx, anx7625_dsc_check(ctx)); > > } > > > > static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, @@ > > -2258,10 +2431,6 @@ static bool anx7625_bridge_mode_fixup(struct > > drm_bridge *bridge, > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n"); > > > > - /* No need fixup for external monitor */ > > - if (!ctx->pdata.panel_bridge) > > - return true; > > - > > hsync = mode->hsync_end - mode->hsync_start; > > hfp = mode->hsync_start - mode->hdisplay; > > hbp = mode->htotal - mode->hsync_end; @@ -2272,12 +2441,24 @@ > > static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > > hsync, hfp, hbp, adj->clock); > > DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d), hsync_end(%d), > htot(%d)\n", > > adj->hsync_start, adj->hsync_end, > > adj->htotal); > > - > > adj_hfp = hfp; > > adj_hsync = hsync; > > adj_hbp = hbp; > > adj_hblanking = hblanking; > > > > + if (mode->clock > DSC_PIXEL_CLOCK) { > > + adj_hsync = DSC_HSYNC_LEN; > > + adj_hfp = DSC_HFP_LEN; > > + adj_hbp = DSC_HBP_LEN; > > + vref = (u32)div_u64((u64)adj->clock * 1000 * 1000, > > + adj->htotal * adj->vtotal); > > + goto calculate_timing; > > + } > > + > > + /* No need fixup for external monitor */ > > + if (!ctx->pdata.panel_bridge) > > + return true; > > + > > /* HFP needs to be even */ > > if (hfp & 0x1) { > > adj_hfp += 1; > > @@ -2349,6 +2530,8 @@ static bool anx7625_bridge_mode_fixup(struct > drm_bridge *bridge, > > adj_hfp -= delta_adj; > > } > > > > +calculate_timing: > > + > > DRM_DEV_DEBUG_DRIVER(dev, "after mode fixup\n"); > > DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d), hfp(%d), hbp(%d), > clock(%d)\n", > > adj_hsync, adj_hfp, adj_hbp, adj->clock); > > @@ -2357,6 +2540,10 @@ static bool anx7625_bridge_mode_fixup(struct > drm_bridge *bridge, > > adj->hsync_start = adj->hdisplay + adj_hfp; > > adj->hsync_end = adj->hsync_start + adj_hsync; > > adj->htotal = adj->hsync_end + adj_hbp; > > + if (mode->clock > DSC_PIXEL_CLOCK) > > + adj->clock = (u32)div_u64((u64)vref * adj->htotal * adj->vtotal, > > + 1000 * 1000); > > + > > DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d), hsync_end(%d), > htot(%d)\n", > > adj->hsync_start, adj->hsync_end, > > adj->htotal); > > > > -- > With best wishes > Dmitry
On Mon, Mar 31, 2025 at 05:44:58AM +0000, Xin Ji wrote: > > > 4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits RGB > > > pixel data format, total transport bandwidth need 297M*24(at least > > > 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, > > > 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot > > > receive 4K30 video timing. > > > > > > When display pixel clock exceed 250M, driver will enable DSC feature, > > > and the compression ratio is 3:1, eg: 4K30's pixel clock around 297M, > > > bandwidth 7.2G will be compressed to 7.2G/3 = 2.4G. Then anx7625 can > > > receive 4K30 video timing and do decompress, then package video data > > > and send to sink device through DP link. > > > > > > Anx7625 is bridge IC, sink monitor only receive normal DP signal from > > > anx7625, sink device didn't know DSC information between SOC and > > > anx7625 > > > > > > v2: > > > 1. Add more commit message > > > 2. Remove unused code > > > 3. Add more comment > > > > This part is useless, it adds no information. It is as good as 'changed it'. > OK, I'll remove it > > > > > 4. Remove dsc_en flag > > > > > > Signed-off-by: Xin Ji <xji@analogixsemi.com> > > > --- > > > drivers/gpu/drm/bridge/analogix/anx7625.c | 299 > > > ++++++++++++++++++---- drivers/gpu/drm/bridge/analogix/anx7625.h | > > > 31 +++ > > > 2 files changed, 274 insertions(+), 56 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > index 4be34d5c7a3b..bae76d9a665f 100644 > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > @@ -9,6 +9,7 @@ > > > #include <linux/interrupt.h> > > > #include <linux/iopoll.h> > > > #include <linux/kernel.h> > > > +#include <linux/math64.h> > > > #include <linux/module.h> > > > #include <linux/mutex.h> > > > #include <linux/pm_runtime.h> > > > @@ -22,6 +23,7 @@ > > > > > > #include <drm/display/drm_dp_aux_bus.h> #include > > > <drm/display/drm_dp_helper.h> > > > +#include <drm/display/drm_dsc_helper.h> > > > #include <drm/display/drm_hdcp_helper.h> #include > > > <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ -476,11 > > > +478,159 @@ static int anx7625_set_k_value(struct anx7625_data *ctx) > > > MIPI_DIGITAL_ADJ_1, 0x3D); } > > > > > > +static bool anx7625_dsc_check(struct anx7625_data *ctx) { > > > + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) > > > + return true; > > > > So, now DSC is enabled unconditionally just because the clock is too high. Let's > > see... > Yes > > > > > + > > > + return false; > > > +} > > > + > > > +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, > > > + struct i2c_client *client, > > > + u8 reg_addr, u16 val, > > > + bool dsc_check) { > > > + int ret; > > > + > > > + if (dsc_check && anx7625_dsc_check(ctx)) > > > + val = dsc_div(val); > > > + > > > + ret = anx7625_reg_write(ctx, client, reg_addr, val); > > > + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> 8); > > > + > > > + return ret; > > > +} > > > + > > > +static int anx7625_h_timing_write(struct anx7625_data *ctx, > > > + struct i2c_client *client, > > > + bool rx_h_timing) { > > > + u16 htotal; > > > + int ret; > > > + > > > + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > + /* Htotal */ > > > + ret = anx7625_h_timing_reg_write(ctx, client, > > HORIZONTAL_TOTAL_PIXELS_L, > > > + htotal, rx_h_timing); > > > + /* Hactive */ > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > HORIZONTAL_ACTIVE_PIXELS_L, > > > + ctx->dt.hactive.min, rx_h_timing); > > > + /* HFP */ > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > HORIZONTAL_FRONT_PORCH_L, > > > + ctx->dt.hfront_porch.min, rx_h_timing); > > > + /* HWS */ > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > HORIZONTAL_SYNC_WIDTH_L, > > > + ctx->dt.hsync_len.min, rx_h_timing); > > > + /* HBP */ > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > HORIZONTAL_BACK_PORCH_L, > > > + ctx->dt.hback_porch.min, > > > + rx_h_timing); > > > + > > > + return ret; > > > +} > > > + > > > +static int anx7625_v_timing_write(struct anx7625_data *ctx, > > > + struct i2c_client *client) { > > > + int ret; > > > + > > > + /* Vactive */ > > > + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, > > > + ctx->dt.vactive.min); > > > + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, > > > + ctx->dt.vactive.min >> 8); > > > + /* VFP */ > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, > > > + ctx->dt.vfront_porch.min); > > > + /* VWS */ > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, > > > + ctx->dt.vsync_len.min); > > > + /* VBP */ > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, > > > + ctx->dt.vback_porch.min); > > > + > > > + return ret; > > > +} > > > + > > > +static int anx7625_set_dsc_params(struct anx7625_data *ctx) { > > > + int ret, i; > > > + u16 htotal, vtotal; > > > + > > > + if (!anx7625_dsc_check(ctx)) > > > + return 0; > > > + > > > + /* Video Horizontal timing */ > > > + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, false); > > > + > > > + /* Video Vertical timing */ > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); > > > + > > > + /* Vtotal */ > > > + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + > > > + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, > > > + vtotal); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, > > > + vtotal >> 8); > > > + /* Htotal */ > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_L_7E, > > > + htotal); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_H_7E, > > > + htotal >> 8); > > > + /* Hactive */ > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); > > > + /* HFP */ > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + HORIZON_FRONT_PORCH_L_7E, > > > + ctx->dt.hfront_porch.min); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + HORIZON_FRONT_PORCH_H_7E, > > > + ctx->dt.hfront_porch.min >> 8); > > > + /* HWS */ > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + HORIZON_SYNC_WIDTH_L_7E, > > > + ctx->dt.hsync_len.min); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + HORIZON_SYNC_WIDTH_H_7E, > > > + ctx->dt.hsync_len.min >> 8); > > > + /* HBP */ > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + HORIZON_BACK_PORCH_L_7E, > > > + ctx->dt.hback_porch.min); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > + HORIZON_BACK_PORCH_H_7E, > > > + ctx->dt.hback_porch.min >> 8); > > > + > > > + /* Config DSC decoder internal blank timing for decoder to start */ > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > + H_BLANK_L, > > > + dsc_div(htotal - ctx->dt.hactive.min)); > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > + H_BLANK_H, > > > + dsc_div(htotal - ctx->dt.hactive.min) > > > + >> 8); > > > + > > > + /* Compress ratio RATIO bit[7:6] */ > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, > > > + (5 - DSC_COMPRESS_RATIO) << 6); > > > + /*PPS table*/ > > > + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) > > > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, > > > + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, > > > + &ctx->pps_table[i]); > > > + > > > + return ret; > > > +} > > > + > > > static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) > > > { > > > struct device *dev = ctx->dev; > > > unsigned long m, n; > > > - u16 htotal; > > > int ret; > > > u8 post_divider = 0; > > > > > > @@ -506,48 +656,12 @@ static int anx7625_dsi_video_timing_config(struct > > anx7625_data *ctx) > > > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, > > > MIPI_LANE_CTRL_0, ctx->pdata.mipi_lanes > > > - 1); > > > > > > - /* Htotal */ > > > - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); > > > - /* Hactive */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); > > > - /* HFP */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_FRONT_PORCH_H, > > > - ctx->dt.hfront_porch.min >> 8); > > > - /* HWS */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); > > > - /* HBP */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> 8); > > > - /* Vactive */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, > > > - ctx->dt.vactive.min); > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, > > > - ctx->dt.vactive.min >> 8); > > > - /* VFP */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); > > > - /* VWS */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); > > > - /* VBP */ > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); > > > + /* Video Horizontal timing */ > > > + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, true); > > > + > > > + /* Video Vertical timing */ > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); > > > + > > > > Please split this part into two commits: one refactoring timing programming into > > two functions and another one introducing DSC support. > > It is hard to review timing programming otherwise. > > OK > > > > > > /* M value */ > > > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); @@ > > > -663,9 +777,15 @@ static int anx7625_dsi_config(struct anx7625_data > > > *ctx) > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); > > > > > > - /* DSC disable */ > > > - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > - R_DSC_CTRL_0, ~DSC_EN); > > > + ret = anx7625_set_dsc_params(ctx); > > > + if (anx7625_dsc_check(ctx)) > > > + /* DSC enable */ > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > > > + R_DSC_CTRL_0, DSC_EN); > > > + else > > > + /* DSC disable */ > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > + R_DSC_CTRL_0, ~DSC_EN); > > > > > > ret |= anx7625_api_dsi_config(ctx); > > > > > > @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct > > anx7625_data *ctx) > > > MIPI_DSI_MODE_VIDEO_HSE | > > > MIPI_DSI_HS_PKT_END_ALIGNED; > > > > > > + dsi->dsc = &ctx->dsc; > > > ctx->dsi = dsi; > > > > > > return 0; > > > @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct drm_bridge > > *bridge, > > > struct device *dev = ctx->dev; > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); > > > + if (mode->clock > SUPPORT_PIXEL_CLOCK) > > > + return MODE_CLOCK_HIGH; > > > + > > > + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) > > > + return MODE_CLOCK_LOW; > > > > > > - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ > > > - if (mode->clock > SUPPORT_PIXEL_CLOCK) { > > > - DRM_DEV_DEBUG_DRIVER(dev, > > > - "drm mode invalid, pixelclock too high.\n"); > > > > Any reason for dropping debug message? > > I'll keep this message. > > > > > > + /* > > > + * If hdisplay cannot be divided by DSC compress ratio, then display > > > + * will have overlap/shift issue > > > + */ > > > + if (mode->clock > DSC_PIXEL_CLOCK && > > > + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) > > > > > > ... and there still no check that the DSI host supports generating DSC data. Nor > > there is an atomic_check() that will check if the mode can be enabled. > > I cannot know whether the DSI host supports DSC or not. Anx7625 driver force > enable DSC feature if pixel clock higher than DSC_PIXEL_CLOCK. This is called upstream. Please work on necessary API changes rather than claiming that it is not possible. Enabling DSC support when DSC is not handled by the DSI host is not acceptable. Few semi-random ideas: - Add DSC checking callbacks to struct mipi_dsi_host_ops. Assume that DSC support is not available if callback is not present. The callback should accept struct drm_dsc_config and populate const and RC params. - Add DSC-related flags to struct mipi_dsi_host, describing DSC feature availability. Make anx7625 check those flags. > > > > > > return MODE_CLOCK_HIGH; > > > - } > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode valid.\n"); > > > > > > return MODE_OK; > > > } > > > > > > +static void anx7625_dsc_enable(struct anx7625_data *ctx, bool en) { > > > + int ret; > > > + struct device *dev = ctx->dev; > > > + > > > + if (en) { > > > + ctx->dsc.dsc_version_major = 1; > > > + ctx->dsc.dsc_version_minor = 1; > > > + ctx->dsc.slice_height = 8; > > > + ctx->dsc.slice_width = ctx->dt.hactive.min / DSC_SLICE_NUM; > > > + ctx->dsc.slice_count = DSC_SLICE_NUM; > > > + ctx->dsc.bits_per_component = 8; > > > + ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */ > > > + ctx->dsc.block_pred_enable = true; > > > + ctx->dsc.native_420 = false; > > > + ctx->dsc.native_422 = false; > > > + ctx->dsc.simple_422 = false; > > > + ctx->dsc.vbr_enable = false; > > > + ctx->dsc.convert_rgb = 1; > > > + ctx->dsc.vbr_enable = 0; > > > > Aren't those 'false' and '0' defaults? If so, you don't need to write those fields. > Ok > > > > > > + > > > + drm_dsc_set_rc_buf_thresh(&ctx->dsc); > > > + drm_dsc_set_const_params(&ctx->dsc); > > > + > > > + ctx->dsc.initial_scale_value = drm_dsc_initial_scale_value(&ctx->dsc); > > > + ctx->dsc.line_buf_depth = ctx->dsc.bits_per_component + 1; > > > + ret = drm_dsc_setup_rc_params(&ctx->dsc, DRM_DSC_1_2_444); > > > + if (ret < 0) > > > + dev_warn(dev, "drm_dsc_setup_rc_params ret > > > + %d\n", ret); > > > + > > > + ret = drm_dsc_compute_rc_parameters(&ctx->dsc); BTW: these calls belong to the DSI host driver. See msm/dsi/dsi_host.c and DSC-enabled panel drivers. > > > + if (ret) > > > + dev_warn(dev, "drm dsc compute rc parameters > > > + failed ret %d\n", ret); I think this should become an error rather than a warning. > > > + > > > + drm_dsc_pps_payload_pack((struct drm_dsc_picture_parameter_set > > *)&ctx->pps_table, > > > + &ctx->dsc); > > > + dev_dbg(dev, "anx7625 enable dsc\n"); > > > + } else { > > > + ctx->dsc.dsc_version_major = 0; > > > + ctx->dsc.dsc_version_minor = 0; > > > + dev_dbg(dev, "anx7625 disable dsc\n"); > > > + } > > > +} > > > +
On Mon, Mar 31, 2025 at 10:28:00AM +0300, Dmitry Baryshkov wrote: > On Mon, Mar 31, 2025 at 05:44:58AM +0000, Xin Ji wrote: > > > > 4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits RGB > > > > pixel data format, total transport bandwidth need 297M*24(at least > > > > 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, > > > > 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot > > > > receive 4K30 video timing. > > > > > > > > When display pixel clock exceed 250M, driver will enable DSC feature, > > > > and the compression ratio is 3:1, eg: 4K30's pixel clock around 297M, > > > > bandwidth 7.2G will be compressed to 7.2G/3 = 2.4G. Then anx7625 can > > > > receive 4K30 video timing and do decompress, then package video data > > > > and send to sink device through DP link. > > > > > > > > Anx7625 is bridge IC, sink monitor only receive normal DP signal from > > > > anx7625, sink device didn't know DSC information between SOC and > > > > anx7625 > > > > > > > > v2: > > > > 1. Add more commit message > > > > 2. Remove unused code > > > > 3. Add more comment > > > > > > This part is useless, it adds no information. It is as good as 'changed it'. > > OK, I'll remove it > > > > > > > 4. Remove dsc_en flag > > > > > > > > Signed-off-by: Xin Ji <xji@analogixsemi.com> > > > > --- > > > > drivers/gpu/drm/bridge/analogix/anx7625.c | 299 > > > > ++++++++++++++++++---- drivers/gpu/drm/bridge/analogix/anx7625.h | > > > > 31 +++ > > > > 2 files changed, 274 insertions(+), 56 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > index 4be34d5c7a3b..bae76d9a665f 100644 > > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > @@ -9,6 +9,7 @@ > > > > #include <linux/interrupt.h> > > > > #include <linux/iopoll.h> > > > > #include <linux/kernel.h> > > > > +#include <linux/math64.h> > > > > #include <linux/module.h> > > > > #include <linux/mutex.h> > > > > #include <linux/pm_runtime.h> > > > > @@ -22,6 +23,7 @@ > > > > > > > > #include <drm/display/drm_dp_aux_bus.h> #include > > > > <drm/display/drm_dp_helper.h> > > > > +#include <drm/display/drm_dsc_helper.h> > > > > #include <drm/display/drm_hdcp_helper.h> #include > > > > <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ -476,11 > > > > +478,159 @@ static int anx7625_set_k_value(struct anx7625_data *ctx) > > > > MIPI_DIGITAL_ADJ_1, 0x3D); } > > > > > > > > +static bool anx7625_dsc_check(struct anx7625_data *ctx) { > > > > + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) > > > > + return true; > > > > > > So, now DSC is enabled unconditionally just because the clock is too high. Let's > > > see... > > Yes > > > > > > > + > > > > + return false; > > > > +} > > > > + > > > > +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, > > > > + struct i2c_client *client, > > > > + u8 reg_addr, u16 val, > > > > + bool dsc_check) { > > > > + int ret; > > > > + > > > > + if (dsc_check && anx7625_dsc_check(ctx)) > > > > + val = dsc_div(val); > > > > + > > > > + ret = anx7625_reg_write(ctx, client, reg_addr, val); > > > > + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> 8); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int anx7625_h_timing_write(struct anx7625_data *ctx, > > > > + struct i2c_client *client, > > > > + bool rx_h_timing) { > > > > + u16 htotal; > > > > + int ret; > > > > + > > > > + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > > + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > > + /* Htotal */ > > > > + ret = anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_TOTAL_PIXELS_L, > > > > + htotal, rx_h_timing); > > > > + /* Hactive */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_ACTIVE_PIXELS_L, > > > > + ctx->dt.hactive.min, rx_h_timing); > > > > + /* HFP */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_FRONT_PORCH_L, > > > > + ctx->dt.hfront_porch.min, rx_h_timing); > > > > + /* HWS */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_SYNC_WIDTH_L, > > > > + ctx->dt.hsync_len.min, rx_h_timing); > > > > + /* HBP */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_BACK_PORCH_L, > > > > + ctx->dt.hback_porch.min, > > > > + rx_h_timing); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int anx7625_v_timing_write(struct anx7625_data *ctx, > > > > + struct i2c_client *client) { > > > > + int ret; > > > > + > > > > + /* Vactive */ > > > > + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, > > > > + ctx->dt.vactive.min); > > > > + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, > > > > + ctx->dt.vactive.min >> 8); > > > > + /* VFP */ > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, > > > > + ctx->dt.vfront_porch.min); > > > > + /* VWS */ > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, > > > > + ctx->dt.vsync_len.min); > > > > + /* VBP */ > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, > > > > + ctx->dt.vback_porch.min); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int anx7625_set_dsc_params(struct anx7625_data *ctx) { > > > > + int ret, i; > > > > + u16 htotal, vtotal; > > > > + > > > > + if (!anx7625_dsc_check(ctx)) > > > > + return 0; > > > > + > > > > + /* Video Horizontal timing */ > > > > + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, false); > > > > + > > > > + /* Video Vertical timing */ > > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); > > > > + > > > > + /* Vtotal */ > > > > + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + > > > > + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, > > > > + vtotal); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, > > > > + vtotal >> 8); > > > > + /* Htotal */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_L_7E, > > > > + htotal); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_H_7E, > > > > + htotal >> 8); > > > > + /* Hactive */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); > > > > + /* HFP */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_FRONT_PORCH_L_7E, > > > > + ctx->dt.hfront_porch.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_FRONT_PORCH_H_7E, > > > > + ctx->dt.hfront_porch.min >> 8); > > > > + /* HWS */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_SYNC_WIDTH_L_7E, > > > > + ctx->dt.hsync_len.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_SYNC_WIDTH_H_7E, > > > > + ctx->dt.hsync_len.min >> 8); > > > > + /* HBP */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_BACK_PORCH_L_7E, > > > > + ctx->dt.hback_porch.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_BACK_PORCH_H_7E, > > > > + ctx->dt.hback_porch.min >> 8); > > > > + > > > > + /* Config DSC decoder internal blank timing for decoder to start */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > + H_BLANK_L, > > > > + dsc_div(htotal - ctx->dt.hactive.min)); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > + H_BLANK_H, > > > > + dsc_div(htotal - ctx->dt.hactive.min) > > > > + >> 8); > > > > + > > > > + /* Compress ratio RATIO bit[7:6] */ > > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); > > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, > > > > + (5 - DSC_COMPRESS_RATIO) << 6); > > > > + /*PPS table*/ > > > > + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) > > > > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, > > > > + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, > > > > + &ctx->pps_table[i]); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) > > > > { > > > > struct device *dev = ctx->dev; > > > > unsigned long m, n; > > > > - u16 htotal; > > > > int ret; > > > > u8 post_divider = 0; > > > > > > > > @@ -506,48 +656,12 @@ static int anx7625_dsi_video_timing_config(struct > > > anx7625_data *ctx) > > > > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, > > > > MIPI_LANE_CTRL_0, ctx->pdata.mipi_lanes > > > > - 1); > > > > > > > > - /* Htotal */ > > > > - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > > - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); > > > > - /* Hactive */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); > > > > - /* HFP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_FRONT_PORCH_H, > > > > - ctx->dt.hfront_porch.min >> 8); > > > > - /* HWS */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); > > > > - /* HBP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> 8); > > > > - /* Vactive */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, > > > > - ctx->dt.vactive.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, > > > > - ctx->dt.vactive.min >> 8); > > > > - /* VFP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); > > > > - /* VWS */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); > > > > - /* VBP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); > > > > + /* Video Horizontal timing */ > > > > + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, true); > > > > + > > > > + /* Video Vertical timing */ > > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); > > > > + > > > > > > Please split this part into two commits: one refactoring timing programming into > > > two functions and another one introducing DSC support. > > > It is hard to review timing programming otherwise. > > > > OK > > > > > > > > > /* M value */ > > > > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); @@ > > > > -663,9 +777,15 @@ static int anx7625_dsi_config(struct anx7625_data > > > > *ctx) > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); > > > > > > > > - /* DSC disable */ > > > > - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > > - R_DSC_CTRL_0, ~DSC_EN); > > > > + ret = anx7625_set_dsc_params(ctx); > > > > + if (anx7625_dsc_check(ctx)) > > > > + /* DSC enable */ > > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > > > > + R_DSC_CTRL_0, DSC_EN); > > > > + else > > > > + /* DSC disable */ > > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > > + R_DSC_CTRL_0, ~DSC_EN); > > > > > > > > ret |= anx7625_api_dsi_config(ctx); > > > > > > > > @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct > > > anx7625_data *ctx) > > > > MIPI_DSI_MODE_VIDEO_HSE | > > > > MIPI_DSI_HS_PKT_END_ALIGNED; > > > > > > > > + dsi->dsc = &ctx->dsc; > > > > ctx->dsi = dsi; > > > > > > > > return 0; > > > > @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct drm_bridge > > > *bridge, > > > > struct device *dev = ctx->dev; > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); > > > > + if (mode->clock > SUPPORT_PIXEL_CLOCK) > > > > + return MODE_CLOCK_HIGH; > > > > + > > > > + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) > > > > + return MODE_CLOCK_LOW; > > > > > > > > - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ > > > > - if (mode->clock > SUPPORT_PIXEL_CLOCK) { > > > > - DRM_DEV_DEBUG_DRIVER(dev, > > > > - "drm mode invalid, pixelclock too high.\n"); > > > > > > Any reason for dropping debug message? > > > > I'll keep this message. > > > > > > > > > + /* > > > > + * If hdisplay cannot be divided by DSC compress ratio, then display > > > > + * will have overlap/shift issue > > > > + */ > > > > + if (mode->clock > DSC_PIXEL_CLOCK && > > > > + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) > > > > > > > > > ... and there still no check that the DSI host supports generating DSC data. Nor > > > there is an atomic_check() that will check if the mode can be enabled. > > > > I cannot know whether the DSI host supports DSC or not. Anx7625 driver force > > enable DSC feature if pixel clock higher than DSC_PIXEL_CLOCK. > > This is called upstream. Please work on necessary API changes rather > than claiming that it is not possible. Enabling DSC support when DSC is > not handled by the DSI host is not acceptable. > > Few semi-random ideas: > - Add DSC checking callbacks to struct mipi_dsi_host_ops. Assume that > DSC support is not available if callback is not present. The callback > should accept struct drm_dsc_config and populate const and RC params. > > - Add DSC-related flags to struct mipi_dsi_host, describing DSC feature > availability. Make anx7625 check those flags. ... - Extend input_/output_bus_format checks to also cover DSC.
> > > > 4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits > > > > RGB pixel data format, total transport bandwidth need 297M*24(at > > > > least > > > > 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, > > > > 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot > > > > receive 4K30 video timing. > > > > > > > > When display pixel clock exceed 250M, driver will enable DSC > > > > feature, and the compression ratio is 3:1, eg: 4K30's pixel clock > > > > around 297M, bandwidth 7.2G will be compressed to 7.2G/3 = 2.4G. > > > > Then anx7625 can receive 4K30 video timing and do decompress, then > > > > package video data and send to sink device through DP link. > > > > > > > > Anx7625 is bridge IC, sink monitor only receive normal DP signal > > > > from anx7625, sink device didn't know DSC information between SOC > > > > and > > > > anx7625 > > > > > > > > v2: > > > > 1. Add more commit message > > > > 2. Remove unused code > > > > 3. Add more comment > > > > > > This part is useless, it adds no information. It is as good as 'changed it'. > > OK, I'll remove it > > > > > > > 4. Remove dsc_en flag > > > > > > > > Signed-off-by: Xin Ji <xji@analogixsemi.com> > > > > --- > > > > drivers/gpu/drm/bridge/analogix/anx7625.c | 299 > > > > ++++++++++++++++++---- drivers/gpu/drm/bridge/analogix/anx7625.h > > > > ++++++++++++++++++| > > > > 31 +++ > > > > 2 files changed, 274 insertions(+), 56 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > index 4be34d5c7a3b..bae76d9a665f 100644 > > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > @@ -9,6 +9,7 @@ > > > > #include <linux/interrupt.h> > > > > #include <linux/iopoll.h> > > > > #include <linux/kernel.h> > > > > +#include <linux/math64.h> > > > > #include <linux/module.h> > > > > #include <linux/mutex.h> > > > > #include <linux/pm_runtime.h> > > > > @@ -22,6 +23,7 @@ > > > > > > > > #include <drm/display/drm_dp_aux_bus.h> #include > > > > <drm/display/drm_dp_helper.h> > > > > +#include <drm/display/drm_dsc_helper.h> > > > > #include <drm/display/drm_hdcp_helper.h> #include > > > > <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ -476,11 > > > > +478,159 @@ static int anx7625_set_k_value(struct anx7625_data > > > > +*ctx) > > > > MIPI_DIGITAL_ADJ_1, 0x3D); } > > > > > > > > +static bool anx7625_dsc_check(struct anx7625_data *ctx) { > > > > + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) > > > > + return true; > > > > > > So, now DSC is enabled unconditionally just because the clock is too > > > high. Let's see... > > Yes > > > > > > > + > > > > + return false; > > > > +} > > > > + > > > > +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, > > > > + struct i2c_client *client, > > > > + u8 reg_addr, u16 val, > > > > + bool dsc_check) { > > > > + int ret; > > > > + > > > > + if (dsc_check && anx7625_dsc_check(ctx)) > > > > + val = dsc_div(val); > > > > + > > > > + ret = anx7625_reg_write(ctx, client, reg_addr, val); > > > > + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> > > > > + 8); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int anx7625_h_timing_write(struct anx7625_data *ctx, > > > > + struct i2c_client *client, > > > > + bool rx_h_timing) { > > > > + u16 htotal; > > > > + int ret; > > > > + > > > > + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > > + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > > + /* Htotal */ > > > > + ret = anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_TOTAL_PIXELS_L, > > > > + htotal, rx_h_timing); > > > > + /* Hactive */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_ACTIVE_PIXELS_L, > > > > + ctx->dt.hactive.min, rx_h_timing); > > > > + /* HFP */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_FRONT_PORCH_L, > > > > + ctx->dt.hfront_porch.min, rx_h_timing); > > > > + /* HWS */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_SYNC_WIDTH_L, > > > > + ctx->dt.hsync_len.min, rx_h_timing); > > > > + /* HBP */ > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > HORIZONTAL_BACK_PORCH_L, > > > > + ctx->dt.hback_porch.min, > > > > + rx_h_timing); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int anx7625_v_timing_write(struct anx7625_data *ctx, > > > > + struct i2c_client *client) { > > > > + int ret; > > > > + > > > > + /* Vactive */ > > > > + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, > > > > + ctx->dt.vactive.min); > > > > + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, > > > > + ctx->dt.vactive.min >> 8); > > > > + /* VFP */ > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, > > > > + ctx->dt.vfront_porch.min); > > > > + /* VWS */ > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, > > > > + ctx->dt.vsync_len.min); > > > > + /* VBP */ > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, > > > > + ctx->dt.vback_porch.min); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int anx7625_set_dsc_params(struct anx7625_data *ctx) { > > > > + int ret, i; > > > > + u16 htotal, vtotal; > > > > + > > > > + if (!anx7625_dsc_check(ctx)) > > > > + return 0; > > > > + > > > > + /* Video Horizontal timing */ > > > > + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, > > > > + false); > > > > + > > > > + /* Video Vertical timing */ > > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); > > > > + > > > > + /* Vtotal */ > > > > + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + > > > > + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, > > > > + vtotal); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, > > > > + vtotal >> 8); > > > > + /* Htotal */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > TOTAL_PIXEL_L_7E, > > > > + htotal); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > TOTAL_PIXEL_H_7E, > > > > + htotal >> 8); > > > > + /* Hactive */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); > > > > + /* HFP */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_FRONT_PORCH_L_7E, > > > > + ctx->dt.hfront_porch.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_FRONT_PORCH_H_7E, > > > > + ctx->dt.hfront_porch.min >> 8); > > > > + /* HWS */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_SYNC_WIDTH_L_7E, > > > > + ctx->dt.hsync_len.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_SYNC_WIDTH_H_7E, > > > > + ctx->dt.hsync_len.min >> 8); > > > > + /* HBP */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_BACK_PORCH_L_7E, > > > > + ctx->dt.hback_porch.min); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > + HORIZON_BACK_PORCH_H_7E, > > > > + ctx->dt.hback_porch.min >> 8); > > > > + > > > > + /* Config DSC decoder internal blank timing for decoder to start */ > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > + H_BLANK_L, > > > > + dsc_div(htotal - ctx->dt.hactive.min)); > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > + H_BLANK_H, > > > > + dsc_div(htotal - > > > > + ctx->dt.hactive.min) > > > > + >> 8); > > > > + > > > > + /* Compress ratio RATIO bit[7:6] */ > > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); > > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, > > > > + (5 - DSC_COMPRESS_RATIO) << 6); > > > > + /*PPS table*/ > > > > + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) > > > > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, > > > > + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, > > > > + &ctx->pps_table[i]); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > static int anx7625_dsi_video_timing_config(struct anx7625_data > > > > *ctx) { > > > > struct device *dev = ctx->dev; > > > > unsigned long m, n; > > > > - u16 htotal; > > > > int ret; > > > > u8 post_divider = 0; > > > > > > > > @@ -506,48 +656,12 @@ static int > > > > anx7625_dsi_video_timing_config(struct > > > anx7625_data *ctx) > > > > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, > > > > MIPI_LANE_CTRL_0, > > > > ctx->pdata.mipi_lanes > > > > - 1); > > > > > > > > - /* Htotal */ > > > > - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > > - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); > > > > - /* Hactive */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); > > > > - /* HFP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_FRONT_PORCH_H, > > > > - ctx->dt.hfront_porch.min >> 8); > > > > - /* HWS */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); > > > > - /* HBP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> 8); > > > > - /* Vactive */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, > > > > - ctx->dt.vactive.min); > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, > > > > - ctx->dt.vactive.min >> 8); > > > > - /* VFP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); > > > > - /* VWS */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); > > > > - /* VBP */ > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); > > > > + /* Video Horizontal timing */ > > > > + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, > > > > + true); > > > > + > > > > + /* Video Vertical timing */ > > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); > > > > + > > > > > > Please split this part into two commits: one refactoring timing > > > programming into two functions and another one introducing DSC support. > > > It is hard to review timing programming otherwise. > > > > OK > > > > > > > > > /* M value */ > > > > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); @@ > > > > -663,9 +777,15 @@ static int anx7625_dsi_config(struct > > > > anx7625_data > > > > *ctx) > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); > > > > > > > > - /* DSC disable */ > > > > - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > > - R_DSC_CTRL_0, ~DSC_EN); > > > > + ret = anx7625_set_dsc_params(ctx); > > > > + if (anx7625_dsc_check(ctx)) > > > > + /* DSC enable */ > > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > > > > + R_DSC_CTRL_0, DSC_EN); > > > > + else > > > > + /* DSC disable */ > > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > > + R_DSC_CTRL_0, ~DSC_EN); > > > > > > > > ret |= anx7625_api_dsi_config(ctx); > > > > > > > > @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct > > > anx7625_data *ctx) > > > > MIPI_DSI_MODE_VIDEO_HSE | > > > > MIPI_DSI_HS_PKT_END_ALIGNED; > > > > > > > > + dsi->dsc = &ctx->dsc; > > > > ctx->dsi = dsi; > > > > > > > > return 0; > > > > @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct > > > > drm_bridge > > > *bridge, > > > > struct device *dev = ctx->dev; > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); > > > > + if (mode->clock > SUPPORT_PIXEL_CLOCK) > > > > + return MODE_CLOCK_HIGH; > > > > + > > > > + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) > > > > + return MODE_CLOCK_LOW; > > > > > > > > - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ > > > > - if (mode->clock > SUPPORT_PIXEL_CLOCK) { > > > > - DRM_DEV_DEBUG_DRIVER(dev, > > > > - "drm mode invalid, pixelclock too high.\n"); > > > > > > Any reason for dropping debug message? > > > > I'll keep this message. > > > > > > > > > + /* > > > > + * If hdisplay cannot be divided by DSC compress ratio, then display > > > > + * will have overlap/shift issue > > > > + */ > > > > + if (mode->clock > DSC_PIXEL_CLOCK && > > > > + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) > > > > > > > > > ... and there still no check that the DSI host supports generating > > > DSC data. Nor there is an atomic_check() that will check if the mode can be > enabled. > > > > I cannot know whether the DSI host supports DSC or not. Anx7625 driver > > force enable DSC feature if pixel clock higher than DSC_PIXEL_CLOCK. > > This is called upstream. Please work on necessary API changes rather than > claiming that it is not possible. Enabling DSC support when DSC is not handled by > the DSI host is not acceptable. > > Few semi-random ideas: > - Add DSC checking callbacks to struct mipi_dsi_host_ops. Assume that > DSC support is not available if callback is not present. The callback > should accept struct drm_dsc_config and populate const and RC params. > > - Add DSC-related flags to struct mipi_dsi_host, describing DSC feature > availability. Make anx7625 check those flags. OK, I'll discuss with Chromium team and MTK DSI host team. > > > > > > > > > > return MODE_CLOCK_HIGH; > > > > - } > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode valid.\n"); > > > > > > > > return MODE_OK; > > > > } > > > > > > > > +static void anx7625_dsc_enable(struct anx7625_data *ctx, bool en) { > > > > + int ret; > > > > + struct device *dev = ctx->dev; > > > > + > > > > + if (en) { > > > > + ctx->dsc.dsc_version_major = 1; > > > > + ctx->dsc.dsc_version_minor = 1; > > > > + ctx->dsc.slice_height = 8; > > > > + ctx->dsc.slice_width = ctx->dt.hactive.min / DSC_SLICE_NUM; > > > > + ctx->dsc.slice_count = DSC_SLICE_NUM; > > > > + ctx->dsc.bits_per_component = 8; > > > > + ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */ > > > > + ctx->dsc.block_pred_enable = true; > > > > + ctx->dsc.native_420 = false; > > > > + ctx->dsc.native_422 = false; > > > > + ctx->dsc.simple_422 = false; > > > > + ctx->dsc.vbr_enable = false; > > > > + ctx->dsc.convert_rgb = 1; > > > > + ctx->dsc.vbr_enable = 0; > > > > > > Aren't those 'false' and '0' defaults? If so, you don't need to write those > fields. > > Ok > > > > > > > > > + > > > > + drm_dsc_set_rc_buf_thresh(&ctx->dsc); > > > > + drm_dsc_set_const_params(&ctx->dsc); > > > > + > > > > + ctx->dsc.initial_scale_value = drm_dsc_initial_scale_value(&ctx- > >dsc); > > > > + ctx->dsc.line_buf_depth = ctx->dsc.bits_per_component + 1; > > > > + ret = drm_dsc_setup_rc_params(&ctx->dsc, DRM_DSC_1_2_444); > > > > + if (ret < 0) > > > > + dev_warn(dev, "drm_dsc_setup_rc_params ret > > > > + %d\n", ret); > > > > + > > > > + ret = drm_dsc_compute_rc_parameters(&ctx->dsc); > > BTW: these calls belong to the DSI host driver. See msm/dsi/dsi_host.c and DSC- > enabled panel drivers. I'm not sure, this interface is suggested by MIPI host. Seems MTK MIPI host driver didn't call this interface. > > > > > + if (ret) > > > > + dev_warn(dev, "drm dsc compute rc parameters > > > > + failed ret %d\n", ret); > > I think this should become an error rather than a warning. OK > > > > > + > > > > + drm_dsc_pps_payload_pack((struct > > > > + drm_dsc_picture_parameter_set > > > *)&ctx->pps_table, > > > > + &ctx->dsc); > > > > + dev_dbg(dev, "anx7625 enable dsc\n"); > > > > + } else { > > > > + ctx->dsc.dsc_version_major = 0; > > > > + ctx->dsc.dsc_version_minor = 0; > > > > + dev_dbg(dev, "anx7625 disable dsc\n"); > > > > + } > > > > +} > > > > + > > -- > With best wishes > Dmitry
> > > > > 4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits > > > > > RGB pixel data format, total transport bandwidth need 297M*24(at > > > > > least > > > > > 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, > > > > > 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot > > > > > receive 4K30 video timing. > > > > > > > > > > When display pixel clock exceed 250M, driver will enable DSC > > > > > feature, and the compression ratio is 3:1, eg: 4K30's pixel > > > > > clock around 297M, bandwidth 7.2G will be compressed to 7.2G/3 = > > > > > 2.4G. Then anx7625 can receive 4K30 video timing and do > > > > > decompress, then package video data and send to sink device through DP > link. > > > > > > > > > > Anx7625 is bridge IC, sink monitor only receive normal DP signal > > > > > from anx7625, sink device didn't know DSC information between > > > > > SOC and > > > > > anx7625 > > > > > > > > > > v2: > > > > > 1. Add more commit message > > > > > 2. Remove unused code > > > > > 3. Add more comment > > > > > > > > This part is useless, it adds no information. It is as good as 'changed it'. > > > OK, I'll remove it > > > > > > > > > 4. Remove dsc_en flag > > > > > > > > > > Signed-off-by: Xin Ji <xji@analogixsemi.com> > > > > > --- > > > > > drivers/gpu/drm/bridge/analogix/anx7625.c | 299 > > > > > ++++++++++++++++++---- > > > > > ++++++++++++++++++drivers/gpu/drm/bridge/analogix/anx7625.h | > > > > > 31 +++ > > > > > 2 files changed, 274 insertions(+), 56 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > > index 4be34d5c7a3b..bae76d9a665f 100644 > > > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > > > @@ -9,6 +9,7 @@ > > > > > #include <linux/interrupt.h> > > > > > #include <linux/iopoll.h> > > > > > #include <linux/kernel.h> > > > > > +#include <linux/math64.h> > > > > > #include <linux/module.h> > > > > > #include <linux/mutex.h> > > > > > #include <linux/pm_runtime.h> > > > > > @@ -22,6 +23,7 @@ > > > > > > > > > > #include <drm/display/drm_dp_aux_bus.h> #include > > > > > <drm/display/drm_dp_helper.h> > > > > > +#include <drm/display/drm_dsc_helper.h> > > > > > #include <drm/display/drm_hdcp_helper.h> #include > > > > > <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ > > > > > -476,11 > > > > > +478,159 @@ static int anx7625_set_k_value(struct anx7625_data > > > > > +*ctx) > > > > > MIPI_DIGITAL_ADJ_1, 0x3D); } > > > > > > > > > > +static bool anx7625_dsc_check(struct anx7625_data *ctx) { > > > > > + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) > > > > > + return true; > > > > > > > > So, now DSC is enabled unconditionally just because the clock is > > > > too high. Let's see... > > > Yes > > > > > > > > > + > > > > > + return false; > > > > > +} > > > > > + > > > > > +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, > > > > > + struct i2c_client *client, > > > > > + u8 reg_addr, u16 val, > > > > > + bool dsc_check) { > > > > > + int ret; > > > > > + > > > > > + if (dsc_check && anx7625_dsc_check(ctx)) > > > > > + val = dsc_div(val); > > > > > + > > > > > + ret = anx7625_reg_write(ctx, client, reg_addr, val); > > > > > + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> > > > > > + 8); > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int anx7625_h_timing_write(struct anx7625_data *ctx, > > > > > + struct i2c_client *client, > > > > > + bool rx_h_timing) { > > > > > + u16 htotal; > > > > > + int ret; > > > > > + > > > > > + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > > > + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > > > + /* Htotal */ > > > > > + ret = anx7625_h_timing_reg_write(ctx, client, > > > > HORIZONTAL_TOTAL_PIXELS_L, > > > > > + htotal, rx_h_timing); > > > > > + /* Hactive */ > > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > > HORIZONTAL_ACTIVE_PIXELS_L, > > > > > + ctx->dt.hactive.min, rx_h_timing); > > > > > + /* HFP */ > > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > > HORIZONTAL_FRONT_PORCH_L, > > > > > + ctx->dt.hfront_porch.min, rx_h_timing); > > > > > + /* HWS */ > > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > > HORIZONTAL_SYNC_WIDTH_L, > > > > > + ctx->dt.hsync_len.min, rx_h_timing); > > > > > + /* HBP */ > > > > > + ret |= anx7625_h_timing_reg_write(ctx, client, > > > > HORIZONTAL_BACK_PORCH_L, > > > > > + ctx->dt.hback_porch.min, > > > > > + rx_h_timing); > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int anx7625_v_timing_write(struct anx7625_data *ctx, > > > > > + struct i2c_client *client) { > > > > > + int ret; > > > > > + > > > > > + /* Vactive */ > > > > > + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, > > > > > + ctx->dt.vactive.min); > > > > > + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, > > > > > + ctx->dt.vactive.min >> 8); > > > > > + /* VFP */ > > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, > > > > > + ctx->dt.vfront_porch.min); > > > > > + /* VWS */ > > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, > > > > > + ctx->dt.vsync_len.min); > > > > > + /* VBP */ > > > > > + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, > > > > > + ctx->dt.vback_porch.min); > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int anx7625_set_dsc_params(struct anx7625_data *ctx) { > > > > > + int ret, i; > > > > > + u16 htotal, vtotal; > > > > > + > > > > > + if (!anx7625_dsc_check(ctx)) > > > > > + return 0; > > > > > + > > > > > + /* Video Horizontal timing */ > > > > > + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, > > > > > + false); > > > > > + > > > > > + /* Video Vertical timing */ > > > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); > > > > > + > > > > > + /* Vtotal */ > > > > > + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + > > > > > + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, > > > > > + vtotal); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, > > > > > + vtotal >> 8); > > > > > + /* Htotal */ > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > TOTAL_PIXEL_L_7E, > > > > > + htotal); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > TOTAL_PIXEL_H_7E, > > > > > + htotal >> 8); > > > > > + /* Hactive */ > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); > > > > > + /* HFP */ > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + HORIZON_FRONT_PORCH_L_7E, > > > > > + ctx->dt.hfront_porch.min); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + HORIZON_FRONT_PORCH_H_7E, > > > > > + ctx->dt.hfront_porch.min >> 8); > > > > > + /* HWS */ > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + HORIZON_SYNC_WIDTH_L_7E, > > > > > + ctx->dt.hsync_len.min); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + HORIZON_SYNC_WIDTH_H_7E, > > > > > + ctx->dt.hsync_len.min >> 8); > > > > > + /* HBP */ > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + HORIZON_BACK_PORCH_L_7E, > > > > > + ctx->dt.hback_porch.min); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > > > > > + HORIZON_BACK_PORCH_H_7E, > > > > > + ctx->dt.hback_porch.min >> 8); > > > > > + > > > > > + /* Config DSC decoder internal blank timing for decoder to start */ > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > > + H_BLANK_L, > > > > > + dsc_div(htotal - ctx->dt.hactive.min)); > > > > > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > > + H_BLANK_H, > > > > > + dsc_div(htotal - > > > > > + ctx->dt.hactive.min) > > > > > + >> 8); > > > > > + > > > > > + /* Compress ratio RATIO bit[7:6] */ > > > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); > > > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, > > > > > + (5 - DSC_COMPRESS_RATIO) << 6); > > > > > + /*PPS table*/ > > > > > + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) > > > > > + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, > > > > > + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, > > > > > + > > > > > + &ctx->pps_table[i]); > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > static int anx7625_dsi_video_timing_config(struct anx7625_data > > > > > *ctx) { > > > > > struct device *dev = ctx->dev; > > > > > unsigned long m, n; > > > > > - u16 htotal; > > > > > int ret; > > > > > u8 post_divider = 0; > > > > > > > > > > @@ -506,48 +656,12 @@ static int > > > > > anx7625_dsi_video_timing_config(struct > > > > anx7625_data *ctx) > > > > > ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, > > > > > MIPI_LANE_CTRL_0, > > > > > ctx->pdata.mipi_lanes > > > > > - 1); > > > > > > > > > > - /* Htotal */ > > > > > - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + > > > > > - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); > > > > > - /* Hactive */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); > > > > > - /* HFP */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_FRONT_PORCH_H, > > > > > - ctx->dt.hfront_porch.min >> 8); > > > > > - /* HWS */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); > > > > > - /* HBP */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> > 8); > > > > > - /* Vactive */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, > > > > > - ctx->dt.vactive.min); > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, > > > > > - ctx->dt.vactive.min >> 8); > > > > > - /* VFP */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); > > > > > - /* VWS */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); > > > > > - /* VBP */ > > > > > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, > > > > > - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); > > > > > + /* Video Horizontal timing */ > > > > > + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, > > > > > + true); > > > > > + > > > > > + /* Video Vertical timing */ > > > > > + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); > > > > > + > > > > > > > > Please split this part into two commits: one refactoring timing > > > > programming into two functions and another one introducing DSC support. > > > > It is hard to review timing programming otherwise. > > > > > > OK > > > > > > > > > > > > /* M value */ > > > > > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, > > > > > MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); > > > > > @@ > > > > > -663,9 +777,15 @@ static int anx7625_dsi_config(struct > > > > > anx7625_data > > > > > *ctx) > > > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); > > > > > > > > > > - /* DSC disable */ > > > > > - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > > > - R_DSC_CTRL_0, ~DSC_EN); > > > > > + ret = anx7625_set_dsc_params(ctx); > > > > > + if (anx7625_dsc_check(ctx)) > > > > > + /* DSC enable */ > > > > > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > > > > > + R_DSC_CTRL_0, DSC_EN); > > > > > + else > > > > > + /* DSC disable */ > > > > > + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, > > > > > + R_DSC_CTRL_0, ~DSC_EN); > > > > > > > > > > ret |= anx7625_api_dsi_config(ctx); > > > > > > > > > > @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct > > > > anx7625_data *ctx) > > > > > MIPI_DSI_MODE_VIDEO_HSE | > > > > > MIPI_DSI_HS_PKT_END_ALIGNED; > > > > > > > > > > + dsi->dsc = &ctx->dsc; > > > > > ctx->dsi = dsi; > > > > > > > > > > return 0; > > > > > @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct > > > > > drm_bridge > > > > *bridge, > > > > > struct device *dev = ctx->dev; > > > > > > > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); > > > > > + if (mode->clock > SUPPORT_PIXEL_CLOCK) > > > > > + return MODE_CLOCK_HIGH; > > > > > + > > > > > + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) > > > > > + return MODE_CLOCK_LOW; > > > > > > > > > > - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ > > > > > - if (mode->clock > SUPPORT_PIXEL_CLOCK) { > > > > > - DRM_DEV_DEBUG_DRIVER(dev, > > > > > - "drm mode invalid, pixelclock too high.\n"); > > > > > > > > Any reason for dropping debug message? > > > > > > I'll keep this message. > > > > > > > > > > > > + /* > > > > > + * If hdisplay cannot be divided by DSC compress ratio, then display > > > > > + * will have overlap/shift issue > > > > > + */ > > > > > + if (mode->clock > DSC_PIXEL_CLOCK && > > > > > + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) > > > > > > > > > > > > ... and there still no check that the DSI host supports generating > > > > DSC data. Nor there is an atomic_check() that will check if the mode can be > enabled. > > > > > > I cannot know whether the DSI host supports DSC or not. Anx7625 > > > driver force enable DSC feature if pixel clock higher than DSC_PIXEL_CLOCK. > > > > This is called upstream. Please work on necessary API changes rather > > than claiming that it is not possible. Enabling DSC support when DSC > > is not handled by the DSI host is not acceptable. > > > > Few semi-random ideas: > > - Add DSC checking callbacks to struct mipi_dsi_host_ops. Assume that > > DSC support is not available if callback is not present. The callback > > should accept struct drm_dsc_config and populate const and RC params. > > > > - Add DSC-related flags to struct mipi_dsi_host, describing DSC feature > > availability. Make anx7625 check those flags. > > ... > > - Extend input_/output_bus_format checks to also cover DSC. OK, I'll discuss it with MTK DSI host engineer. > > -- > With best wishes > Dmitry
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 4be34d5c7a3b..bae76d9a665f 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -9,6 +9,7 @@ #include <linux/interrupt.h> #include <linux/iopoll.h> #include <linux/kernel.h> +#include <linux/math64.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/pm_runtime.h> @@ -22,6 +23,7 @@ #include <drm/display/drm_dp_aux_bus.h> #include <drm/display/drm_dp_helper.h> +#include <drm/display/drm_dsc_helper.h> #include <drm/display/drm_hdcp_helper.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ -476,11 +478,159 @@ static int anx7625_set_k_value(struct anx7625_data *ctx) MIPI_DIGITAL_ADJ_1, 0x3D); } +static bool anx7625_dsc_check(struct anx7625_data *ctx) +{ + if (ctx->dt.pixelclock.min > DSC_PIXEL_CLOCK) + return true; + + return false; +} + +static inline int anx7625_h_timing_reg_write(struct anx7625_data *ctx, + struct i2c_client *client, + u8 reg_addr, u16 val, + bool dsc_check) +{ + int ret; + + if (dsc_check && anx7625_dsc_check(ctx)) + val = dsc_div(val); + + ret = anx7625_reg_write(ctx, client, reg_addr, val); + ret |= anx7625_reg_write(ctx, client, reg_addr + 1, val >> 8); + + return ret; +} + +static int anx7625_h_timing_write(struct anx7625_data *ctx, + struct i2c_client *client, + bool rx_h_timing) +{ + u16 htotal; + int ret; + + htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + + ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; + /* Htotal */ + ret = anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_TOTAL_PIXELS_L, + htotal, rx_h_timing); + /* Hactive */ + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_ACTIVE_PIXELS_L, + ctx->dt.hactive.min, rx_h_timing); + /* HFP */ + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_FRONT_PORCH_L, + ctx->dt.hfront_porch.min, rx_h_timing); + /* HWS */ + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_SYNC_WIDTH_L, + ctx->dt.hsync_len.min, rx_h_timing); + /* HBP */ + ret |= anx7625_h_timing_reg_write(ctx, client, HORIZONTAL_BACK_PORCH_L, + ctx->dt.hback_porch.min, rx_h_timing); + + return ret; +} + +static int anx7625_v_timing_write(struct anx7625_data *ctx, + struct i2c_client *client) +{ + int ret; + + /* Vactive */ + ret = anx7625_reg_write(ctx, client, ACTIVE_LINES_L, + ctx->dt.vactive.min); + ret |= anx7625_reg_write(ctx, client, ACTIVE_LINES_H, + ctx->dt.vactive.min >> 8); + /* VFP */ + ret |= anx7625_reg_write(ctx, client, VERTICAL_FRONT_PORCH, + ctx->dt.vfront_porch.min); + /* VWS */ + ret |= anx7625_reg_write(ctx, client, VERTICAL_SYNC_WIDTH, + ctx->dt.vsync_len.min); + /* VBP */ + ret |= anx7625_reg_write(ctx, client, VERTICAL_BACK_PORCH, + ctx->dt.vback_porch.min); + + return ret; +} + +static int anx7625_set_dsc_params(struct anx7625_data *ctx) +{ + int ret, i; + u16 htotal, vtotal; + + if (!anx7625_dsc_check(ctx)) + return 0; + + /* Video Horizontal timing */ + ret = anx7625_h_timing_write(ctx, ctx->i2c.tx_p2_client, false); + + /* Video Vertical timing */ + ret |= anx7625_v_timing_write(ctx, ctx->i2c.tx_p2_client); + + /* Vtotal */ + vtotal = ctx->dt.vactive.min + ctx->dt.vfront_porch.min + + ctx->dt.vback_porch.min + ctx->dt.vsync_len.min; + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_L, + vtotal); + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client, TOTAL_LINES_H, + vtotal >> 8); + /* Htotal */ + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_L_7E, + htotal); + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, TOTAL_PIXEL_H_7E, + htotal >> 8); + /* Hactive */ + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + ACTIVE_PIXEL_L_7E, ctx->dt.hactive.min); + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + ACTIVE_PIXEL_H_7E, ctx->dt.hactive.min >> 8); + /* HFP */ + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + HORIZON_FRONT_PORCH_L_7E, + ctx->dt.hfront_porch.min); + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + HORIZON_FRONT_PORCH_H_7E, + ctx->dt.hfront_porch.min >> 8); + /* HWS */ + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + HORIZON_SYNC_WIDTH_L_7E, + ctx->dt.hsync_len.min); + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + HORIZON_SYNC_WIDTH_H_7E, + ctx->dt.hsync_len.min >> 8); + /* HBP */ + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + HORIZON_BACK_PORCH_L_7E, + ctx->dt.hback_porch.min); + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + HORIZON_BACK_PORCH_H_7E, + ctx->dt.hback_porch.min >> 8); + + /* Config DSC decoder internal blank timing for decoder to start */ + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, + H_BLANK_L, + dsc_div(htotal - ctx->dt.hactive.min)); + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, + H_BLANK_H, + dsc_div(htotal - ctx->dt.hactive.min) >> 8); + + /* Compress ratio RATIO bit[7:6] */ + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, R_I2C_1, 0x3F); + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, R_I2C_1, + (5 - DSC_COMPRESS_RATIO) << 6); + /*PPS table*/ + for (i = 0; i < PPS_SIZE; i += PPS_BLOCK_SIZE) + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p2_client, + R_PPS_REG_0 + i, PPS_BLOCK_SIZE, + &ctx->pps_table[i]); + + return ret; +} + static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) { struct device *dev = ctx->dev; unsigned long m, n; - u16 htotal; int ret; u8 post_divider = 0; @@ -506,48 +656,12 @@ static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx) ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, MIPI_LANE_CTRL_0, ctx->pdata.mipi_lanes - 1); - /* Htotal */ - htotal = ctx->dt.hactive.min + ctx->dt.hfront_porch.min + - ctx->dt.hback_porch.min + ctx->dt.hsync_len.min; - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_TOTAL_PIXELS_L, htotal & 0xFF); - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_TOTAL_PIXELS_H, htotal >> 8); - /* Hactive */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_ACTIVE_PIXELS_L, ctx->dt.hactive.min & 0xFF); - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_ACTIVE_PIXELS_H, ctx->dt.hactive.min >> 8); - /* HFP */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_FRONT_PORCH_L, ctx->dt.hfront_porch.min); - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_FRONT_PORCH_H, - ctx->dt.hfront_porch.min >> 8); - /* HWS */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_SYNC_WIDTH_L, ctx->dt.hsync_len.min); - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_SYNC_WIDTH_H, ctx->dt.hsync_len.min >> 8); - /* HBP */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_BACK_PORCH_L, ctx->dt.hback_porch.min); - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - HORIZONTAL_BACK_PORCH_H, ctx->dt.hback_porch.min >> 8); - /* Vactive */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_L, - ctx->dt.vactive.min); - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, ACTIVE_LINES_H, - ctx->dt.vactive.min >> 8); - /* VFP */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - VERTICAL_FRONT_PORCH, ctx->dt.vfront_porch.min); - /* VWS */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - VERTICAL_SYNC_WIDTH, ctx->dt.vsync_len.min); - /* VBP */ - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p2_client, - VERTICAL_BACK_PORCH, ctx->dt.vback_porch.min); + /* Video Horizontal timing */ + ret |= anx7625_h_timing_write(ctx, ctx->i2c.rx_p2_client, true); + + /* Video Vertical timing */ + ret |= anx7625_v_timing_write(ctx, ctx->i2c.rx_p2_client); + /* M value */ ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, MIPI_PLL_M_NUM_23_16, (m >> 16) & 0xff); @@ -663,9 +777,15 @@ static int anx7625_dsi_config(struct anx7625_data *ctx) DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n"); - /* DSC disable */ - ret = anx7625_write_and(ctx, ctx->i2c.rx_p0_client, - R_DSC_CTRL_0, ~DSC_EN); + ret = anx7625_set_dsc_params(ctx); + if (anx7625_dsc_check(ctx)) + /* DSC enable */ + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, + R_DSC_CTRL_0, DSC_EN); + else + /* DSC disable */ + ret |= anx7625_write_and(ctx, ctx->i2c.rx_p0_client, + R_DSC_CTRL_0, ~DSC_EN); ret |= anx7625_api_dsi_config(ctx); @@ -2083,6 +2203,7 @@ static int anx7625_setup_dsi_device(struct anx7625_data *ctx) MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_HS_PKT_END_ALIGNED; + dsi->dsc = &ctx->dsc; ctx->dsi = dsi; return 0; @@ -2187,19 +2308,69 @@ anx7625_bridge_mode_valid(struct drm_bridge *bridge, struct device *dev = ctx->dev; DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n"); + if (mode->clock > SUPPORT_PIXEL_CLOCK) + return MODE_CLOCK_HIGH; + + if (mode->clock < SUPPORT_MIN_PIXEL_CLOCK) + return MODE_CLOCK_LOW; - /* Max 1200p at 5.4 Ghz, one lane, pixel clock 300M */ - if (mode->clock > SUPPORT_PIXEL_CLOCK) { - DRM_DEV_DEBUG_DRIVER(dev, - "drm mode invalid, pixelclock too high.\n"); + /* + * If hdisplay cannot be divided by DSC compress ratio, then display + * will have overlap/shift issue + */ + if (mode->clock > DSC_PIXEL_CLOCK && + (mode->hdisplay % DSC_COMPRESS_RATIO != 0)) return MODE_CLOCK_HIGH; - } DRM_DEV_DEBUG_DRIVER(dev, "drm mode valid.\n"); return MODE_OK; } +static void anx7625_dsc_enable(struct anx7625_data *ctx, bool en) +{ + int ret; + struct device *dev = ctx->dev; + + if (en) { + ctx->dsc.dsc_version_major = 1; + ctx->dsc.dsc_version_minor = 1; + ctx->dsc.slice_height = 8; + ctx->dsc.slice_width = ctx->dt.hactive.min / DSC_SLICE_NUM; + ctx->dsc.slice_count = DSC_SLICE_NUM; + ctx->dsc.bits_per_component = 8; + ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */ + ctx->dsc.block_pred_enable = true; + ctx->dsc.native_420 = false; + ctx->dsc.native_422 = false; + ctx->dsc.simple_422 = false; + ctx->dsc.vbr_enable = false; + ctx->dsc.convert_rgb = 1; + ctx->dsc.vbr_enable = 0; + + drm_dsc_set_rc_buf_thresh(&ctx->dsc); + drm_dsc_set_const_params(&ctx->dsc); + + ctx->dsc.initial_scale_value = drm_dsc_initial_scale_value(&ctx->dsc); + ctx->dsc.line_buf_depth = ctx->dsc.bits_per_component + 1; + ret = drm_dsc_setup_rc_params(&ctx->dsc, DRM_DSC_1_2_444); + if (ret < 0) + dev_warn(dev, "drm_dsc_setup_rc_params ret %d\n", ret); + + ret = drm_dsc_compute_rc_parameters(&ctx->dsc); + if (ret) + dev_warn(dev, "drm dsc compute rc parameters failed ret %d\n", ret); + + drm_dsc_pps_payload_pack((struct drm_dsc_picture_parameter_set *)&ctx->pps_table, + &ctx->dsc); + dev_dbg(dev, "anx7625 enable dsc\n"); + } else { + ctx->dsc.dsc_version_major = 0; + ctx->dsc.dsc_version_minor = 0; + dev_dbg(dev, "anx7625 disable dsc\n"); + } +} + static void anx7625_bridge_mode_set(struct drm_bridge *bridge, const struct drm_display_mode *old_mode, const struct drm_display_mode *mode) @@ -2244,6 +2415,8 @@ static void anx7625_bridge_mode_set(struct drm_bridge *bridge, DRM_DEV_DEBUG_DRIVER(dev, "vsync_end(%d),vtotal(%d).\n", mode->vsync_end, mode->vtotal); + + anx7625_dsc_enable(ctx, anx7625_dsc_check(ctx)); } static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, @@ -2258,10 +2431,6 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n"); - /* No need fixup for external monitor */ - if (!ctx->pdata.panel_bridge) - return true; - hsync = mode->hsync_end - mode->hsync_start; hfp = mode->hsync_start - mode->hdisplay; hbp = mode->htotal - mode->hsync_end; @@ -2272,12 +2441,24 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, hsync, hfp, hbp, adj->clock); DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d), hsync_end(%d), htot(%d)\n", adj->hsync_start, adj->hsync_end, adj->htotal); - adj_hfp = hfp; adj_hsync = hsync; adj_hbp = hbp; adj_hblanking = hblanking; + if (mode->clock > DSC_PIXEL_CLOCK) { + adj_hsync = DSC_HSYNC_LEN; + adj_hfp = DSC_HFP_LEN; + adj_hbp = DSC_HBP_LEN; + vref = (u32)div_u64((u64)adj->clock * 1000 * 1000, + adj->htotal * adj->vtotal); + goto calculate_timing; + } + + /* No need fixup for external monitor */ + if (!ctx->pdata.panel_bridge) + return true; + /* HFP needs to be even */ if (hfp & 0x1) { adj_hfp += 1; @@ -2349,6 +2530,8 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, adj_hfp -= delta_adj; } +calculate_timing: + DRM_DEV_DEBUG_DRIVER(dev, "after mode fixup\n"); DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d), hfp(%d), hbp(%d), clock(%d)\n", adj_hsync, adj_hfp, adj_hbp, adj->clock); @@ -2357,6 +2540,10 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, adj->hsync_start = adj->hdisplay + adj_hfp; adj->hsync_end = adj->hsync_start + adj_hsync; adj->htotal = adj->hsync_end + adj_hbp; + if (mode->clock > DSC_PIXEL_CLOCK) + adj->clock = (u32)div_u64((u64)vref * adj->htotal * adj->vtotal, + 1000 * 1000); + DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d), hsync_end(%d), htot(%d)\n", adj->hsync_start, adj->hsync_end, adj->htotal); diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h index eb5580f1ab2f..50619d6f3545 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.h +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h @@ -149,6 +149,8 @@ #define HFP_HBP_DEF ((HBLANKING_MIN - SYNC_LEN_DEF) / 2) #define VIDEO_CONTROL_0 0x08 +#define TOTAL_LINES_L 0x12 +#define TOTAL_LINES_H 0x13 #define ACTIVE_LINES_L 0x14 #define ACTIVE_LINES_H 0x15 /* Bit[7:6] are reserved */ #define VERTICAL_FRONT_PORCH 0x16 @@ -166,6 +168,32 @@ #define HORIZONTAL_BACK_PORCH_L 0x21 #define HORIZONTAL_BACK_PORCH_H 0x22 /* Bit[7:4] are reserved */ +#define H_BLANK_L 0x3E +#define H_BLANK_H 0x3F +#define DSC_COMPRESS_RATIO 3 +#define dsc_div(X) ((X) / DSC_COMPRESS_RATIO) +#define DSC_SLICE_NUM 2 +#define DSC_PIXEL_CLOCK 250000 +#define DSC_HSYNC_LEN 90 +#define DSC_HFP_LEN 177 +#define DSC_HBP_LEN 297 + +#define TOTAL_PIXEL_L_7E 0x50 +#define TOTAL_PIXEL_H_7E 0x51 /* bit[7:6] are reserved */ +#define ACTIVE_PIXEL_L_7E 0x52 +#define ACTIVE_PIXEL_H_7E 0x53 /* bit[7:6] are reserved */ +#define HORIZON_FRONT_PORCH_L_7E 0x54 +#define HORIZON_FRONT_PORCH_H_7E 0x55 +#define HORIZON_SYNC_WIDTH_L_7E 0x56 +#define HORIZON_SYNC_WIDTH_H_7E 0x57 +#define HORIZON_BACK_PORCH_L_7E 0x58 +#define HORIZON_BACK_PORCH_H_7E 0x59 + +#define PPS_SIZE 128 +#define PPS_BLOCK_SIZE 32 +#define R_PPS_REG_0 0x80 +#define R_I2C_1 0x81 + /******** END of I2C Address 0x72 *********/ /***************************************************************/ @@ -415,6 +443,7 @@ enum audio_wd_len { #define MAX_EDID_BLOCK 3 #define EDID_TRY_CNT 3 #define SUPPORT_PIXEL_CLOCK 300000 +#define SUPPORT_MIN_PIXEL_CLOCK 22000 /***************** Display End *****************/ @@ -479,6 +508,8 @@ struct anx7625_data { struct drm_connector *connector; struct mipi_dsi_device *dsi; struct drm_dp_aux aux; + struct drm_dsc_config dsc; + char pps_table[PPS_SIZE]; }; #endif /* __ANX7625_H__ */
4K30(3840x2160 30Hz) timing pixel clock around 297M, for 24bits RGB pixel data format, total transport bandwidth need 297M*24(at least 7.2Gbps) more than anx7625 mipi rx lane bandwidth(maximum 6Gbps, 4lanes, each lane 1.5Gbps). Without DSC function, anx7625 cannot receive 4K30 video timing. When display pixel clock exceed 250M, driver will enable DSC feature, and the compression ratio is 3:1, eg: 4K30's pixel clock around 297M, bandwidth 7.2G will be compressed to 7.2G/3 = 2.4G. Then anx7625 can receive 4K30 video timing and do decompress, then package video data and send to sink device through DP link. Anx7625 is bridge IC, sink monitor only receive normal DP signal from anx7625, sink device didn't know DSC information between SOC and anx7625 v2: 1. Add more commit message 2. Remove unused code 3. Add more comment 4. Remove dsc_en flag Signed-off-by: Xin Ji <xji@analogixsemi.com> --- drivers/gpu/drm/bridge/analogix/anx7625.c | 299 ++++++++++++++++++---- drivers/gpu/drm/bridge/analogix/anx7625.h | 31 +++ 2 files changed, 274 insertions(+), 56 deletions(-)