[PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Arve Hjønnevåg]

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(-)

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)
-- 
1.7.3.1

--
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

Re: [PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Tomi Valkeinen]

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

Re: [PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Arve Hjønnevåg]

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)
>
>
>



-- 
Arve Hjønnevåg
--
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

Re: [PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Tomi Valkeinen]

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

Re: [PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Arve Hjønnevåg]

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.

-- 
Arve Hjønnevåg
--
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

Re: [PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Tomi Valkeinen]

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

Re: [PATCH] OMAP: DSS2: DSI: Support non-dcs long read

[Arve Hjønnevåg]

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.

-- 
Arve Hjønnevåg
--
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