Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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)


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux