On 1/14/19 4:43 PM, Noralf Trønnes wrote:
Den 14.01.2019 23.33, skrev David Lechner:
On 1/14/19 3:50 PM, David Lechner wrote:
On 1/14/19 10:13 AM, Noralf Trønnes wrote:
I see that you have this call chain:
st7586_pipe_enable() -> mipi_dbi_enable_flush() -> mipi_dbi_fb_dirty().
That doesn't look safe. The st7586 driver allocates a tx_buf with size:
size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;
whereas mipi_dbi_enable_flush() will trigger a copy to tx_buf with len:
fb->width * fb->height * 2
It looks like you're writing zeroes way past the end of the buffer.
Noralf.
Thanks! That does indeed seem to be the problem. I'll put together
a patch to fix this. I'm thinking it will be easier to make the
fix before applying this series so that it will be easier to
backport.
Well, now that I am looking into it more, I see that the problem
was not preexisting. This patch ("drm/tinydrm: Use damage helper
for dirtyfb") also changes mipi_dbi_enable_flush() from calling
tdev->fb_dirty() to mipi_dbi_fb_dirty().
Perhaps we should not be dropping tdev->fb_dirty()?
Ah, now it makes sense. I thought it strange that you hadn't experienced
the corruption before.
How about this change?
diff --git a/drivers/gpu/drm/tinydrm/st7586.c
b/drivers/gpu/drm/tinydrm/st7586.c
index d39417b74e8b..01a8077954b3 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -177,6 +177,13 @@ static void st7586_pipe_enable(struct
drm_simple_display_pipe *pipe,
{
struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect rect = {
+ .x1 = 0,
+ .x2 = fb->width,
+ .y1 = 0,
+ .y2 = fb->height,
+ };
int ret;
u8 addr_mode;
@@ -233,9 +240,10 @@ static void st7586_pipe_enable(struct
drm_simple_display_pipe *pipe,
msleep(100);
- mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+ mipi->enabled = true;
+ st7586_fb_dirty(fb, &rect);
- mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+ mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
}
static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
That looks good. I also noticed that ili9225 has a custom dirty
function, so it will need a similar change as well.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
