All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] drm/tegra: Changes for v4.16-rc1
@ 2018-01-05 14:17 Thierry Reding
       [not found] ` <20180105141726.29713-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2018-01-05 14:17 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-tegra, dri-devel

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

 .../display/tegra/nvidia,tegra20-host1x.txt        |   14 +-
 drivers/gpu/drm/tegra/Makefile                     |    2 +
 drivers/gpu/drm/tegra/dc.c                         | 1901 +++++++++++---------
 drivers/gpu/drm/tegra/dc.h                         |  289 ++-
 drivers/gpu/drm/tegra/dpaux.c                      |  120 +-
 drivers/gpu/drm/tegra/drm.c                        |  164 +-
 drivers/gpu/drm/tegra/drm.h                        |   26 +-
 drivers/gpu/drm/tegra/dsi.c                        |  228 ++-
 drivers/gpu/drm/tegra/fb.c                         |   36 +-
 drivers/gpu/drm/tegra/gem.c                        |   15 +-
 drivers/gpu/drm/tegra/hdmi.c                       |  504 +++---
 drivers/gpu/drm/tegra/hub.c                        |  806 +++++++++
 drivers/gpu/drm/tegra/hub.h                        |   81 +
 drivers/gpu/drm/tegra/output.c                     |   24 +
 drivers/gpu/drm/tegra/plane.c                      |  378 ++++
 drivers/gpu/drm/tegra/plane.h                      |   70 +
 drivers/gpu/drm/tegra/sor.c                        | 1119 ++++++++----
 drivers/gpu/drm/tegra/sor.h                        |   16 +
 drivers/gpu/drm/tegra/vic.c                        |   20 +-
 drivers/gpu/host1x/bus.c                           |   11 +-
 drivers/gpu/host1x/dev.c                           |   36 +-
 drivers/gpu/host1x/dev.h                           |    1 +
 include/uapi/drm/drm_fourcc.h                      |   38 +-
 23 files changed, 4068 insertions(+), 1831 deletions(-)
 create mode 100644 drivers/gpu/drm/tegra/hub.c
 create mode 100644 drivers/gpu/drm/tegra/hub.h
 create mode 100644 drivers/gpu/drm/tegra/plane.c
 create mode 100644 drivers/gpu/drm/tegra/plane.h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
       [not found] ` <20180105141726.29713-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-01-05 14:58   ` Dmitry Osipenko
       [not found]     ` <558f572b-8174-c8ee-3ff8-79e591c0648d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2018-01-05 14:58 UTC (permalink / raw)
  To: Thierry Reding, Dave Airlie
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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.

[snip]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
       [not found]     ` <558f572b-8174-c8ee-3ff8-79e591c0648d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-01-08  7:42       ` Thierry Reding
  2018-01-08 12:39         ` Thierry Reding
  2018-01-08 13:47         ` Dmitry Osipenko
  0 siblings, 2 replies; 11+ messages in thread
From: Thierry Reding @ 2018-01-08  7:42 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 5003 bytes --]

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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08  7:42       ` Thierry Reding
@ 2018-01-08 12:39         ` Thierry Reding
  2018-01-08 13:47           ` Dmitry Osipenko
  2018-01-08 13:47         ` Dmitry Osipenko
  1 sibling, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2018-01-08 12:39 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 6398 bytes --]

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.

Thierry

--- >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;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08  7:42       ` Thierry Reding
  2018-01-08 12:39         ` Thierry Reding
@ 2018-01-08 13:47         ` Dmitry Osipenko
  2018-01-08 15:47           ` Thierry Reding
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2018-01-08 13:47 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 08.01.2018 10:42, 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.

My expectation was that you'll update the patch. I'm not sure why you'd want to
go with something half-working while there is already a version of the patch
that works correctly (or at least better), but of course it's up to you to decide.

If you'll decide to change your mind, feel free to cherry-pick and squash the
patch-fix [0] which also contains the fix for the YUV and adds missed ARGB4444
format.

> 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?

Video displaying with libvdpau-tegra is broken. Your fix for the YUV is correct.

> 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.

Unfortunately there is another problem that this patch uncovered. The
kms-planes-blend hangs my T30 which has a bit different display configuration
compared to T20 (it lacks HDMI). The fix [1] is quite simple, you may
cherry-pick the patch and include it in the pull-request if it's fine to do so
and if the patch looks okay, alternatively I can send the patch to the ML a bit
later today.

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

