On Tue, Mar 23, 2021 at 10:05:23PM +0300, Dmitry Osipenko wrote:
> 23.03.2021 21:24, Thierry Reding пишет:
> > On Tue, Mar 23, 2021 at 08:57:42PM +0300, Dmitry Osipenko wrote:
> >> 23.03.2021 18:54, Thierry Reding пишет:
> >>> @@ -920,15 +934,42 @@ static void tegra_cursor_atomic_update(struct drm_plane *plane,
> >>> value = tegra_dc_readl(dc, DC_DISP_BLEND_CURSOR_CONTROL);
> >>> value &= ~CURSOR_DST_BLEND_MASK;
> >>> value &= ~CURSOR_SRC_BLEND_MASK;
> >>> - value |= CURSOR_MODE_NORMAL;
> >>> +
> >>> + if (dc->soc->has_nvdisplay)
> >>> + value &= ~CURSOR_COMPOSITION_MODE_XOR;
> >>> + else
> >>> + value |= CURSOR_MODE_NORMAL;
> >>> +
> >>> value |= CURSOR_DST_BLEND_NEG_K1_TIMES_SRC;
> >>> value |= CURSOR_SRC_BLEND_K1_TIMES_SRC;
> >>> value |= CURSOR_ALPHA;
> >>> tegra_dc_writel(dc, value, DC_DISP_BLEND_CURSOR_CONTROL);
> >>>
> >>> + /* nvdisplay relies on software for clipping */
> >>> + if (dc->soc->has_nvdisplay) {
> >>
> >> But coordinates already should be clipped by
> >> drm_atomic_helper_check_plane_state().
> >
> > Yes, and the driver goes on to use the clipped coordinates later on in
> > this function.
>
> I see now what it does, looks okay.
>
> Minor nit: the i/j aren't very expressive names, something like sx/sy
> sw/sh should be a bit more appropriate naming.
>
> You could also make use of drm_rect_width/height helpers.
>
> But this doesn't deserve a v2 if there is nothing more important to improve.
As I was browsing through the drm_rect helpers, I also came across
drm_rect_fp_to_int(), which helps simplify this further. As a result I
was able to just get rid of most of these temporary variables in favor
of just the integer version of drm_rect.
Thanks for the hint.
Thierry
Attachment:
signature.asc
Description: PGP signature
