| Message ID | 20200825023142.2561220-3-rodrigosiqueiramelo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Introduces writeback support | expand |
On 08/24, Rodrigo Siqueira wrote: > The compute_crc() function is responsible for calculating the > framebuffer CRC value; due to the XRGB format, this function has to > ignore the alpha channel during the CRC computation. Therefore, > compute_crc() set zero to the alpha channel directly in the input > framebuffer, which is not a problem since this function receives a copy > of the original buffer. However, if we want to use this function in a > context without a buffer copy, it will change the initial value. This > patch makes compute_crc() calculate the CRC value without modifying the > input framebuffer. > > Change in V4 (Emil): > - Move bitmap_clear operation and comments to get_pixel function > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > --- > drivers/gpu/drm/vkms/vkms_composer.c | 38 ++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c > index 4d8bc04bb6ee..387b0690a64a 100644 > --- a/drivers/gpu/drm/vkms/vkms_composer.c > +++ b/drivers/gpu/drm/vkms/vkms_composer.c > @@ -9,33 +9,43 @@ > > #include "vkms_drv.h" > > +static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer, > + const struct vkms_composer *composer) > +{ > + u32 pixel; > + int src_offset = composer->offset + (y * composer->pitch) > + + (x * composer->cpp); > + > + pixel = *(u32 *)&buffer[src_offset]; > + /* XRGB format ignores Alpha channel */ > + bitmap_clear((void *)&pixel, 0, 8); I noticed a small problem in this bitmap_clear as I recently sent a patch changing it. The starting point for zeroing the alpha is not 0, but 24. As in the removed line below: - bitmap_clear(vaddr_out + src_offset, 24, 8); It affects the kms_cursor_crc/cursor-alpha-transparent subtest. > + > + return pixel; > +} > + > /** > * compute_crc - Compute CRC value on output frame > * > - * @vaddr_out: address to final framebuffer > + * @vaddr: address to final framebuffer > * @composer: framebuffer's metadata > * > * returns CRC value computed using crc32 on the visible portion of > * the final framebuffer at vaddr_out > */ > -static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer) > +static uint32_t compute_crc(const u8 *vaddr, > + const struct vkms_composer *composer) > { > - int i, j, src_offset; > + int x, y; > + u32 crc = 0, pixel = 0; > int x_src = composer->src.x1 >> 16; > int y_src = composer->src.y1 >> 16; > int h_src = drm_rect_height(&composer->src) >> 16; > int w_src = drm_rect_width(&composer->src) >> 16; > - u32 crc = 0; > - > - for (i = y_src; i < y_src + h_src; ++i) { > - for (j = x_src; j < x_src + w_src; ++j) { > - src_offset = composer->offset > - + (i * composer->pitch) > - + (j * composer->cpp); > - /* XRGB format ignores Alpha channel */ > - bitmap_clear(vaddr_out + src_offset, 24, 8); > - crc = crc32_le(crc, vaddr_out + src_offset, > - sizeof(u32)); > + > + for (y = y_src; y < y_src + h_src; ++y) { > + for (x = x_src; x < x_src + w_src; ++x) { > + pixel = get_pixel_from_buffer(x, y, vaddr, composer); > + crc = crc32_le(crc, (void *)&pixel, sizeof(u32)); > } > } > > -- > 2.28.0 >
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c index 4d8bc04bb6ee..387b0690a64a 100644 --- a/drivers/gpu/drm/vkms/vkms_composer.c +++ b/drivers/gpu/drm/vkms/vkms_composer.c @@ -9,33 +9,43 @@ #include "vkms_drv.h" +static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer, + const struct vkms_composer *composer) +{ + u32 pixel; + int src_offset = composer->offset + (y * composer->pitch) + + (x * composer->cpp); + + pixel = *(u32 *)&buffer[src_offset]; + /* XRGB format ignores Alpha channel */ + bitmap_clear((void *)&pixel, 0, 8); + + return pixel; +} + /** * compute_crc - Compute CRC value on output frame * - * @vaddr_out: address to final framebuffer + * @vaddr: address to final framebuffer * @composer: framebuffer's metadata * * returns CRC value computed using crc32 on the visible portion of * the final framebuffer at vaddr_out */ -static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer) +static uint32_t compute_crc(const u8 *vaddr, + const struct vkms_composer *composer) { - int i, j, src_offset; + int x, y; + u32 crc = 0, pixel = 0; int x_src = composer->src.x1 >> 16; int y_src = composer->src.y1 >> 16; int h_src = drm_rect_height(&composer->src) >> 16; int w_src = drm_rect_width(&composer->src) >> 16; - u32 crc = 0; - - for (i = y_src; i < y_src + h_src; ++i) { - for (j = x_src; j < x_src + w_src; ++j) { - src_offset = composer->offset - + (i * composer->pitch) - + (j * composer->cpp); - /* XRGB format ignores Alpha channel */ - bitmap_clear(vaddr_out + src_offset, 24, 8); - crc = crc32_le(crc, vaddr_out + src_offset, - sizeof(u32)); + + for (y = y_src; y < y_src + h_src; ++y) { + for (x = x_src; x < x_src + w_src; ++x) { + pixel = get_pixel_from_buffer(x, y, vaddr, composer); + crc = crc32_le(crc, (void *)&pixel, sizeof(u32)); } }
The compute_crc() function is responsible for calculating the framebuffer CRC value; due to the XRGB format, this function has to ignore the alpha channel during the CRC computation. Therefore, compute_crc() set zero to the alpha channel directly in the input framebuffer, which is not a problem since this function receives a copy of the original buffer. However, if we want to use this function in a context without a buffer copy, it will change the initial value. This patch makes compute_crc() calculate the CRC value without modifying the input framebuffer. Change in V4 (Emil): - Move bitmap_clear operation and comments to get_pixel function Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> --- drivers/gpu/drm/vkms/vkms_composer.c | 38 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 14 deletions(-)