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