Hello Thomas,
On 9/30/22 12:11, Thomas Zimmermann wrote:
[...]
>>
>>>> +
>>>> + ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &damage);
>>>
>>> In simpledrm, we adjust the destination address with dst_clip like this:
>>>
>>> iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format,
>>> &dst_clip));
>>>
>>> How does this work in ssd130x? You never use dst_clip to adjust to the
>>> changed location. Won't you have out-of-bounds writes on the device?
>>>
>>
>> Right, in ssd130x what I do is:
>>
>> static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_map *vmap,
>> struct drm_rect *rect)
>> {
>> struct iosys_map dst;
>> ...
>> u8 *buf = NULL;
>> ...
>> buf = kcalloc(dst_pitch, drm_rect_height(rect), GFP_KERNEL);
>> ...
>> iosys_map_set_vaddr(&dst, buf);
>> drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
>> ...
>> ssd130x_update_rect(ssd130x, buf, rect);
>> }
>>
>> I understand that's correct too?
>
> From what I understand about ssd130x, blit_rect looks correct up to the
> call to update_rect. The values in the rect parameter are for the
> damage area of the plane. In update_rect, the destination coords x and y
> are also taken from rect. But they should come from dst_clip, which is
> the on-screen location. Does that make sense?
>
I believe you are correct. Then what I should do is to not pass the damage
area to ssd130x_fb_blit_rect() as the struct drm_rect argument but instead
the dst_clip as filled by drm_rect_intersect(). Does that sound correct ?
In other words, the following:
drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
drm_atomic_for_each_plane_damage(&iter, &damage) {
dst_clip = plane_state->dst;
if (!drm_rect_intersect(&dst_clip, &damage))
continue;
ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &dst_clip);
}
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
