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

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

On 08.01.2018 18:49, Thierry Reding wrote:
> On Mon, Jan 08, 2018 at 04:47:42PM +0300, Dmitry Osipenko wrote:
>> 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.
> 
> I'd like to keep the possibility to return an error. We've already seen
> how this caught an issue (return -EINVAL for YUV formats). Returning the
> format would silently return the opaque format, which may not be the
> right thing to do in all cases. Again, I'd like this to remain as
> explicit as possible so we don't start to rely on implicit behaviour and
> have hard to find errors creep in that way.

Unfortunately RGB565 is also broken, maybe a simple/implicit format adjustment
is less error-prone after all.