| Message ID | 1362743515-10152-20-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote: > During the initialization of the DSI protocol registers, we always set > the sources of all DSI channels to L4. However, we don't update the > value in the dsi_data, so we may end up with a different value in the > register and in the dsi_data, leading to DSI problems. > > This patch fixes the issue by initializing also the channel source in > the dsi_data. We set in omap_dsihw_probe: static int __init omap_dsihw_probe(struct platform_device *dsidev) { ... ... /* DSI VCs initialization */ for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) { dsi->vc[i].source = DSI_VC_SOURCE_L4; dsi->vc[i].dssdev = NULL; dsi->vc[i].vc_id = 0; } ... ... } <snip> Archit -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2013-03-11 08:10, Archit Taneja wrote: > Hi, > > On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote: >> During the initialization of the DSI protocol registers, we always set >> the sources of all DSI channels to L4. However, we don't update the >> value in the dsi_data, so we may end up with a different value in the >> register and in the dsi_data, leading to DSI problems. >> >> This patch fixes the issue by initializing also the channel source in >> the dsi_data. > > We set in omap_dsihw_probe: > > static int __init omap_dsihw_probe(struct platform_device *dsidev) > { > ... > ... > /* DSI VCs initialization */ > for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) { > dsi->vc[i].source = DSI_VC_SOURCE_L4; > dsi->vc[i].dssdev = NULL; > dsi->vc[i].vc_id = 0; > } > ... > ... > } Hmm... I did have a bug related to this when prototyping CDF. Ah. Consider this: Panel powers up and uses DSI normally. A DSI VC is set to video mode. Then the panel power down. Then it powers up again, and enables DSI. At this time, dsi_vc_initial_config() is called again, setting the source in the registers to L4. But the source in dsi_data is still VP. So perhaps the whole piece of code from omap_dsihw_probe should be moved to somewhere else (dsi_vc_initial_config() sounds like a good place), so that they are initialized each time the registers are initialized. Tomi
On Monday 11 March 2013 12:32 PM, Tomi Valkeinen wrote: > On 2013-03-11 08:10, Archit Taneja wrote: >> Hi, >> >> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote: >>> During the initialization of the DSI protocol registers, we always set >>> the sources of all DSI channels to L4. However, we don't update the >>> value in the dsi_data, so we may end up with a different value in the >>> register and in the dsi_data, leading to DSI problems. >>> >>> This patch fixes the issue by initializing also the channel source in >>> the dsi_data. >> >> We set in omap_dsihw_probe: >> >> static int __init omap_dsihw_probe(struct platform_device *dsidev) >> { >> ... >> ... >> /* DSI VCs initialization */ >> for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) { >> dsi->vc[i].source = DSI_VC_SOURCE_L4; >> dsi->vc[i].dssdev = NULL; >> dsi->vc[i].vc_id = 0; >> } >> ... >> ... >> } > > Hmm... I did have a bug related to this when prototyping CDF. Ah. > Consider this: > > Panel powers up and uses DSI normally. A DSI VC is set to video mode. > Then the panel power down. Then it powers up again, and enables DSI. At > this time, dsi_vc_initial_config() is called again, setting the source > in the registers to L4. But the source in dsi_data is still VP. Oh ok. > > So perhaps the whole piece of code from omap_dsihw_probe should be moved > to somewhere else (dsi_vc_initial_config() sounds like a good place), so > that they are initialized each time the registers are initialized. VC source is something which seems fine to do in dsi_vc_initial_config(). I'm not sure about the dssdev and vc_id fields though, we would need to call omap_dsi_request_vc() and omap_dsi_set_vc_id() again after a power off. Currently, we do it only once in taal_probe(). Archit -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2013-03-11 10:15, Archit Taneja wrote: > On Monday 11 March 2013 12:32 PM, Tomi Valkeinen wrote: >> On 2013-03-11 08:10, Archit Taneja wrote: >>> Hi, >>> >>> On Friday 08 March 2013 05:21 PM, Tomi Valkeinen wrote: >>>> During the initialization of the DSI protocol registers, we always set >>>> the sources of all DSI channels to L4. However, we don't update the >>>> value in the dsi_data, so we may end up with a different value in the >>>> register and in the dsi_data, leading to DSI problems. >>>> >>>> This patch fixes the issue by initializing also the channel source in >>>> the dsi_data. >>> >>> We set in omap_dsihw_probe: >>> >>> static int __init omap_dsihw_probe(struct platform_device *dsidev) >>> { >>> ... >>> ... >>> /* DSI VCs initialization */ >>> for (i = 0; i < ARRAY_SIZE(dsi->vc); i++) { >>> dsi->vc[i].source = DSI_VC_SOURCE_L4; >>> dsi->vc[i].dssdev = NULL; >>> dsi->vc[i].vc_id = 0; >>> } >>> ... >>> ... >>> } >> >> Hmm... I did have a bug related to this when prototyping CDF. Ah. >> Consider this: >> >> Panel powers up and uses DSI normally. A DSI VC is set to video mode. >> Then the panel power down. Then it powers up again, and enables DSI. At >> this time, dsi_vc_initial_config() is called again, setting the source >> in the registers to L4. But the source in dsi_data is still VP. > > Oh ok. > >> >> So perhaps the whole piece of code from omap_dsihw_probe should be moved >> to somewhere else (dsi_vc_initial_config() sounds like a good place), so >> that they are initialized each time the registers are initialized. > > VC source is something which seems fine to do in > dsi_vc_initial_config(). I'm not sure about the dssdev and vc_id fields > though, we would need to call omap_dsi_request_vc() and > omap_dsi_set_vc_id() again after a power off. Currently, we do it only > once in taal_probe(). Oh, right. Well, neither dssdev nor vc_id is written to registers, so they won't have similar issues in any case. So I guess this patch is fine as it is, and we do not need to touch dssdev nor vc_id. Tomi
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c index da8a678..2d95ed9 100644 --- a/drivers/video/omap2/dss/dsi.c +++ b/drivers/video/omap2/dss/dsi.c @@ -2794,6 +2794,7 @@ static int dsi_vc_enable(struct platform_device *dsidev, int channel, static void dsi_vc_initial_config(struct platform_device *dsidev, int channel) { + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev); u32 r; DSSDBG("Initial config of virtual channel %d", channel); @@ -2818,6 +2819,8 @@ static void dsi_vc_initial_config(struct platform_device *dsidev, int channel) r = FLD_MOD(r, 4, 23, 21); /* DMA_TX_REQ_NB = no dma */ dsi_write_reg(dsidev, DSI_VC_CTRL(channel), r); + + dsi->vc[channel].source = DSI_VC_SOURCE_L4; } static int dsi_vc_config_source(struct platform_device *dsidev, int channel,
During the initialization of the DSI protocol registers, we always set the sources of all DSI channels to L4. However, we don't update the value in the dsi_data, so we may end up with a different value in the register and in the dsi_data, leading to DSI problems. This patch fixes the issue by initializing also the channel source in the dsi_data. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/video/omap2/dss/dsi.c | 3 +++ 1 file changed, 3 insertions(+)