[1]
https://github.com/grate-driver/linux/commit/692c59d7f74142ce30e775d607e4a077160dbfa3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08 12:39         ` Thierry Reding
@ 2018-01-08 13:47           ` Dmitry Osipenko
       [not found]             ` <58e08004-cd76-997d-8d80-e102e31c8cda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2018-01-08 13:47 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08 13:47         ` Dmitry Osipenko
@ 2018-01-08 15:47           ` Thierry Reding
  2018-01-08 17:27             ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2018-01-08 15:47 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: linux-tegra, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 8336 bytes --]

On Mon, Jan 08, 2018 at 04:47:32PM +0300, Dmitry Osipenko wrote:
> On 08.01.2018 10:42, 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.
> 
> My expectation was that you'll update the patch. I'm not sure why you'd want to
> go with something half-working while there is already a version of the patch
> that works correctly (or at least better), but of course it's up to you to decide.

The patch that you had proposed had some rough corners that I wasn't
happy about, though I don't think I ever properly reviewed it. The
bottom line is that I wanted to avoid writing window registers in the
->atomic_flush() callback and the patch was encoding the plane Z-order
in a very fixed way. Granted, that's still true for the variant included
in this pull request, but in my opinion it paves the way to add proper
zpos property support.

Also, I seem to remember getting different test results than you, so,
again, I think we need to nail this down using proper test cases to make
sure we're testing the same things (and with the added bonus that we can
use them for regression testing). I'll set aside some time over the next
few days to write up these tests.

> If you'll decide to change your mind, feel free to cherry-pick and squash the
> patch-fix [0] which also contains the fix for the YUV and adds missed ARGB4444
> format.

We can't easily support the ARGB4444 format in the same way as the
others because there is no equivalent opaque format. We could possible
do this if we converted between opaque and alpha formats via the DRM
formats rather than the Tegra native formats, but that would be a bit
too much churn at this point. Let's revisit that for v4.17.

> > 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?
> 
> Video displaying with libvdpau-tegra is broken. Your fix for the YUV is correct.
> 
> > 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.
> 
> Unfortunately there is another problem that this patch uncovered. The
> kms-planes-blend hangs my T30 which has a bit different display configuration
> compared to T20 (it lacks HDMI). The fix [1] is quite simple, you may
> cherry-pick the patch and include it in the pull-request if it's fine to do so
> and if the patch looks okay, alternatively I can send the patch to the ML a bit
> later today.

I've picked up some bits from the fix into two separate patches. I think
setting the possible_crtcs mask to 0 for primary and cursor planes is
not correct. It looks as if this might work now because of the leases
code, but I'd like to be explicit in the driver and specify the correct
mask from the start. Otherwise we start relying on the leases code to do
the right thing and errors may creep back in if that part of the code
ever changes.

I also picked up the plane cleanup fix from your patch, though I had to
modify it a little to make sure we don't create additional planes on
Tegra186 and later (essentially kept the tegra_dc_add_planes() function
but rolled in your fixes into that instead). Also I took care of the XXX
tegra_plane_destroy() TODO while at it.

I've uploaded all of the changes to the drm/tegra/for-next branch,
though I didn't squash anything in yet to leave the branch intact for
the pull request.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
       [not found]             ` <58e08004-cd76-997d-8d80-e102e31c8cda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-01-08 15:49               ` Thierry Reding
  2018-01-08 17:28                 ` Dmitry Osipenko
  2018-01-18  0:54                 ` Dmitry Osipenko
  0 siblings, 2 replies; 11+ messages in thread
From: Thierry Reding @ 2018-01-08 15:49 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 7384 bytes --]

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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08 15:47           ` Thierry Reding
@ 2018-01-08 17:27             ` Dmitry Osipenko
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2018-01-08 17:27 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 08.01.2018 18:47, Thierry Reding wrote:
> On Mon, Jan 08, 2018 at 04:47:32PM +0300, Dmitry Osipenko wrote:
>> On 08.01.2018 10:42, 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.
>>
>> My expectation was that you'll update the patch. I'm not sure why you'd want to
>> go with something half-working while there is already a version of the patch
>> that works correctly (or at least better), but of course it's up to you to decide.
> 
> The patch that you had proposed had some rough corners that I wasn't
> happy about, though I don't think I ever properly reviewed it. The
> bottom line is that I wanted to avoid writing window registers in the
> ->atomic_flush() callback and the patch was encoding the plane Z-order
> in a very fixed way. Granted, that's still true for the variant included
> in this pull request, but in my opinion it paves the way to add proper
> zpos property support.

