Re: [PATCH 4/9] drm/tegra: dc: Implement hardware cursor on Tegra186 and later

Dmitry Osipenko
2021-03-23T19:06:14Z

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.