* [PATCH 2/3] drm/panel: use mipi_dsi_dcs_write_buffer where possible
2020-05-05 16:03 [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
@ 2020-05-05 16:03 ` Emil Velikov
2020-05-11 11:28 ` Emil Velikov
2020-05-05 16:03 ` [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline Emil Velikov
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Emil Velikov @ 2020-05-05 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: Heiko Stuebner, Thierry Reding, emil.l.velikov, Sam Ravnborg
From: Emil Velikov <emil.velikov@collabora.com>
A few of the new panels create a local macro wrapping around
mipi_dsi_dcs_write. At the same time, they don't really care about the
command/payload split.
mipi_dsi_dcs_write does a kmalloc/memcpy/kfree for payload > 7 bytes.
kmalloc - avoid that all together by using the _buffer function.
Aside:
panel-xinpeng-xpp055c272.c calls its wrapper "generic" although it
should be "dcs". But that for another day/patch.
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
drivers/gpu/drm/panel/panel-elida-kd35t133.c | 4 ++--
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 4 ++--
drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-elida-kd35t133.c b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
index 711ded453c44..00e3d67af812 100644
--- a/drivers/gpu/drm/panel/panel-elida-kd35t133.c
+++ b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
@@ -52,9 +52,9 @@ static inline struct kd35t133 *panel_to_kd35t133(struct drm_panel *panel)
}
#define dsi_dcs_write_seq(dsi, cmd, seq...) do { \
- static const u8 d[] = { seq }; \
+ static const u8 b[] = { cmd, seq }; \
int ret; \
- ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
+ ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
if (ret < 0) \
return ret; \
} while (0)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
index 5a7a31c8513e..eaa9da3ebbea 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
@@ -246,9 +246,9 @@ struct ltk050h3146w *panel_to_ltk050h3146w(struct drm_panel *panel)
}
#define dsi_dcs_write_seq(dsi, cmd, seq...) do { \
- static const u8 d[] = { seq }; \
+ static const u8 b[] = { cmd, seq }; \
int ret; \
- ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
+ ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
if (ret < 0) \
return ret; \
} while (0)
diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
index 1645aceab597..9e07d7e86807 100644
--- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
+++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
@@ -62,9 +62,9 @@ static inline struct xpp055c272 *panel_to_xpp055c272(struct drm_panel *panel)
}
#define dsi_generic_write_seq(dsi, cmd, seq...) do { \
- static const u8 d[] = { seq }; \
+ static const u8 b[] = { cmd, seq }; \
int ret; \
- ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
+ ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
if (ret < 0) \
return ret; \
} while (0)
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/3] drm/panel: use mipi_dsi_dcs_write_buffer where possible
2020-05-05 16:03 ` [PATCH 2/3] drm/panel: use mipi_dsi_dcs_write_buffer where possible Emil Velikov
@ 2020-05-11 11:28 ` Emil Velikov
2020-06-29 7:46 ` Sam Ravnborg
0 siblings, 1 reply; 15+ messages in thread
From: Emil Velikov @ 2020-05-11 11:28 UTC (permalink / raw)
To: ML dri-devel; +Cc: Heiko Stuebner, Thierry Reding, Sam Ravnborg
On Tue, 5 May 2020 at 17:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> From: Emil Velikov <emil.velikov@collabora.com>
>
> A few of the new panels create a local macro wrapping around
> mipi_dsi_dcs_write. At the same time, they don't really care about the
> command/payload split.
>
> mipi_dsi_dcs_write does a kmalloc/memcpy/kfree for payload > 7 bytes.
> kmalloc - avoid that all together by using the _buffer function.
>
Seems like I've left an extra word here - will fix in v2, alongside
any review feedback.
s/kmalloc - avoid/Avoid/
> Aside:
> panel-xinpeng-xpp055c272.c calls its wrapper "generic" although it
> should be "dcs". But that for another day/patch.
>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> drivers/gpu/drm/panel/panel-elida-kd35t133.c | 4 ++--
> drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 4 ++--
> drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-elida-kd35t133.c b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> index 711ded453c44..00e3d67af812 100644
> --- a/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> +++ b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> @@ -52,9 +52,9 @@ static inline struct kd35t133 *panel_to_kd35t133(struct drm_panel *panel)
> }
>
> #define dsi_dcs_write_seq(dsi, cmd, seq...) do { \
> - static const u8 d[] = { seq }; \
> + static const u8 b[] = { cmd, seq }; \
> int ret; \
> - ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
> if (ret < 0) \
> return ret; \
> } while (0)
> diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
> index 5a7a31c8513e..eaa9da3ebbea 100644
> --- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
> +++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
> @@ -246,9 +246,9 @@ struct ltk050h3146w *panel_to_ltk050h3146w(struct drm_panel *panel)
> }
>
> #define dsi_dcs_write_seq(dsi, cmd, seq...) do { \
> - static const u8 d[] = { seq }; \
> + static const u8 b[] = { cmd, seq }; \
> int ret; \
> - ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
> if (ret < 0) \
> return ret; \
> } while (0)
> diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> index 1645aceab597..9e07d7e86807 100644
> --- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> +++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> @@ -62,9 +62,9 @@ static inline struct xpp055c272 *panel_to_xpp055c272(struct drm_panel *panel)
> }
>
> #define dsi_generic_write_seq(dsi, cmd, seq...) do { \
> - static const u8 d[] = { seq }; \
> + static const u8 b[] = { cmd, seq }; \
> int ret; \
> - ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
> if (ret < 0) \
> return ret; \
> } while (0)
> --
> 2.25.1
>
Humble poke?
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/3] drm/panel: use mipi_dsi_dcs_write_buffer where possible
2020-05-11 11:28 ` Emil Velikov
@ 2020-06-29 7:46 ` Sam Ravnborg
0 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2020-06-29 7:46 UTC (permalink / raw)
To: Emil Velikov; +Cc: Heiko Stuebner, Thierry Reding, ML dri-devel
On Mon, May 11, 2020 at 12:28:40PM +0100, Emil Velikov wrote:
> On Tue, 5 May 2020 at 17:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > A few of the new panels create a local macro wrapping around
> > mipi_dsi_dcs_write. At the same time, they don't really care about the
> > command/payload split.
> >
> > mipi_dsi_dcs_write does a kmalloc/memcpy/kfree for payload > 7 bytes.
> > kmalloc - avoid that all together by using the _buffer function.
> >
> Seems like I've left an extra word here - will fix in v2, alongside
> any review feedback.
> s/kmalloc - avoid/Avoid/
>
> > Aside:
> > panel-xinpeng-xpp055c272.c calls its wrapper "generic" although it
> > should be "dcs". But that for another day/patch.
> >
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Fixed up changelog when applied to drm-misc-next.
Sam
> > ---
> > drivers/gpu/drm/panel/panel-elida-kd35t133.c | 4 ++--
> > drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 4 ++--
> > drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 4 ++--
> > 3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-elida-kd35t133.c b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> > index 711ded453c44..00e3d67af812 100644
> > --- a/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> > +++ b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> > @@ -52,9 +52,9 @@ static inline struct kd35t133 *panel_to_kd35t133(struct drm_panel *panel)
> > }
> >
> > #define dsi_dcs_write_seq(dsi, cmd, seq...) do { \
> > - static const u8 d[] = { seq }; \
> > + static const u8 b[] = { cmd, seq }; \
> > int ret; \
> > - ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> > + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
> > if (ret < 0) \
> > return ret; \
> > } while (0)
> > diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
> > index 5a7a31c8513e..eaa9da3ebbea 100644
> > --- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
> > +++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
> > @@ -246,9 +246,9 @@ struct ltk050h3146w *panel_to_ltk050h3146w(struct drm_panel *panel)
> > }
> >
> > #define dsi_dcs_write_seq(dsi, cmd, seq...) do { \
> > - static const u8 d[] = { seq }; \
> > + static const u8 b[] = { cmd, seq }; \
> > int ret; \
> > - ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> > + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
> > if (ret < 0) \
> > return ret; \
> > } while (0)
> > diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> > index 1645aceab597..9e07d7e86807 100644
> > --- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> > +++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> > @@ -62,9 +62,9 @@ static inline struct xpp055c272 *panel_to_xpp055c272(struct drm_panel *panel)
> > }
> >
> > #define dsi_generic_write_seq(dsi, cmd, seq...) do { \
> > - static const u8 d[] = { seq }; \
> > + static const u8 b[] = { cmd, seq }; \
> > int ret; \
> > - ret = mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> > + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \
> > if (ret < 0) \
> > return ret; \
> > } while (0)
> > --
> > 2.25.1
> >
>
> Humble poke?
>
> -Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-05 16:03 [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
2020-05-05 16:03 ` [PATCH 2/3] drm/panel: use mipi_dsi_dcs_write_buffer where possible Emil Velikov
@ 2020-05-05 16:03 ` Emil Velikov
2020-05-07 12:29 ` Vinay Simha B N
` (2 more replies)
2020-05-28 16:47 ` [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
` (2 subsequent siblings)
4 siblings, 3 replies; 15+ messages in thread
From: Emil Velikov @ 2020-05-05 16:03 UTC (permalink / raw)
To: dri-devel
Cc: Robert Chiras, Vinay Simha BN, Jani Nikula, Thierry Reding,
emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
using the generic write. This does not look right.
Perhaps some platforms don't distinguish between the two writers?
Cc: Robert Chiras <robert.chiras@nxp.com>
Cc: Vinay Simha BN <simhavcs@gmail.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Thierry Reding <treding@nvidia.com>
Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Robert, can you please test this against the only user - the Raydium
RM67191 panel driver that you introduced.
Thanks
Vinay, can you confirm if this is a genuine typo or there's something
really subtle happening.
---
drivers/gpu/drm/drm_mipi_dsi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index b96d5b4629d7..07102d8da58f 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -1082,11 +1082,11 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
*/
int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline)
{
- u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, scanline >> 8,
- scanline & 0xff };
+ u8 payload[2] = { scanline >> 8, scanline & 0xff };
ssize_t err;
- err = mipi_dsi_generic_write(dsi, payload, sizeof(payload));
+ err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, payload,
+ sizeof(payload));
if (err < 0)
return err;
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-05 16:03 ` [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline Emil Velikov
@ 2020-05-07 12:29 ` Vinay Simha B N
2020-05-07 16:18 ` Emil Velikov
2020-06-05 17:36 ` Thierry Reding
2020-06-29 7:47 ` Sam Ravnborg
2 siblings, 1 reply; 15+ messages in thread
From: Vinay Simha B N @ 2020-05-07 12:29 UTC (permalink / raw)
To: Emil Velikov; +Cc: Jani Nikula, Robert Chiras, Thierry Reding, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2065 bytes --]
Emil,
Reply inline
On Tue, 5 May 2020 at 9:35 PM, Emil Velikov <emil.l.velikov@gmail.com>
wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
> using the generic write. This does not look right.
>
> Perhaps some platforms don't distinguish between the two writers?
>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Robert, can you please test this against the only user - the Raydium
> RM67191 panel driver that you introduced.
>
> Thanks
>
> Vinay, can you confirm if this is a genuine typo or there's something
> really subtle happening.
this has been tested on nexus 7 with jdi panel. I did not understand what
is the typo here?
We need to use DC’s write instead of generic write?
>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c
> b/drivers/gpu/drm/drm_mipi_dsi.c
> index b96d5b4629d7..07102d8da58f 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -1082,11 +1082,11 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
> */
> int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16
> scanline)
> {
> - u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, scanline >> 8,
> - scanline & 0xff };
> + u8 payload[2] = { scanline >> 8, scanline & 0xff };
> ssize_t err;
>
> - err = mipi_dsi_generic_write(dsi, payload, sizeof(payload));
> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, payload,
> + sizeof(payload));
> if (err < 0)
> return err;
>
> --
> 2.25.1
>
> --
regards,
vinaysimha
[-- Attachment #1.2: Type: text/html, Size: 3221 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-07 12:29 ` Vinay Simha B N
@ 2020-05-07 16:18 ` Emil Velikov
2020-05-13 9:44 ` Emil Velikov
0 siblings, 1 reply; 15+ messages in thread
From: Emil Velikov @ 2020-05-07 16:18 UTC (permalink / raw)
To: Vinay Simha B N; +Cc: Jani Nikula, Robert Chiras, Thierry Reding, ML dri-devel
On Thu, 7 May 2020 at 13:29, Vinay Simha B N <simhavcs@gmail.com> wrote:
>
> Emil,
>
> Reply inline
>
> On Tue, 5 May 2020 at 9:35 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>
>> From: Emil Velikov <emil.velikov@collabora.com>
>>
>> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
>> using the generic write. This does not look right.
>>
>> Perhaps some platforms don't distinguish between the two writers?
>>
>> Cc: Robert Chiras <robert.chiras@nxp.com>
>> Cc: Vinay Simha BN <simhavcs@gmail.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Thierry Reding <treding@nvidia.com>
>> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>> ---
>> Robert, can you please test this against the only user - the Raydium
>> RM67191 panel driver that you introduced.
>>
>> Thanks
>>
>> Vinay, can you confirm if this is a genuine typo or there's something
>> really subtle happening.
>
> this has been tested on nexus 7 with jdi panel.
The jdi panel (JDI LT070ME05000 I believe) does not use the function, hmm.
Looking through the ML archive - the call in the first 4 revisions of the patch.
Then with v5 it has magically disappeared alongside mipi_dsi_dcs_set_tear_on().
No comment explaining why though - does the driver work w/o both of those?
> I did not understand what is the typo here?
> We need to use DC’s write instead of generic write?
I believe the clue is in the command name - MIPI_DSI_DCS. I was going
to double-check with the spec although it's members only :-\
Based on the usage in DRM, all DCS commands are issued via
mipi_dsi_dcs_{read,write}
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-07 16:18 ` Emil Velikov
@ 2020-05-13 9:44 ` Emil Velikov
2020-06-07 16:17 ` Vinay Simha B N
0 siblings, 1 reply; 15+ messages in thread
From: Emil Velikov @ 2020-05-13 9:44 UTC (permalink / raw)
To: Vinay Simha B N; +Cc: Jani Nikula, Robert Chiras, Thierry Reding, ML dri-devel
Hi Vinay,
On Thu, 7 May 2020 at 17:18, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Thu, 7 May 2020 at 13:29, Vinay Simha B N <simhavcs@gmail.com> wrote:
> >
> > Emil,
> >
> > Reply inline
> >
> > On Tue, 5 May 2020 at 9:35 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >>
> >> From: Emil Velikov <emil.velikov@collabora.com>
> >>
> >> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
> >> using the generic write. This does not look right.
> >>
> >> Perhaps some platforms don't distinguish between the two writers?
> >>
> >> Cc: Robert Chiras <robert.chiras@nxp.com>
> >> Cc: Vinay Simha BN <simhavcs@gmail.com>
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> Cc: Thierry Reding <treding@nvidia.com>
> >> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
> >> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> >> ---
> >> Robert, can you please test this against the only user - the Raydium
> >> RM67191 panel driver that you introduced.
> >>
> >> Thanks
> >>
> >> Vinay, can you confirm if this is a genuine typo or there's something
> >> really subtle happening.
> >
> > this has been tested on nexus 7 with jdi panel.
> The jdi panel (JDI LT070ME05000 I believe) does not use the function, hmm.
>
> Looking through the ML archive - the call in the first 4 revisions of the patch.
> Then with v5 it has magically disappeared alongside mipi_dsi_dcs_set_tear_on().
>
> No comment explaining why though - does the driver work w/o both of those?
>
Any ideas, does the driver work in today's state?
> > I did not understand what is the typo here?
> > We need to use DC’s write instead of generic write?
>
> I believe the clue is in the command name - MIPI_DSI_DCS. I was going
> to double-check with the spec although it's members only :-\
> Based on the usage in DRM, all DCS commands are issued via
> mipi_dsi_dcs_{read,write}
>
Do you agree with the rationale? Alternatively, do you have a
reference to the Android tree where the generic write is used.
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-13 9:44 ` Emil Velikov
@ 2020-06-07 16:17 ` Vinay Simha B N
0 siblings, 0 replies; 15+ messages in thread
From: Vinay Simha B N @ 2020-06-07 16:17 UTC (permalink / raw)
To: Emil Velikov; +Cc: Jani Nikula, Robert Chiras, Thierry Reding, ML dri-devel
hi emil,
On Wed, May 13, 2020 at 3:17 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> Hi Vinay,
>
> On Thu, 7 May 2020 at 17:18, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > On Thu, 7 May 2020 at 13:29, Vinay Simha B N <simhavcs@gmail.com> wrote:
> > >
> > > Emil,
> > >
> > > Reply inline
> > >
> > > On Tue, 5 May 2020 at 9:35 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > >>
> > >> From: Emil Velikov <emil.velikov@collabora.com>
> > >>
> > >> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
> > >> using the generic write. This does not look right.
> > >>
> > >> Perhaps some platforms don't distinguish between the two writers?
> > >>
> > >> Cc: Robert Chiras <robert.chiras@nxp.com>
> > >> Cc: Vinay Simha BN <simhavcs@gmail.com>
> > >> Cc: Jani Nikula <jani.nikula@intel.com>
> > >> Cc: Thierry Reding <treding@nvidia.com>
> > >> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
> > >> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > >> ---
> > >> Robert, can you please test this against the only user - the Raydium
> > >> RM67191 panel driver that you introduced.
> > >>
> > >> Thanks
> > >>
> > >> Vinay, can you confirm if this is a genuine typo or there's something
> > >> really subtle happening.
> > >
> > > this has been tested on nexus 7 with jdi panel.
> > The jdi panel (JDI LT070ME05000 I believe) does not use the function, hmm.
> >
> > Looking through the ML archive - the call in the first 4 revisions of the patch.
> > Then with v5 it has magically disappeared alongside mipi_dsi_dcs_set_tear_on().
> >
> > No comment explaining why though - does the driver work w/o both of those?
> >
> Any ideas, does the driver work in today's state?
Initially I had used cmd mode, later modified to video mode panel,
since to control the panel in cmd mode is not available for mdp4.
so mipi_dsi_dcs_set_tear_on was not used.
>
> > > I did not understand what is the typo here?
> > > We need to use DC’s write instead of generic write?
> >
> > I believe the clue is in the command name - MIPI_DSI_DCS. I was going
> > to double-check with the spec although it's members only :-\
> > Based on the usage in DRM, all DCS commands are issued via
> > mipi_dsi_dcs_{read,write}
> >
> Do you agree with the rationale? Alternatively, do you have a
> reference to the Android tree where the generic write is used.
>
default android nexus 7 kernel
https://github.com/vinaysimhabn/msm/commits/nexus7-msm-flo-3.4-lollipop-release
> Thanks
> Emil
--
regards,
vinaysimha
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-05 16:03 ` [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline Emil Velikov
2020-05-07 12:29 ` Vinay Simha B N
@ 2020-06-05 17:36 ` Thierry Reding
2020-06-29 7:47 ` Sam Ravnborg
2 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2020-06-05 17:36 UTC (permalink / raw)
To: Emil Velikov; +Cc: Robert Chiras, Vinay Simha BN, Jani Nikula, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2556 bytes --]
On Tue, May 05, 2020 at 05:03:29PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
> using the generic write. This does not look right.
>
> Perhaps some platforms don't distinguish between the two writers?
I don't think platforms usually care about this level of detail. The
Tegra driver for example doesn't really look at the packet type and just
packets the data in the standard DSI format and then sends it off on the
bus. It does inspect some fields of the packet, but none that are
related to the packet type, I think.
So it's possible that the panel will accept the packet irrespective of
type and handle it correctly. I can imagine that the decoding logic in
these panels might be rather primitive, so perhaps it's not very strict
as to what exactly the type is as long as it can do something with the
data.
In any case, it does make sense to send DCS commands using the DCS type,
so I'd say let's merge this and see if somebody complains:
Reviewed-by: Thierry Reding <treding@nvidia.com>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Robert, can you please test this against the only user - the Raydium
> RM67191 panel driver that you introduced.
>
> Thanks
>
> Vinay, can you confirm if this is a genuine typo or there's something
> really subtle happening.
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index b96d5b4629d7..07102d8da58f 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -1082,11 +1082,11 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
> */
> int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline)
> {
> - u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, scanline >> 8,
> - scanline & 0xff };
> + u8 payload[2] = { scanline >> 8, scanline & 0xff };
> ssize_t err;
>
> - err = mipi_dsi_generic_write(dsi, payload, sizeof(payload));
> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, payload,
> + sizeof(payload));
> if (err < 0)
> return err;
>
> --
> 2.25.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline
2020-05-05 16:03 ` [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline Emil Velikov
2020-05-07 12:29 ` Vinay Simha B N
2020-06-05 17:36 ` Thierry Reding
@ 2020-06-29 7:47 ` Sam Ravnborg
2 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2020-06-29 7:47 UTC (permalink / raw)
To: Emil Velikov
Cc: Robert Chiras, Vinay Simha BN, Jani Nikula, Thierry Reding, dri-devel
On Tue, May 05, 2020 at 05:03:29PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
> using the generic write. This does not look right.
>
> Perhaps some platforms don't distinguish between the two writers?
>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Robert, can you please test this against the only user - the Raydium
> RM67191 panel driver that you introduced.
>
> Thanks
>
> Vinay, can you confirm if this is a genuine typo or there's something
> really subtle happening.
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
The discussion did not reveal anything new. Followed the advice of
Thierry and applied.
Sam
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index b96d5b4629d7..07102d8da58f 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -1082,11 +1082,11 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
> */
> int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline)
> {
> - u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, scanline >> 8,
> - scanline & 0xff };
> + u8 payload[2] = { scanline >> 8, scanline & 0xff };
> ssize_t err;
>
> - err = mipi_dsi_generic_write(dsi, payload, sizeof(payload));
> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, payload,
> + sizeof(payload));
> if (err < 0)
> return err;
>
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
2020-05-05 16:03 [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
2020-05-05 16:03 ` [PATCH 2/3] drm/panel: use mipi_dsi_dcs_write_buffer where possible Emil Velikov
2020-05-05 16:03 ` [PATCH 3/3] drm/mipi: use dcs write for mipi_dsi_dcs_set_tear_scanline Emil Velikov
@ 2020-05-28 16:47 ` Emil Velikov
2020-06-05 17:37 ` Thierry Reding
2020-06-05 17:26 ` Thierry Reding
2020-06-29 7:46 ` Sam Ravnborg
4 siblings, 1 reply; 15+ messages in thread
From: Emil Velikov @ 2020-05-28 16:47 UTC (permalink / raw)
To: ML dri-devel; +Cc: Jani Nikula, Thierry Reding
On Tue, 5 May 2020 at 17:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> Currently the function heap allocates when we have any payload. Where in
> many case the payload is 1 byte - ouch.
>
> From casual observation, vast majority of the payloads are smaller than
> 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and
> kfree dance.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 55531895dde6..b96d5b4629d7 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> {
> ssize_t err;
> size_t size;
> + u8 stack_tx[8];
> u8 *tx;
>
> - if (len > 0) {
> - size = 1 + len;
> -
> + size = 1 + len;
> + if (len > ARRAY_SIZE(stack_tx) - 1) {
> tx = kmalloc(size, GFP_KERNEL);
> if (!tx)
> return -ENOMEM;
> -
> - /* concatenate the DCS command byte and the payload */
> - tx[0] = cmd;
> - memcpy(&tx[1], data, len);
> } else {
> - tx = &cmd;
> - size = 1;
> + tx = stack_tx;
> }
>
> + /* concatenate the DCS command byte and the payload */
> + tx[0] = cmd;
> + if (data)
> + memcpy(&tx[1], data, len);
> +
> err = mipi_dsi_dcs_write_buffer(dsi, tx, size);
>
> - if (len > 0)
> + if (tx != stack_tx)
> kfree(tx);
>
> return err;
> --
Thierry, others - humble ping.
Can you check through the series?
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
2020-05-28 16:47 ` [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
@ 2020-06-05 17:37 ` Thierry Reding
0 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2020-06-05 17:37 UTC (permalink / raw)
To: Emil Velikov; +Cc: Jani Nikula, ML dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2277 bytes --]
On Thu, May 28, 2020 at 05:47:41PM +0100, Emil Velikov wrote:
> On Tue, 5 May 2020 at 17:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > Currently the function heap allocates when we have any payload. Where in
> > many case the payload is 1 byte - ouch.
> >
> > From casual observation, vast majority of the payloads are smaller than
> > 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and
> > kfree dance.
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> > drivers/gpu/drm/drm_mipi_dsi.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> > index 55531895dde6..b96d5b4629d7 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> > {
> > ssize_t err;
> > size_t size;
> > + u8 stack_tx[8];
> > u8 *tx;
> >
> > - if (len > 0) {
> > - size = 1 + len;
> > -
> > + size = 1 + len;
> > + if (len > ARRAY_SIZE(stack_tx) - 1) {
> > tx = kmalloc(size, GFP_KERNEL);
> > if (!tx)
> > return -ENOMEM;
> > -
> > - /* concatenate the DCS command byte and the payload */
> > - tx[0] = cmd;
> > - memcpy(&tx[1], data, len);
> > } else {
> > - tx = &cmd;
> > - size = 1;
> > + tx = stack_tx;
> > }
> >
> > + /* concatenate the DCS command byte and the payload */
> > + tx[0] = cmd;
> > + if (data)
> > + memcpy(&tx[1], data, len);
> > +
> > err = mipi_dsi_dcs_write_buffer(dsi, tx, size);
> >
> > - if (len > 0)
> > + if (tx != stack_tx)
> > kfree(tx);
> >
> > return err;
> > --
>
> Thierry, others - humble ping.
> Can you check through the series?
I don't see patch 2 of this series anywhere? Did it fall victim to some
filter?
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
2020-05-05 16:03 [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
` (2 preceding siblings ...)
2020-05-28 16:47 ` [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
@ 2020-06-05 17:26 ` Thierry Reding
2020-06-29 7:46 ` Sam Ravnborg
4 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2020-06-05 17:26 UTC (permalink / raw)
To: Emil Velikov; +Cc: Jani Nikula, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1258 bytes --]
On Tue, May 05, 2020 at 05:03:27PM +0100, Emil Velikov wrote:
> Currently the function heap allocates when we have any payload. Where in
> many case the payload is 1 byte - ouch.
>
> From casual observation, vast majority of the payloads are smaller than
> 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and
> kfree dance.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 55531895dde6..b96d5b4629d7 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> {
> ssize_t err;
> size_t size;
> + u8 stack_tx[8];
> u8 *tx;
>
> - if (len > 0) {
> - size = 1 + len;
> -
> + size = 1 + len;
> + if (len > ARRAY_SIZE(stack_tx) - 1) {
I think it would be clearer to do:
if (size > ARRAY_SIZE(stack_tx))
but either way:
Reviewed-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
2020-05-05 16:03 [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write() Emil Velikov
` (3 preceding siblings ...)
2020-06-05 17:26 ` Thierry Reding
@ 2020-06-29 7:46 ` Sam Ravnborg
4 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2020-06-29 7:46 UTC (permalink / raw)
To: Emil Velikov; +Cc: Jani Nikula, Thierry Reding, dri-devel
On Tue, May 05, 2020 at 05:03:27PM +0100, Emil Velikov wrote:
> Currently the function heap allocates when we have any payload. Where in
> many case the payload is 1 byte - ouch.
>
> >From casual observation, vast majority of the payloads are smaller than
> 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and
> kfree dance.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Applied to drm-misc-next. Sorry for taking so long.
Sam
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 55531895dde6..b96d5b4629d7 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> {
> ssize_t err;
> size_t size;
> + u8 stack_tx[8];
> u8 *tx;
>
> - if (len > 0) {
> - size = 1 + len;
> -
> + size = 1 + len;
> + if (len > ARRAY_SIZE(stack_tx) - 1) {
> tx = kmalloc(size, GFP_KERNEL);
> if (!tx)
> return -ENOMEM;
> -
> - /* concatenate the DCS command byte and the payload */
> - tx[0] = cmd;
> - memcpy(&tx[1], data, len);
> } else {
> - tx = &cmd;
> - size = 1;
> + tx = stack_tx;
> }
>
> + /* concatenate the DCS command byte and the payload */
> + tx[0] = cmd;
> + if (data)
> + memcpy(&tx[1], data, len);
> +
> err = mipi_dsi_dcs_write_buffer(dsi, tx, size);
>
> - if (len > 0)
> + if (tx != stack_tx)
> kfree(tx);
>
> return err;
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread