| Message ID | 6c6551b5f9543c954467448d3c4f71b74eca7c84.1464010715.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/23/16 17:41, Jyri Sarha wrote: > @@ -4112,7 +4115,8 @@ static const struct dispc_features omap54xx_dispc_feats = { > .has_writeback = true, > .supports_double_pixel = true, > .reverse_ilace_field_order = true, > - .has_gamma_tables = true, > + .has_gamma_table = true, > + .has_gamma_i734_bug = true, > }; Oops, the has_gamma_tables => has_gamma_table should be in the previous patch.
On 23/05/16 17:41, Jyri Sarha wrote: > Work-a-round for errata i734 in DSS dispc I think it's "workaround" =). > - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled > > For gamma tables to work on LCD1 the GFX plane has to be used at least > once after DSS HW has come out of reset. The work-a-round sets up a > minimal LCD setup with GFX plane and waits for one vertical sync irq > before disabling the setup and continuing with the context > restore. The physical outputs are gated during the operation. This > work-a-round requires that gamma table's LOADMODE is set to 0x2 in > DISPC_CONTROL1 register. This LOADMODE comment is a bit odd. You should say why, and how it's handled, and what's "0x2". But then, I'm not sure if the whole comment is needed at all. The driver is made to work only with LOADMODE=2. Maybe the real point here is that the WA needs to happen after the initial DSS register config. > For details see: > OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata > Literature Number: SWPZ037E > Or some other relevant errata document for the DSS IP version. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dispc.c | 176 +++++++++++++++++++++++++++++++++++- > 1 file changed, 174 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c > index b284622..0e04bed 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > @@ -115,6 +115,8 @@ struct dispc_features { > bool reverse_ilace_field_order:1; > > bool has_gamma_table:1; > + > + bool has_gamma_i734_bug:1; > }; > > #define DISPC_MAX_NR_FIFOS 5 > @@ -3931,7 +3933,7 @@ static int dispc_init_gamma_tables(void) > int channel; > > if (!dispc.feat->has_gamma_table) > - return; > + return 0; This doesn't look like it belongs to this patch. > > for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) { > const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma; > @@ -4087,6 +4089,7 @@ static const struct dispc_features omap44xx_dispc_feats = { > .supports_double_pixel = true, > .reverse_ilace_field_order = true, > .has_gamma_table = true, > + .has_gamma_i734_bug = true, > }; > > static const struct dispc_features omap54xx_dispc_feats = { > @@ -4112,7 +4115,8 @@ static const struct dispc_features omap54xx_dispc_feats = { > .has_writeback = true, > .supports_double_pixel = true, > .reverse_ilace_field_order = true, > - .has_gamma_tables = true, > + .has_gamma_table = true, > + .has_gamma_i734_bug = true, > }; > > static int dispc_init_features(struct platform_device *pdev) > @@ -4204,6 +4208,166 @@ void dispc_free_irq(void *dev_id) > } > EXPORT_SYMBOL(dispc_free_irq); > > +/* > + * Work-a-round for errata i734 in DSS dispc > + * - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled > + * > + * For gamma tables to work on LCD1 the GFX plane has to be used at > + * least once after DSS HW has come out of reset. The work-a-round > + * sets up a minimal LCD setup with GFX plane and waits for one > + * vertical sync irq before disabling the setup and continuing with > + * the context restore. The physical outputs are gated during the > + * operation. This work-a-round requires that gamma table's LOADMODE > + * is set to 0x2 in DISPC_CONTROL1 register. > + * > + * For details see: > + * OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata > + * Literature Number: SWPZ037E > + * Or some other relevant errata document for the DSS IP version. > + */ > + > +static const struct dispc_errata_i734_data { > + struct omap_video_timings timings; > + struct omap_overlay_info ovli; > + struct omap_overlay_manager_info mgri; > + struct dss_lcd_mgr_config lcd_conf; > +} i734 = { > + .timings = { > + .x_res = 8, .y_res = 1, > + .pixelclock = 16000000, > + .hsw = 8, .hfp = 4, .hbp = 4, > + .vsw = 1, .vfp = 1, .vbp = 1, > + .vsync_level = OMAPDSS_SIG_ACTIVE_LOW, > + .hsync_level = OMAPDSS_SIG_ACTIVE_LOW, > + .interlace = false, > + .data_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE, > + .de_level = OMAPDSS_SIG_ACTIVE_HIGH, > + .sync_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE, > + .double_pixel = false, > + }, > + .ovli = { > + .screen_width = 1, > + .width = 1, .height = 1, > + .color_mode = OMAP_DSS_COLOR_RGB24U, > + .rotation = OMAP_DSS_ROT_0, > + .rotation_type = OMAP_DSS_ROT_DMA, > + .mirror = 0, > + .pos_x = 0, .pos_y = 0, > + .out_width = 0, .out_height = 0, > + .global_alpha = 0xff, > + .pre_mult_alpha = 0, > + .zorder = 0, > + }, > + .mgri = { > + .default_color = 0, > + .trans_enabled = false, > + .partial_alpha_enabled = false, > + .cpr_enable = false, > + }, > + .lcd_conf = { > + .io_pad_mode = DSS_IO_PAD_MODE_BYPASS, > + .stallmode = false, > + .fifohandcheck = false, > + .clock_info = { > + .lck_div = 1, > + .pck_div = 2, > + }, > + .video_port_width = 24, > + .lcden_sig_polarity = 0, > + }, > +}; > + > +static struct i734_buf { > + size_t size; > + dma_addr_t paddr; > + void *vaddr; > +} i734_buf; > + > +static int dispc_errata_i734_wa_init(void) > +{ > + if (!dispc.feat->has_gamma_i734_bug) > + return 0; > + > + i734_buf.size = i734.ovli.width * i734.ovli.height * > + color_mode_to_bpp(i734.ovli.color_mode) / 8; > + > + i734_buf.vaddr = dma_alloc_writecombine(&dispc.pdev->dev, i734_buf.size, > + &i734_buf.paddr, GFP_KERNEL); > + if (IS_ERR(i734_buf.vaddr)) { Hmm I don't think dma_alloc_* returns an err ptr? > + dev_err(&dispc.pdev->dev, > + "%s: dma_alloc_writecombine failed: %ld\n", > + __func__, PTR_ERR(i734_buf.vaddr)); > + return PTR_ERR(i734_buf.vaddr); > + } > + > + return 0; > +} > + > +static void dispc_errata_i734_wa_fini(void) > +{ > + if (!dispc.feat->has_gamma_i734_bug) > + return; > + > + dma_free_writecombine(&dispc.pdev->dev, i734_buf.size, i734_buf.vaddr, > + i734_buf.paddr); > +} > + > +static void dispc_errata_i734_wa(void) > +{ > + u32 framedone_irq = dispc_mgr_get_framedone_irq(OMAP_DSS_CHANNEL_LCD); > + struct omap_overlay_info ovli; > + struct dss_lcd_mgr_config lcd_conf; > + u32 gatestate; > + unsigned int count; > + > + if (!dispc.feat->has_gamma_i734_bug) > + return; > + > + gatestate = REG_GET(DISPC_CONTROL, 8, 4); Still wrong register. > + > + ovli = i734.ovli; > + ovli.paddr = i734_buf.paddr; > + lcd_conf = i734.lcd_conf; > + > + /* Gate all LCD1 outputs */ > + REG_FLD_MOD(DISPC_CONFIG, 0x1f, 8, 4); > + > + /* Setup and enable GFX plane */ > + dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD); > + dispc_ovl_setup(OMAP_DSS_GFX, &ovli, false, &i734.timings, false); > + dispc_ovl_enable(OMAP_DSS_GFX, true); > + > + /* Set up and enable display manager for LCD1 */ > + dispc_mgr_setup(OMAP_DSS_CHANNEL_LCD, &i734.mgri); > + dispc_calc_clock_rates(dss_get_dispc_clk_rate(), > + &lcd_conf.clock_info); > + dispc_mgr_set_lcd_config(OMAP_DSS_CHANNEL_LCD, &lcd_conf); > + dispc_mgr_set_timings(OMAP_DSS_CHANNEL_LCD, &i734.timings); > + > + dispc_clear_irqstatus(framedone_irq); > + > + /* Enable and shut the channel to produce just one frame */ > + dispc_mgr_enable(OMAP_DSS_CHANNEL_LCD, true); > + dispc_mgr_enable(OMAP_DSS_CHANNEL_LCD, false); > + > + /* Busy wait for framedone (can't fiddle with irq handlers in resume) */ > + count = 0; > + while (!(dispc_read_irqstatus() & framedone_irq)) > + if (count++ > 10000) { > + dev_err(&dispc.pdev->dev, "%s: framedone timeout\n", > + __func__); > + break; > + } Add { } for the 'while' too. Busyloops are always a bit tricky... I would perhaps change the loop above so that you first busy loop for a certain amount of loops, but after that udelay() for a short period in each loop. And it would be good to have a note here why busyloop is fine and what is the calculated time the frame takes. I mean, the reason why busy loop is preferred is what you mention, that we can't easily have a irq handler here, but the reason busyloop is fine is that the frame is so small. Tomi
On 05/23/16 18:06, Tomi Valkeinen wrote: >> For gamma tables to work on LCD1 the GFX plane has to be used at least >> > once after DSS HW has come out of reset. The work-a-round sets up a >> > minimal LCD setup with GFX plane and waits for one vertical sync irq >> > before disabling the setup and continuing with the context >> > restore. The physical outputs are gated during the operation. This >> > work-a-round requires that gamma table's LOADMODE is set to 0x2 in >> > DISPC_CONTROL1 register. > This LOADMODE comment is a bit odd. You should say why, and how it's > handled, and what's "0x2". But then, I'm not sure if the whole comment > is needed at all. The driver is made to work only with LOADMODE=2. > It is there also in the comment and my point is to notify that if the gamma support is ever changed to use some other mode, the workaround needs to change too. It is pretty hard to draw the line to how deep in details one should go in comments or commit descriptions. I added the register name so that someone is interested he can find the full description of the LOADMODE from TRM using that as a key word. I think I wont mention the LOADMODE in the description but leave it on the code comments. > Maybe the real point here is that the WA needs to happen after the > initial DSS register config. > I think that detail should be in the code comments, if it is needed at all. BR, Jyri
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index b284622..0e04bed 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -115,6 +115,8 @@ struct dispc_features { bool reverse_ilace_field_order:1; bool has_gamma_table:1; + + bool has_gamma_i734_bug:1; }; #define DISPC_MAX_NR_FIFOS 5 @@ -3931,7 +3933,7 @@ static int dispc_init_gamma_tables(void) int channel; if (!dispc.feat->has_gamma_table) - return; + return 0; for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) { const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma; @@ -4087,6 +4089,7 @@ static const struct dispc_features omap44xx_dispc_feats = { .supports_double_pixel = true, .reverse_ilace_field_order = true, .has_gamma_table = true, + .has_gamma_i734_bug = true, }; static const struct dispc_features omap54xx_dispc_feats = { @@ -4112,7 +4115,8 @@ static const struct dispc_features omap54xx_dispc_feats = { .has_writeback = true, .supports_double_pixel = true, .reverse_ilace_field_order = true, - .has_gamma_tables = true, + .has_gamma_table = true, + .has_gamma_i734_bug = true, }; static int dispc_init_features(struct platform_device *pdev) @@ -4204,6 +4208,166 @@ void dispc_free_irq(void *dev_id) } EXPORT_SYMBOL(dispc_free_irq); +/* + * Work-a-round for errata i734 in DSS dispc + * - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled + * + * For gamma tables to work on LCD1 the GFX plane has to be used at + * least once after DSS HW has come out of reset. The work-a-round + * sets up a minimal LCD setup with GFX plane and waits for one + * vertical sync irq before disabling the setup and continuing with + * the context restore. The physical outputs are gated during the + * operation. This work-a-round requires that gamma table's LOADMODE + * is set to 0x2 in DISPC_CONTROL1 register. + * + * For details see: + * OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata + * Literature Number: SWPZ037E + * Or some other relevant errata document for the DSS IP version. + */ + +static const struct dispc_errata_i734_data { + struct omap_video_timings timings; + struct omap_overlay_info ovli; + struct omap_overlay_manager_info mgri; + struct dss_lcd_mgr_config lcd_conf; +} i734 = { + .timings = { + .x_res = 8, .y_res = 1, + .pixelclock = 16000000, + .hsw = 8, .hfp = 4, .hbp = 4, + .vsw = 1, .vfp = 1, .vbp = 1, + .vsync_level = OMAPDSS_SIG_ACTIVE_LOW, + .hsync_level = OMAPDSS_SIG_ACTIVE_LOW, + .interlace = false, + .data_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE, + .de_level = OMAPDSS_SIG_ACTIVE_HIGH, + .sync_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE, + .double_pixel = false, + }, + .ovli = { + .screen_width = 1, + .width = 1, .height = 1, + .color_mode = OMAP_DSS_COLOR_RGB24U, + .rotation = OMAP_DSS_ROT_0, + .rotation_type = OMAP_DSS_ROT_DMA, + .mirror = 0, + .pos_x = 0, .pos_y = 0, + .out_width = 0, .out_height = 0, + .global_alpha = 0xff, + .pre_mult_alpha = 0, + .zorder = 0, + }, + .mgri = { + .default_color = 0, + .trans_enabled = false, + .partial_alpha_enabled = false, + .cpr_enable = false, + }, + .lcd_conf = { + .io_pad_mode = DSS_IO_PAD_MODE_BYPASS, + .stallmode = false, + .fifohandcheck = false, + .clock_info = { + .lck_div = 1, + .pck_div = 2, + }, + .video_port_width = 24, + .lcden_sig_polarity = 0, + }, +}; + +static struct i734_buf { + size_t size; + dma_addr_t paddr; + void *vaddr; +} i734_buf; + +static int dispc_errata_i734_wa_init(void) +{ + if (!dispc.feat->has_gamma_i734_bug) + return 0; + + i734_buf.size = i734.ovli.width * i734.ovli.height * + color_mode_to_bpp(i734.ovli.color_mode) / 8; + + i734_buf.vaddr = dma_alloc_writecombine(&dispc.pdev->dev, i734_buf.size, + &i734_buf.paddr, GFP_KERNEL); + if (IS_ERR(i734_buf.vaddr)) { + dev_err(&dispc.pdev->dev, + "%s: dma_alloc_writecombine failed: %ld\n", + __func__, PTR_ERR(i734_buf.vaddr)); + return PTR_ERR(i734_buf.vaddr); + } + + return 0; +} + +static void dispc_errata_i734_wa_fini(void) +{ + if (!dispc.feat->has_gamma_i734_bug) + return; + + dma_free_writecombine(&dispc.pdev->dev, i734_buf.size, i734_buf.vaddr, + i734_buf.paddr); +} + +static void dispc_errata_i734_wa(void) +{ + u32 framedone_irq = dispc_mgr_get_framedone_irq(OMAP_DSS_CHANNEL_LCD); + struct omap_overlay_info ovli; + struct dss_lcd_mgr_config lcd_conf; + u32 gatestate; + unsigned int count; + + if (!dispc.feat->has_gamma_i734_bug) + return; + + gatestate = REG_GET(DISPC_CONTROL, 8, 4); + + ovli = i734.ovli; + ovli.paddr = i734_buf.paddr; + lcd_conf = i734.lcd_conf; + + /* Gate all LCD1 outputs */ + REG_FLD_MOD(DISPC_CONFIG, 0x1f, 8, 4); + + /* Setup and enable GFX plane */ + dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD); + dispc_ovl_setup(OMAP_DSS_GFX, &ovli, false, &i734.timings, false); + dispc_ovl_enable(OMAP_DSS_GFX, true); + + /* Set up and enable display manager for LCD1 */ + dispc_mgr_setup(OMAP_DSS_CHANNEL_LCD, &i734.mgri); + dispc_calc_clock_rates(dss_get_dispc_clk_rate(), + &lcd_conf.clock_info); + dispc_mgr_set_lcd_config(OMAP_DSS_CHANNEL_LCD, &lcd_conf); + dispc_mgr_set_timings(OMAP_DSS_CHANNEL_LCD, &i734.timings); + + dispc_clear_irqstatus(framedone_irq); + + /* Enable and shut the channel to produce just one frame */ + dispc_mgr_enable(OMAP_DSS_CHANNEL_LCD, true); + dispc_mgr_enable(OMAP_DSS_CHANNEL_LCD, false); + + /* Busy wait for framedone (can't fiddle with irq handlers in resume) */ + count = 0; + while (!(dispc_read_irqstatus() & framedone_irq)) + if (count++ > 10000) { + dev_err(&dispc.pdev->dev, "%s: framedone timeout\n", + __func__); + break; + } + + dispc_ovl_enable(OMAP_DSS_GFX, false); + + /* Clear all irq bits before continuing */ + dispc_clear_irqstatus(0xffffffff); + + /* Restore the original state to LCD1 output gates */ + REG_FLD_MOD(DISPC_CONFIG, gatestate, 8, 4); +} + /* DISPC HW IP initialisation */ static int dispc_bind(struct device *dev, struct device *master, void *data) { @@ -4221,6 +4385,10 @@ static int dispc_bind(struct device *dev, struct device *master, void *data) if (r) return r; + r = dispc_errata_i734_wa_init(); + if (r) + return r; + dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0); if (!dispc_mem) { DSSERR("can't get IORESOURCE_MEM DISPC\n"); @@ -4285,6 +4453,8 @@ static void dispc_unbind(struct device *dev, struct device *master, void *data) { pm_runtime_disable(dev); + + dispc_errata_i734_wa_fini(); } static const struct component_ops dispc_component_ops = { @@ -4327,6 +4497,8 @@ static int dispc_runtime_resume(struct device *dev) if (REG_GET(DISPC_CONFIG, 2, 1) != OMAP_DSS_LOAD_FRAME_ONLY) { _omap_dispc_initial_config(); + dispc_errata_i734_wa(); + dispc_restore_context(); dispc_restore_gamma_tables();
Work-a-round for errata i734 in DSS dispc - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled For gamma tables to work on LCD1 the GFX plane has to be used at least once after DSS HW has come out of reset. The work-a-round sets up a minimal LCD setup with GFX plane and waits for one vertical sync irq before disabling the setup and continuing with the context restore. The physical outputs are gated during the operation. This work-a-round requires that gamma table's LOADMODE is set to 0x2 in DISPC_CONTROL1 register. For details see: OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata Literature Number: SWPZ037E Or some other relevant errata document for the DSS IP version. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/omapdrm/dss/dispc.c | 176 +++++++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 2 deletions(-)