Den 15.01.2019 00.03, skrev David Lechner:
> 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.
>
Thanks for tracking this down, I'll spin a new version tomorrow.
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
