Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1

[Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 08.01.2018 15:39, Thierry Reding wrote:
> On Mon, Jan 08, 2018 at 08:42:50AM +0100, Thierry Reding wrote:
>> On Fri, Jan 05, 2018 at 05:58:17PM +0300, Dmitry Osipenko wrote:
>>> On 05.01.2018 17:17, Thierry Reding wrote:
>>>> Hi Dave,
>>>>
>>>> The following changes since commit 9428088c90b6f7d5edd2a1b0d742c75339b36f6e:
>>>>
>>>>   drm/qxl: reapply cursor after resetting primary (2017-12-08 13:37:02 +1000)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>   git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-4.16-rc1
>>>>
>>>> for you to fetch changes up to ebae8d07435ae91314f4a28d69b530d09c625815:
>>>>
>>>>   drm/tegra: dc: Implement legacy blending (2017-12-21 14:55:55 +0100)
>>>>
>>>> Thanks,
>>>> Thierry
>>>>
>>>> ----------------------------------------------------------------
>>>> drm/tegra: Changes for v4.16-rc1
>>>>
>>>> The bulk of these changes are preparation work and addition of support
>>>> for Tegra186. Currently only HDMI output (the primary output on Jetson
>>>> TX2) is supported, but the hardware is also capable of doing DSI and
>>>> DisplayPort.
>>>>
>>>> Tegra DRM now also uses the atomic commit helpers instead of the open-
>>>> coded variant that was only doing half its job. As a bit of a byproduct
>>>> of the Tegra186 support the driver also gained HDMI 2.0 as well as zpos
>>>> property support.
>>>>
>>>> Along the way there are also a few patches to clean up a few things and
>>>> fix minor issues.
>>>>
>>>> ----------------------------------------------------------------
>>>> Arnd Bergmann (2):
>>>>       drm/tegra: Mark Tegra186 display hub PM functions __maybe_unused
>>>>       drm/tegra: Fix non-debugfs builds
>>>>
>>>> Dmitry Osipenko (3):
>>>>       drm/tegra: dc: Link DC1 to DC0 on Tegra20
>>>>       drm/tegra: gem: Correct iommu_map_sg() error checking
>>>>       drm/tegra: Correct timeout in tegra_syncpt_wait
>>>>
>>>> Thierry Reding (43):
>>>>       drm/fourcc: Fix fourcc_mod_code() definition
>>>>       drm/tegra: Sanitize format modifiers
>>>>       gpu: host1x: Rewrite conditional for better readability
>>>>       gpu: host1x: Cleanup on initialization failure
>>>>       dt-bindings: display: tegra: Update SOR for Tegra186
>>>>       drm/tegra: dc: Move register definitions into a table
>>>>       drm/tegra: dsi: Move register definitions into a table
>>>>       drm/tegra: hdmi: Move register definitions into a table
>>>>       drm/tegra: sor: Move register definitions into a table
>>>>       drm/tegra: dc: Reshuffle some code
>>>>       drm/tegra: dc: Register debugfs in ->late_register()
>>>>       drm/tegra: dsi: Register debugfs in ->late_register()
>>>>       drm/tegra: hdmi: Register debugfs in ->late_register()
>>>>       drm/tegra: sor: Root debugfs files at the connector
>>>>       drm/tegra: sor: Register debugfs in ->late_register()
>>>>       drm/tegra: Do not wrap lines unnecessarily
>>>>       drm/tegra: vic: Properly align arguments
>>>>       drm/tegra: dc: Support background color
>>>>       drm/tegra: Use atomic commit helpers
>>>>       drm/tegra: Remove custom page-flip handler
>>>>       drm/tegra: dc: Remove tegra_primary_plane_destroy()
>>>>       drm/tegra: dc: Remove duplicate plane funcs
>>>>       drm/tegra: dc: Remove tegra_overlay_plane_destroy()
>>>>       drm/tegra: dc: Remove duplicate plane funcs
>>>>       drm/tegra: dc: Move state definition to header
>>>>       drm/tegra: Move common plane code to separate file
>>>>       drm/tegra: Add Tegra186 display hub support
>>>>       drm/tegra: dc: Add Tegra186 support
>>>>       drm/tegra: Support ARGB and ABGR formats
>>>>       drm/tegra: sor: Parameterize register offsets
>>>>       drm/tegra: sor: Add Tegra186 support
>>>>       drm/tegra: sor: Support HDMI 2.0 modes
>>>>       drm/tegra: dpaux: Implement runtime PM
>>>>       drm/tegra: dpaux: Add Tegra186 support
>>>>       drm/tegra: fb: Force alpha formats
>>>>       drm/tegra: dc: Support more formats
>>>>       drm/tegra: dc: Use direct offset to plane registers
>>>>       drm/tegra: dc: Remove redundant spinlock
>>>>       drm/tegra: Implement zpos property
>>>>       gpu: host1x: Use IOMMU groups
>>>>       drm/tegra: Use IOMMU groups
>>>>       drm/tegra: dpaux: Keep reset defaults for hybrid pad parameters
>>>
>>>
>>>>       drm/tegra: dc: Implement legacy blending
>>>
>>> Please hold on this patch. First of all it doesn't work correctly, see my last
>>> reply to the patch. Secondly, it introduces bug that breaks YUV plane.
>>
>> I thought we had already concluded that this version doesn't cause any
>> regressions. And since this is new functionality I'm not too worried if
>> it doesn't work in all cases, we've got plenty of time to fix those up.
>>
>> As for the YUV plane bug, can you point me at a test case, or describe
>> what exactly you're trying to do so that I can reproduce and fix it?
>>
>> I'd like to make forward progress on this rather than hold back on
>> patches out of fear that they may break existing functionality. In order
>> to do that we need tests that can be run to validate existing
>> functionality.
> 
> I think I was able to reproduce the bug you mentioned by running this on
> a TrimSlice:
> 
> 	$ modetest -M tegra -s HDMI-A-1@34:1920x1080 -P 28@34:256x256+64+64@XR24 -P 30@34:256x256+384+64@UYVY
> 
> This currently fails with the above patch, but I can fix it using the
> below diff. Essentially we treat YUV planes as opaque, but currently
> error out on YUV formats because they have no "alpha" equivalent. The
> fix is to simply return the same format.
> 
> Can you see any more issues after applying this fix? I wasn't able to
> find a situation where it doesn't work.

This is the only issue with YUV that I experienced.

> 
> --- >8 ---
> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> index 154b4d337d0a..36a06a993698 100644
> --- a/drivers/gpu/drm/tegra/plane.c
> +++ b/drivers/gpu/drm/tegra/plane.c
> @@ -276,6 +276,11 @@ bool tegra_plane_format_has_alpha(unsigned int format)
> 
>  int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha)
>  {
> +	if (tegra_plane_format_is_yuv(opaque, NULL)) {
> +		*alpha = opaque;
> +		return 0;
> +	}
> +
>  	switch (opaque) {
>  	case WIN_COLOR_DEPTH_B5G5R5X1:
>  		*alpha = WIN_COLOR_DEPTH_B5G5R5A1;
> 

Alternatively we can make tegra_plane_format_get_alpha() to return modified
format like I did here [0], which I think makes code a bit cleaner.

[0]
https://github.com/grate-driver/linux/commit/9f7cf5943eb37995a38275627a26dee4aa0d0d94#diff-d54189ce320219e9a4fa2cf564c78899L264