| Message ID | 1309405483-6876-1-git-send-email-arve@android.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Tomi Valkeinen |
| Headers | show |
Hi, On Wed, 2011-06-29 at 20:44 -0700, Arve Hjønnevåg wrote: > Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 > Signed-off-by: Arve Hjønnevåg <arve@android.com> > --- > drivers/video/omap2/dss/dsi.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) This feels somehow partial... Why do you want to read generic packets if there are no functions to send generic packets? And always write a patch description. Tomi > > diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c > index c16b933..6975645 100644 > --- a/drivers/video/omap2/dss/dsi.c > +++ b/drivers/video/omap2/dss/dsi.c > @@ -206,6 +206,7 @@ struct dsi_reg { u16 idx; }; > #define DSI_DT_DCS_LONG_WRITE 0x39 > > #define DSI_DT_RX_ACK_WITH_ERR 0x02 > +#define DSI_DT_RX_LONG_READ 0x1a > #define DSI_DT_RX_DCS_LONG_READ 0x1c > #define DSI_DT_RX_SHORT_READ_1 0x21 > #define DSI_DT_RX_SHORT_READ_2 0x22 > @@ -2943,6 +2944,10 @@ static u16 dsi_vc_flush_receive_data(struct platform_device *dsidev, > } else if (dt == DSI_DT_RX_SHORT_READ_2) { > DSSERR("\tDCS short response, 2 byte: %#x\n", > FLD_GET(val, 23, 8)); > + } else if (dt == DSI_DT_RX_LONG_READ) { > + DSSERR("\tlong response, len %d\n", > + FLD_GET(val, 23, 8)); > + dsi_vc_flush_long_data(dsidev, channel); > } else if (dt == DSI_DT_RX_DCS_LONG_READ) { > DSSERR("\tDCS long response, len %d\n", > FLD_GET(val, 23, 8)); > @@ -3287,7 +3292,7 @@ int dsi_vc_dcs_read(struct omap_dss_device *dssdev, int channel, u8 dcs_cmd, > buf[1] = (data >> 8) & 0xff; > > return 2; > - } else if (dt == DSI_DT_RX_DCS_LONG_READ) { > + } else if (dt == DSI_DT_RX_DCS_LONG_READ || dt == DSI_DT_RX_LONG_READ) { > int w; > int len = FLD_GET(val, 23, 8); > if (dsi->debug_read) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/8/4 Tomi Valkeinen <tomi.valkeinen@ti.com>: > Hi, > > On Wed, 2011-06-29 at 20:44 -0700, Arve Hjønnevåg wrote: >> Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 >> Signed-off-by: Arve Hjønnevåg <arve@android.com> >> --- >> drivers/video/omap2/dss/dsi.c | 7 ++++++- >> 1 files changed, 6 insertions(+), 1 deletions(-) > > This feels somehow partial... Why do you want to read generic packets if > there are no functions to send generic packets? > The chip responds with a generic packet when reading from some registers. This is a simple fix while adding support for sending generic packets would probably require an api change. > And always write a patch description. > > Tomi > >> >> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c >> index c16b933..6975645 100644 >> --- a/drivers/video/omap2/dss/dsi.c >> +++ b/drivers/video/omap2/dss/dsi.c >> @@ -206,6 +206,7 @@ struct dsi_reg { u16 idx; }; >> #define DSI_DT_DCS_LONG_WRITE 0x39 >> >> #define DSI_DT_RX_ACK_WITH_ERR 0x02 >> +#define DSI_DT_RX_LONG_READ 0x1a >> #define DSI_DT_RX_DCS_LONG_READ 0x1c >> #define DSI_DT_RX_SHORT_READ_1 0x21 >> #define DSI_DT_RX_SHORT_READ_2 0x22 >> @@ -2943,6 +2944,10 @@ static u16 dsi_vc_flush_receive_data(struct platform_device *dsidev, >> } else if (dt == DSI_DT_RX_SHORT_READ_2) { >> DSSERR("\tDCS short response, 2 byte: %#x\n", >> FLD_GET(val, 23, 8)); >> + } else if (dt == DSI_DT_RX_LONG_READ) { >> + DSSERR("\tlong response, len %d\n", >> + FLD_GET(val, 23, 8)); >> + dsi_vc_flush_long_data(dsidev, channel); >> } else if (dt == DSI_DT_RX_DCS_LONG_READ) { >> DSSERR("\tDCS long response, len %d\n", >> FLD_GET(val, 23, 8)); >> @@ -3287,7 +3292,7 @@ int dsi_vc_dcs_read(struct omap_dss_device *dssdev, int channel, u8 dcs_cmd, >> buf[1] = (data >> 8) & 0xff; >> >> return 2; >> - } else if (dt == DSI_DT_RX_DCS_LONG_READ) { >> + } else if (dt == DSI_DT_RX_DCS_LONG_READ || dt == DSI_DT_RX_LONG_READ) { >> int w; >> int len = FLD_GET(val, 23, 8); >> if (dsi->debug_read) > > >
On Thu, 2011-08-04 at 18:15 -0700, Arve Hjønnevåg wrote: > 2011/8/4 Tomi Valkeinen <tomi.valkeinen@ti.com>: > > Hi, > > > > On Wed, 2011-06-29 at 20:44 -0700, Arve Hjønnevåg wrote: > >> Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 > >> Signed-off-by: Arve Hjønnevåg <arve@android.com> > >> --- > >> drivers/video/omap2/dss/dsi.c | 7 ++++++- > >> 1 files changed, 6 insertions(+), 1 deletions(-) > > > > This feels somehow partial... Why do you want to read generic packets if > > there are no functions to send generic packets? > > > > The chip responds with a generic packet when reading from some > registers. This is a simple fix while adding support for sending > generic packets would probably require an api change. What command do you use to read the register? DCS? If so, sounds rather strange HW implementation. If you use generic commands, for which you have support in your kernel, I think it's better to add both write and read support at the same time. Adding just read support, without any way to actually use the read, doesn't sound sensible. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/8/5 Tomi Valkeinen <tomi.valkeinen@ti.com>: > On Thu, 2011-08-04 at 18:15 -0700, Arve Hjønnevåg wrote: >> 2011/8/4 Tomi Valkeinen <tomi.valkeinen@ti.com>: >> > Hi, >> > >> > On Wed, 2011-06-29 at 20:44 -0700, Arve Hjønnevåg wrote: >> >> Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 >> >> Signed-off-by: Arve Hjønnevåg <arve@android.com> >> >> --- >> >> drivers/video/omap2/dss/dsi.c | 7 ++++++- >> >> 1 files changed, 6 insertions(+), 1 deletions(-) >> > >> > This feels somehow partial... Why do you want to read generic packets if >> > there are no functions to send generic packets? >> > >> >> The chip responds with a generic packet when reading from some >> registers. This is a simple fix while adding support for sending >> generic packets would probably require an api change. > > What command do you use to read the register? DCS? If so, sounds rather > strange HW implementation. > This change modifies dsi_vc_dcs_read to accept a generic response packet. If you want a separate generic read command that would also work, but the it would make it a little harder to write the driver since you then would need to know what type of response the chip sends. > If you use generic commands, for which you have support in your kernel, > I think it's better to add both write and read support at the same time. > Adding just read support, without any way to actually use the read, > doesn't sound sensible. > This change works with the existing read command. More commands can be added later.
Hi, On Fri, 2011-08-05 at 00:17 -0700, Arve Hjønnevåg wrote: > 2011/8/5 Tomi Valkeinen <tomi.valkeinen@ti.com>: > > On Thu, 2011-08-04 at 18:15 -0700, Arve Hjønnevåg wrote: > >> 2011/8/4 Tomi Valkeinen <tomi.valkeinen@ti.com>: > >> > Hi, > >> > > >> > On Wed, 2011-06-29 at 20:44 -0700, Arve Hjønnevåg wrote: > >> >> Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 > >> >> Signed-off-by: Arve Hjønnevåg <arve@android.com> > >> >> --- > >> >> drivers/video/omap2/dss/dsi.c | 7 ++++++- > >> >> 1 files changed, 6 insertions(+), 1 deletions(-) > >> > > >> > This feels somehow partial... Why do you want to read generic packets if > >> > there are no functions to send generic packets? > >> > > >> > >> The chip responds with a generic packet when reading from some > >> registers. This is a simple fix while adding support for sending > >> generic packets would probably require an api change. > > > > What command do you use to read the register? DCS? If so, sounds rather > > strange HW implementation. > > > > This change modifies dsi_vc_dcs_read to accept a generic response Ok, so you do send a DCS read command, and you get a generic response. > packet. If you want a separate generic read command that would also > work, but the it would make it a little harder to write the driver > since you then would need to know what type of response the chip > sends. Well, you do know what type of response the chip sends. According to MIPI DSI spec, a DCS read is answered with DCS short/long response, and a generic read is answered with generic short/long response. If your HW doesn't work like that, it's against the spec. If a broken chip like that was widely used, I guess we would have to add a hack for that, but until such day I want to keep the driver working as the spec says. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/8/5 Tomi Valkeinen <tomi.valkeinen@ti.com>: > Hi, > > On Fri, 2011-08-05 at 00:17 -0700, Arve Hjønnevåg wrote: >> 2011/8/5 Tomi Valkeinen <tomi.valkeinen@ti.com>: >> > On Thu, 2011-08-04 at 18:15 -0700, Arve Hjønnevåg wrote: >> >> 2011/8/4 Tomi Valkeinen <tomi.valkeinen@ti.com>: >> >> > Hi, >> >> > >> >> > On Wed, 2011-06-29 at 20:44 -0700, Arve Hjønnevåg wrote: >> >> >> Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 >> >> >> Signed-off-by: Arve Hjønnevåg <arve@android.com> >> >> >> --- >> >> >> drivers/video/omap2/dss/dsi.c | 7 ++++++- >> >> >> 1 files changed, 6 insertions(+), 1 deletions(-) >> >> > >> >> > This feels somehow partial... Why do you want to read generic packets if >> >> > there are no functions to send generic packets? >> >> > >> >> >> >> The chip responds with a generic packet when reading from some >> >> registers. This is a simple fix while adding support for sending >> >> generic packets would probably require an api change. >> > >> > What command do you use to read the register? DCS? If so, sounds rather >> > strange HW implementation. >> > >> >> This change modifies dsi_vc_dcs_read to accept a generic response > > Ok, so you do send a DCS read command, and you get a generic response. > >> packet. If you want a separate generic read command that would also >> work, but the it would make it a little harder to write the driver >> since you then would need to know what type of response the chip >> sends. > > Well, you do know what type of response the chip sends. According to Only because the dsi_vc_dcs_read failed. The examples in the datasheet only show dcs commands and responses. > MIPI DSI spec, a DCS read is answered with DCS short/long response, and > a generic read is answered with generic short/long response. > I don't have that spec, but if the command type dictates the response type, then I agree that having separate a generic_read command is better. > If your HW doesn't work like that, it's against the spec. If a broken > chip like that was widely used, I guess we would have to add a hack for > that, but until such day I want to keep the driver working as the spec > says. > My guess is that it would also respond to a generic read, but I have not tried this since I did not know there were multiple read commands.
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c index c16b933..6975645 100644 --- a/drivers/video/omap2/dss/dsi.c +++ b/drivers/video/omap2/dss/dsi.c @@ -206,6 +206,7 @@ struct dsi_reg { u16 idx; }; #define DSI_DT_DCS_LONG_WRITE 0x39 #define DSI_DT_RX_ACK_WITH_ERR 0x02 +#define DSI_DT_RX_LONG_READ 0x1a #define DSI_DT_RX_DCS_LONG_READ 0x1c #define DSI_DT_RX_SHORT_READ_1 0x21 #define DSI_DT_RX_SHORT_READ_2 0x22 @@ -2943,6 +2944,10 @@ static u16 dsi_vc_flush_receive_data(struct platform_device *dsidev, } else if (dt == DSI_DT_RX_SHORT_READ_2) { DSSERR("\tDCS short response, 2 byte: %#x\n", FLD_GET(val, 23, 8)); + } else if (dt == DSI_DT_RX_LONG_READ) { + DSSERR("\tlong response, len %d\n", + FLD_GET(val, 23, 8)); + dsi_vc_flush_long_data(dsidev, channel); } else if (dt == DSI_DT_RX_DCS_LONG_READ) { DSSERR("\tDCS long response, len %d\n", FLD_GET(val, 23, 8)); @@ -3287,7 +3292,7 @@ int dsi_vc_dcs_read(struct omap_dss_device *dssdev, int channel, u8 dcs_cmd, buf[1] = (data >> 8) & 0xff; return 2; - } else if (dt == DSI_DT_RX_DCS_LONG_READ) { + } else if (dt == DSI_DT_RX_DCS_LONG_READ || dt == DSI_DT_RX_LONG_READ) { int w; int len = FLD_GET(val, 23, 8); if (dsi->debug_read)
Change-Id: I18168c887e1384c07dc033a1ffc57abdacb26073 Signed-off-by: Arve Hjønnevåg <arve@android.com> --- drivers/video/omap2/dss/dsi.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)