| Message ID | 1433943748-15849-2-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10 June 2015 at 14:42, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> 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@math.uni-bielefeld.de> > --- > 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. 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. The nouveau mesa drivers do a similar thing - see PUSH_SPACE(). -Emil -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Emil, Emil Velikov wrote: > On 10 June 2015 at 14:42, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> 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@math.uni-bielefeld.de> >> --- >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); - g2d_flush(ctx); - - return 0; + return g2d_flush(ctx); }
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@math.uni-bielefeld.de> --- exynos/exynos_fimg2d.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)