Would be awesome if you could review the patch properly and give a concrete
suggestions. I know that writing blend config on CRTC flush feels a bit icky and
I'm open for suggestions, but meanwhile it does the right thing.

I think Z-order could be implemented quite easily based on the fixed-order patch
that I proposed. Would be easier to implement it once we will have a proper test
cases.

> Also, I seem to remember getting different test results than you, so,
> again, I think we need to nail this down using proper test cases to make
> sure we're testing the same things (and with the added bonus that we can
> use them for regression testing). I'll set aside some time over the next
> few days to write up these tests.

I don't recall anything other than the blend test crashing on T20, I've looked
at the issue since then and it is just a trivial KMS test bug.

I'm not sure how to automate the regression testing, somehow you'd have to get
the result of the displayed stuff. I don't think you can do it on Tegra, at
least on older ones. But maybe we can semi-automate the testing by first
displaying the planes blending done on HW and then show the expected result
drawn in software, so you'd have something to compare with.

We would also need a test case with a moving planes.

Feel free to ping me once you'll have something to try out or will need some help.

>> If you'll decide to change your mind, feel free to cherry-pick and squash the
>> patch-fix [0] which also contains the fix for the YUV and adds missed ARGB4444
>> format.
> 
> We can't easily support the ARGB4444 format in the same way as the
> others because there is no equivalent opaque format. We could possible
> do this if we converted between opaque and alpha formats via the DRM
> formats rather than the Tegra native formats, but that would be a bit
> too much churn at this point. Let's revisit that for v4.17.

Okay, tegra_plane_format_has_alpha() tells that ARGB4444 is format without
'alpha', that confuses me. Would be nice if you could adjust the code to avoid
that confusion, maybe rename function to tegra_plane_format_non_native() or
something like that.

>>> 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?
>>
>> Video displaying with libvdpau-tegra is broken. Your fix for the YUV is correct.
>>
>>> 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.
>>
>> Unfortunately there is another problem that this patch uncovered. The
>> kms-planes-blend hangs my T30 which has a bit different display configuration
>> compared to T20 (it lacks HDMI). The fix [1] is quite simple, you may
>> cherry-pick the patch and include it in the pull-request if it's fine to do so
>> and if the patch looks okay, alternatively I can send the patch to the ML a bit
>> later today.
> 
> I've picked up some bits from the fix into two separate patches. I think
> setting the possible_crtcs mask to 0 for primary and cursor planes is
> not correct. It looks as if this might work now because of the leases
> code, but I'd like to be explicit in the driver and specify the correct
> mask from the start. Otherwise we start relying on the leases code to do
> the right thing and errors may creep back in if that part of the code
> ever changes.

Either way is fine to me.

> I also picked up the plane cleanup fix from your patch, though I had to
> modify it a little to make sure we don't create additional planes on
> Tegra186 and later (essentially kept the tegra_dc_add_planes() function
> but rolled in your fixes into that instead). Also I took care of the XXX
> tegra_plane_destroy() TODO while at it.

Ok.

> I've uploaded all of the changes to the drm/tegra/for-next branch,
> though I didn't squash anything in yet to leave the branch intact for
> the pull request.

I've tested the patches, everything works correctly.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08 15:49               ` Thierry Reding
@ 2018-01-08 17:28                 ` Dmitry Osipenko
  2018-01-18  0:54                 ` Dmitry Osipenko
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2018-01-08 17:28 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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.

Ok.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
  2018-01-08 15:49               ` Thierry Reding
  2018-01-08 17:28                 ` Dmitry Osipenko
@ 2018-01-18  0:54                 ` Dmitry Osipenko
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2018-01-18  0:54 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-01-18  0:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-05 14:17 [GIT PULL] drm/tegra: Changes for v4.16-rc1 Thierry Reding
     [not found] ` <20180105141726.29713-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-05 14:58   ` Dmitry Osipenko
     [not found]     ` <558f572b-8174-c8ee-3ff8-79e591c0648d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-08  7:42       ` Thierry Reding
2018-01-08 12:39         ` Thierry Reding
2018-01-08 13:47           ` Dmitry Osipenko
     [not found]             ` <58e08004-cd76-997d-8d80-e102e31c8cda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-08 15:49               ` Thierry Reding
2018-01-08 17:28                 ` Dmitry Osipenko
2018-01-18  0:54                 ` Dmitry Osipenko
2018-01-08 13:47         ` Dmitry Osipenko
2018-01-08 15:47           ` Thierry Reding
2018-01-08 17:27             ` Dmitry Osipenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.