| Message ID | 20200531131237.24781-1-realwakka@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Optimize compute_crc(), blend() | expand |
On Sun, 31 May 2020 at 14:12, Sidong Yang <realwakka@gmail.com> wrote: > > Optimize looping pixels in compute_crc() and blend(). Calculate > src_offset in start of looping horizontally and increase it. > It's better than calculating in every pixels. > When you say "optimize" have you observed any actual benefits of the patch - be that smaller binary, faster execution time, etc? If there are - mentioned them in the commit message. Otherwise, it doesn't optimise anything. A while back, I've suggested something similar [1] mostly for cosmetic purposes - doubt there's much benefits beyond that. HTH -Emil [1] https://patchwork.freedesktop.org/patch/365177/#comment_674314
Hi, First of all, thanks a lot for all your patch. And thanks Emil for your feedback. I have a suggestion here: Emil: Could you give me your Acked-by or maybe Reviewed-by for the writeback series? With that, I can finally apply the series. Sidong: Secondly, after applying the writeback series, I would suggest you to understand Emil's comments (he already provides the link) and prepare a new patch based on that. Before you submit your patch, I recommend you to test it with https://patchwork.freedesktop.org/series/68352/ and kms_flip. How about that? Best Regards On 05/31, Emil Velikov wrote: > On Sun, 31 May 2020 at 14:12, Sidong Yang <realwakka@gmail.com> wrote: > > > > Optimize looping pixels in compute_crc() and blend(). Calculate > > src_offset in start of looping horizontally and increase it. > > It's better than calculating in every pixels. > > > When you say "optimize" have you observed any actual benefits of the > patch - be that smaller binary, faster execution time, etc? > If there are - mentioned them in the commit message. Otherwise, it > doesn't optimise anything. > > A while back, I've suggested something similar [1] mostly for cosmetic > purposes - doubt there's much benefits beyond that. > > HTH > -Emil > [1] https://patchwork.freedesktop.org/patch/365177/#comment_674314
On Mon, 1 Jun 2020 at 01:25, Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> wrote: > > Hi, > > First of all, thanks a lot for all your patch. And thanks Emil for your > feedback. > > I have a suggestion here: > > Emil: > Could you give me your Acked-by or maybe Reviewed-by for the writeback > series? With that, I can finally apply the series. > Sure, once the issues highlighted are resolved. Just left you some more comprehensive feedback. -Emil P.S. Something something top posting sucks :-P
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c index 4af2f19480f4..9d2a765ca1fb 100644 --- a/drivers/gpu/drm/vkms/vkms_composer.c +++ b/drivers/gpu/drm/vkms/vkms_composer.c @@ -28,14 +28,14 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer) 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); + src_offset = composer->offset + (i * composer->pitch) + + (x_src * composer->cpp); + for (j = 0 ; j < w_src; ++j) { /* XRGB format ignores Alpha channel */ memset(vaddr_out + src_offset + 24, 0, 8); crc = crc32_le(crc, vaddr_out + src_offset, sizeof(u32)); + src_offset += composer->cpp; } } @@ -61,7 +61,7 @@ static void blend(void *vaddr_dst, void *vaddr_src, struct vkms_composer *dest_composer, struct vkms_composer *src_composer) { - int i, j, j_dst, i_dst; + int i, j, i_dst; int offset_src, offset_dst; int x_src = src_composer->src.x1 >> 16; @@ -73,21 +73,23 @@ static void blend(void *vaddr_dst, void *vaddr_src, int w_dst = drm_rect_width(&src_composer->dst); int y_limit = y_src + h_dst; - int x_limit = x_src + w_dst; - for (i = y_src, i_dst = y_dst; i < y_limit; ++i) { - for (j = x_src, j_dst = x_dst; j < x_limit; ++j) { - offset_dst = dest_composer->offset - + (i_dst * dest_composer->pitch) - + (j_dst++ * dest_composer->cpp); - offset_src = src_composer->offset - + (i * src_composer->pitch) - + (j * src_composer->cpp); + for (i = y_src, i_dst = y_dst; i < y_limit; ++i, ++i_dst) { + offset_dst = dest_composer->offset + + (i_dst * dest_composer->pitch) + + (x_dst * dest_composer->cpp); + offset_src = src_composer->offset + + (i * src_composer->pitch) + + (x_src * src_composer->cpp); + + for (j = 0; j < w_dst; ++j) { memcpy(vaddr_dst + offset_dst, vaddr_src + offset_src, sizeof(u32)); + + offset_dst += dest_composer->cpp; + offset_src += src_composer->cpp; } - i_dst++; } }
Optimize looping pixels in compute_crc() and blend(). Calculate src_offset in start of looping horizontally and increase it. It's better than calculating in every pixels. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Sidong Yang <realwakka@gmail.com> --- drivers/gpu/drm/vkms/vkms_composer.c | 32 +++++++++++++++------------- 1 file changed, 17 insertions(+), 15 deletions(-)