Re: [PATCH v2 05/10] drm/tegra: dc: Implement hardware cursor on Tegra186 and later

[Dmitry Osipenko <digetx@gmail.com>]

26.03.2021 17:51, Thierry Reding пишет:
> From: Thierry Reding <treding@nvidia.com>
> 
> The hardware cursor on Tegra186 differs slightly from the implementation
> on older SoC generations. In particular the new implementation relies on
> software for clipping the cursor against the screen. Fortunately, atomic
> KMS already computes clipped coordinates for (cursor) planes, so this is
> trivial to implement.
> 
> The format supported by the hardware cursor is also slightly different.
> 
> v2: use more drm_rect helpers (Dmitry)
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dc.c | 59 ++++++++++++++++++++++++++++++++------
>  drivers/gpu/drm/tegra/dc.h |  5 ++++
>  2 files changed, 56 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 0541d7b5c841..7758d64822ae 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -832,10 +832,14 @@ static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
>  	return &plane->base;
>  }
>  
> -static const u32 tegra_cursor_plane_formats[] = {
> +static const u32 tegra_legacy_cursor_plane_formats[] = {
>  	DRM_FORMAT_RGBA8888,
>  };
>  
> +static const u32 tegra_cursor_plane_formats[] = {
> +	DRM_FORMAT_ARGB8888,
> +};
> +
>  static int tegra_cursor_atomic_check(struct drm_plane *plane,
>  				     struct drm_atomic_state *state)
>  {
> @@ -875,12 +879,22 @@ static void tegra_cursor_atomic_update(struct drm_plane *plane,
>  									   plane);
>  	struct tegra_plane_state *tegra_plane_state = to_tegra_plane_state(new_state);
>  	struct tegra_dc *dc = to_tegra_dc(new_state->crtc);
> -	u32 value = CURSOR_CLIP_DISPLAY;
> +	struct tegra_drm *tegra = plane->dev->dev_private;
> +	u64 dma_mask = *dc->dev->dma_mask;
> +	unsigned int x, y;
> +	u32 value = 0;
>  
>  	/* rien ne va plus */
>  	if (!new_state->crtc || !new_state->fb)
>  		return;
>  
> +	/*
> +	 * Legacy display supports hardware clipping of the cursor, but
> +	 * nvdisplay relies on software to clip the cursor to the screen.
> +	 */
> +	if (!dc->soc->has_nvdisplay)
> +		value |= CURSOR_CLIP_DISPLAY;
> +
>  	switch (new_state->crtc_w) {
>  	case 32:
>  		value |= CURSOR_SIZE_32x32;
> @@ -908,7 +922,7 @@ static void tegra_cursor_atomic_update(struct drm_plane *plane,
>  	tegra_dc_writel(dc, value, DC_DISP_CURSOR_START_ADDR);
>  
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -	value = (tegra_plane_state->iova[0] >> 32) & 0x3;
> +	value = (tegra_plane_state->iova[0] >> 32) & (dma_mask >> 32);
>  	tegra_dc_writel(dc, value, DC_DISP_CURSOR_START_ADDR_HI);
>  #endif
>  
> @@ -920,15 +934,39 @@ 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) {
> +		struct drm_rect src;
> +
> +		x = new_state->dst.x1;
> +		y = new_state->dst.y1;
> +
> +		drm_rect_fp_to_int(&src, &new_state->src);
> +
> +		value = (src.y1 & tegra->vmask) << 16 | (src.x1 & tegra->hmask);
> +		tegra_dc_writel(dc, value, DC_DISP_PCALC_HEAD_SET_CROPPED_POINT_IN_CURSOR);
> +
> +		value = (drm_rect_height(&src) & tegra->vmask) << 16 |
> +			(drm_rect_width(&src) & tegra->hmask);
> +		tegra_dc_writel(dc, value, DC_DISP_PCALC_HEAD_SET_CROPPED_SIZE_IN_CURSOR);
> +	} else {
> +		x = new_state->crtc_x;
> +		y = new_state->crtc_y;
> +	}
> +
>  	/* position the cursor */
> -	value = (new_state->crtc_y & 0x3fff) << 16 |
> -		(new_state->crtc_x & 0x3fff);
> +	value = ((y & tegra->vmask) << 16) | (x & tegra->hmask);
>  	tegra_dc_writel(dc, value, DC_DISP_CURSOR_POSITION);
>  }
>  
> @@ -982,8 +1020,13 @@ static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
>  	plane->index = 6;
>  	plane->dc = dc;
>  
> -	num_formats = ARRAY_SIZE(tegra_cursor_plane_formats);
> -	formats = tegra_cursor_plane_formats;
> +	if (!dc->soc->has_nvdisplay) {
> +		num_formats = ARRAY_SIZE(tegra_legacy_cursor_plane_formats);
> +		formats = tegra_legacy_cursor_plane_formats;
> +	} else {
> +		num_formats = ARRAY_SIZE(tegra_cursor_plane_formats);
> +		formats = tegra_cursor_plane_formats;
> +	}

Will be nice to have all tegra_legacy_ renamed to the corresponding h/w
versions, like tegra124_; and not to use the inverted checks, like
!dc->soc->has_nvdisplay. I think this will ease following of the code.
But this should be done separately.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel