Hello Emil, Emil Velikov wrote: > On 10 June 2015 at 14:42, Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> wrote: >> Even if flushing the command buffer doesn't succeed, the >> G2D calls would still return zero. Fix this by just passing >> the flush return code. >> >> Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> >> --- >> exynos/exynos_fimg2d.c | 20 +++++--------------- >> 1 file changed, 5 insertions(+), 15 deletions(-) >> >> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c >> index 86ae898..5ea42e6 100644 >> --- a/exynos/exynos_fimg2d.c >> +++ b/exynos/exynos_fimg2d.c >> @@ -330,9 +330,7 @@ g2d_solid_fill(struct g2d_context *ctx, struct g2d_image *img, >> bitblt.data.fast_solid_color_fill_en = 1; >> g2d_add_cmd(ctx, BITBLT_COMMAND_REG, bitblt.val); >> >> - g2d_flush(ctx); >> - >> - return 0; >> + return g2d_flush(ctx); >> } >> >> /** >> @@ -415,9 +413,7 @@ g2d_copy(struct g2d_context *ctx, struct g2d_image *src, >> rop4.data.unmasked_rop3 = G2D_ROP3_SRC; >> g2d_add_cmd(ctx, ROP4_REG, rop4.val); >> >> - g2d_flush(ctx); >> - >> - return 0; >> + return g2d_flush(ctx); >> } >> >> /** >> @@ -527,9 +523,7 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct g2d_image *src, >> pt.data.y = dst_y + dst_h; >> g2d_add_cmd(ctx, DST_RIGHT_BOTTOM_REG, pt.val); >> >> - g2d_flush(ctx); >> - >> - return 0; >> + return g2d_flush(ctx); >> } >> >> /** >> @@ -636,9 +630,7 @@ g2d_blend(struct g2d_context *ctx, struct g2d_image *src, >> pt.data.y = dst_y + h; >> g2d_add_cmd(ctx, DST_RIGHT_BOTTOM_REG, pt.val); >> >> - g2d_flush(ctx); >> - >> - return 0; >> + return g2d_flush(ctx); >> } >> >> /** >> @@ -766,7 +758,5 @@ g2d_scale_and_blend(struct g2d_context *ctx, struct g2d_image *src, >> pt.data.y = dst_y + dst_h; >> g2d_add_cmd(ctx, DST_RIGHT_BOTTOM_REG, pt.val); >> > Strictly speaking g2d_add_cmd() can fail, and all the functions that > build upon it. In the latter case most ignore the return value which > is a bit bad. That plus the fact that these are part of the public API > makes things easier to misuse. I'm totally aware of that. In fact I've already rewritten the error checking logic. But that would be for a later series. I prefer to do this in small steps, in particular because I see the tendency that nobody from Samsung reviews the larger patches anyway. Or any patches at all. And yes, I'm voicing my frustration here... > One way to avoid all this is to implement an internal function that > does the size checks ahead of time, and drop them from g2d_add_cmd(), > apart from this patch. I'm doing something different, which however results in more or less the same thing. > > The nouveau mesa drivers do a similar thing - see PUSH_SPACE(). > > -Emil > With best wishes, Tobias _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
