Commit Graph

5 Commits

Author SHA1 Message Date
Radhakrishna Sripada
47e22ff1a9 drm: Add connector property to limit max bpc
At times 12bpc HDMI cannot be driven due to faulty cables, dongles
level shifters etc. To workaround them we may need to drive the output
at a lower bpc. Currently the user space does not have a way to limit
the bpc. The default bpc to be programmed is decided by the driver and
is run against connector limitations.

Creating a new connector property "max bpc" in order to limit the bpc.
xrandr can make use of this connector property to make sure that bpc does
not exceed the configured value. This property can be used by userspace to
set the bpc.

V2: Initialize max_bpc to satisfy kms_properties
V3: Move the property to drm_connector
V4: Split drm and i915 components(Ville)
V5: Make the property per connector(Ville)
V6: Compare the requested bpc to connector bpc(Daniel)
    Move the attach_property function to core(Ville)
V7: Fix checkpatch warnings
V8: Simplify the connector check code(Ville)
V9: Const display_info(Ville)
V10,V11: Fix CI issues.
V12: Add the Kernel documentation(Daniel)
V14: Crossreference the function name in the doc(Daniel)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Sunpeng Li <sunpeng.li@amd.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181012184233.29250-1-radhakrishna.sripada@intel.com
2018-11-02 09:15:58 -07:00
Lyude Paul
39b50c6038 drm/atomic_helper: Stop modesets on unregistered connectors harder
Unfortunately, it appears our fix in:
commit b5d29843d8 ("drm/atomic_helper: Allow DPMS On<->Off changes
for unregistered connectors")

Which attempted to work around the problems introduced by:
commit 4d80273976 ("drm/atomic_helper: Disallow new modesets on
unregistered connectors")

Is still not the right solution, as modesets can still be triggered
outside of drm_atomic_set_crtc_for_connector().

So in order to fix this, while still being careful that we don't break
modesets that a driver may perform before being registered with
userspace, we replace connector->registered with a tristate member,
connector->registration_state. This allows us to keep track of whether
or not a connector is still initializing and hasn't been exposed to
userspace, is currently registered and exposed to userspace, or has been
legitimately removed from the system after having once been present.

Using this info, we can prevent userspace from performing new modesets
on unregistered connectors while still allowing the driver to perform
modesets on unregistered connectors before the driver has finished being
registered.

Changes since v1:
- Fix WARN_ON() in drm_connector_cleanup() that CI caught with this
  patchset in igt@drv_module_reload@basic-reload-inject and
  igt@drv_module_reload@basic-reload by checking if the connector is
  registered instead of unregistered, as calling drm_connector_cleanup()
  on a connector that hasn't been registered with userspace yet should
  stay valid.
- Remove unregistered_connector_check(), and just go back to what we
  were doing before in commit 4d80273976 ("drm/atomic_helper: Disallow
  new modesets on unregistered connectors") except replacing
  READ_ONCE(connector->registered) with drm_connector_is_unregistered().
  This gets rid of the behavior of allowing DPMS On<->Off, but that should
  be fine as it's more consistent with the UAPI we had before - danvet
- s/drm_connector_unregistered/drm_connector_is_unregistered/ - danvet
- Update documentation, fix some typos.

Fixes: b5d29843d8 ("drm/atomic_helper: Allow DPMS On<->Off changes for unregistered connectors")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: stable@vger.kernel.org
Cc: David Airlie <airlied@linux.ie>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20181016203946.9601-1-lyude@redhat.com
2018-10-17 13:50:14 -04:00
Lyude Paul
b5d29843d8 drm/atomic_helper: Allow DPMS On<->Off changes for unregistered connectors
It appears when testing my previous fix for some of the legacy
modesetting issues with MST, I misattributed some kernel splats that
started appearing on my machine after a rebase as being from upstream.
But it appears they actually came from my patch series:

[    2.980512] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Updating routing for [CONNECTOR:65:eDP-1]
[    2.980516] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CONNECTOR:65:eDP-1] is not registered
[    2.980516] ------------[ cut here ]------------
[    2.980519] Could not determine valid watermarks for inherited state
[    2.980553] WARNING: CPU: 3 PID: 551 at drivers/gpu/drm/i915/intel_display.c:14983 intel_modeset_init+0x14d7/0x19f0 [i915]
[    2.980556] Modules linked in: i915(O+) i2c_algo_bit drm_kms_helper(O) syscopyarea sysfillrect sysimgblt fb_sys_fops drm(O) intel_rapl x86_pkg_temp_thermal iTCO_wdt wmi_bmof coretemp crc32_pclmul psmouse i2c_i801 mei_me mei i2c_core lpc_ich mfd_core tpm_tis tpm_tis_core wmi tpm thinkpad_acpi pcc_cpufreq video ehci_pci crc32c_intel serio_raw ehci_hcd xhci_pci xhci_hcd
[    2.980577] CPU: 3 PID: 551 Comm: systemd-udevd Tainted: G           O      4.19.0-rc7Lyude-Test+ #1
[    2.980579] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET63WW (1.27 ) 11/10/2016
[    2.980605] RIP: 0010:intel_modeset_init+0x14d7/0x19f0 [i915]
[    2.980607] Code: 89 df e8 ec 27 02 00 e9 24 f2 ff ff be 03 00 00 00 48 89 df e8 da 27 02 00 e9 26 f2 ff ff 48 c7 c7 c8 d1 34 a0 e8 23 cf dc e0 <0f> 0b e9 7c fd ff ff f6 c4 04 0f 85 37 f7 ff ff 48 8b 83 60 08 00
[    2.980611] RSP: 0018:ffffc90000287988 EFLAGS: 00010282
[    2.980614] RAX: 0000000000000000 RBX: ffff88031b488000 RCX: 0000000000000006
[    2.980617] RDX: 0000000000000007 RSI: 0000000000000086 RDI: ffff880321ad54d0
[    2.980620] RBP: ffffc90000287a10 R08: 000000000000040a R09: 0000000000000065
[    2.980623] R10: ffff88030ebb8f00 R11: ffffffff81416590 R12: ffff88031b488000
[    2.980626] R13: ffff88031b4883a0 R14: ffffc900002879a8 R15: ffff880319099800
[    2.980630] FS:  00007f475620d180(0000) GS:ffff880321ac0000(0000) knlGS:0000000000000000
[    2.980633] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    2.980636] CR2: 00007f9ef28018a0 CR3: 000000031b72c001 CR4: 00000000003606e0
[    2.980639] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    2.980642] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    2.980645] Call Trace:
[    2.980675]  i915_driver_load+0xb0e/0xdc0 [i915]
[    2.980681]  ? kernfs_add_one+0xe7/0x130
[    2.980709]  i915_pci_probe+0x46/0x60 [i915]
[    2.980715]  pci_device_probe+0xd4/0x150
[    2.980719]  really_probe+0x243/0x3b0
[    2.980722]  driver_probe_device+0xba/0x100
[    2.980726]  __driver_attach+0xe4/0x110
[    2.980729]  ? driver_probe_device+0x100/0x100
[    2.980733]  bus_for_each_dev+0x74/0xb0
[    2.980736]  driver_attach+0x1e/0x20
[    2.980739]  bus_add_driver+0x159/0x230
[    2.980743]  ? 0xffffffffa0393000
[    2.980746]  driver_register+0x70/0xc0
[    2.980749]  ? 0xffffffffa0393000
[    2.980753]  __pci_register_driver+0x57/0x60
[    2.980780]  i915_init+0x55/0x58 [i915]
[    2.980785]  do_one_initcall+0x4a/0x1c4
[    2.980789]  ? do_init_module+0x27/0x210
[    2.980793]  ? kmem_cache_alloc_trace+0x131/0x190
[    2.980797]  do_init_module+0x60/0x210
[    2.980800]  load_module+0x2063/0x22e0
[    2.980804]  ? vfs_read+0x116/0x140
[    2.980807]  ? vfs_read+0x116/0x140
[    2.980811]  __do_sys_finit_module+0xbd/0x120
[    2.980814]  ? __do_sys_finit_module+0xbd/0x120
[    2.980818]  __x64_sys_finit_module+0x1a/0x20
[    2.980821]  do_syscall_64+0x5a/0x110
[    2.980824]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[    2.980826] RIP: 0033:0x7f4754e32879
[    2.980828] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d f7 45 2c 00 f7 d8 64 89 01 48
[    2.980831] RSP: 002b:00007fff43fd97d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[    2.980834] RAX: ffffffffffffffda RBX: 0000559a44ca64f0 RCX: 00007f4754e32879
[    2.980836] RDX: 0000000000000000 RSI: 00007f475599f4cd RDI: 0000000000000018
[    2.980838] RBP: 00007f475599f4cd R08: 0000000000000000 R09: 0000000000000000
[    2.980839] R10: 0000000000000018 R11: 0000000000000246 R12: 0000000000000000
[    2.980841] R13: 0000559a44c92fd0 R14: 0000000000020000 R15: 0000000000000000
[    2.980881] WARNING: CPU: 3 PID: 551 at drivers/gpu/drm/i915/intel_display.c:14983 intel_modeset_init+0x14d7/0x19f0 [i915]
[    2.980884] ---[ end trace 5eb47a76277d4731 ]---

The cause of this appears to be due to the fact that if there's
pre-existing display state that was set by the BIOS when i915 loads, it
will attempt to perform a modeset before the driver is registered with
userspace. Since this happens before the driver's registered with
userspace, it's connectors are also unregistered and thus-states which
would turn on DPMS on a connector end up getting rejected since the
connector isn't registered.

These bugs managed to get past Intel's CI partially due to the fact it
never ran a full test on my patches for some reason, but also because
all of the tests unload the GPU once before running. Since this bug is
only really triggered when the drivers tries to perform a modeset before
it's been fully registered with userspace when coming from whatever
display configuration the firmware left us with, it likely would never
have been picked up by CI in the first place.

After some discussion with vsyrjala, we decided the best course of
action would be to just move the unregistered connector checks out of
update_connector_routing() and into drm_atomic_set_crtc_for_connector().
The reason for this being that legacy modesetting isn't going to be
expecting failures anywhere (at least this is the case with X), so
ideally we want to ensure that any DPMS changes will still work even on
unregistered connectors. Instead, we now only reject new modesets which
would change the current CRTC assigned to an unregistered connector
unless no new CRTC is being assigned to replace the connector's previous
one.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: 4d80273976 ("drm/atomic_helper: Disallow new modesets on unregistered connectors")
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181009204424.21462-1-lyude@redhat.com
2018-10-10 15:24:20 -04:00
Chris Wilson
69fdf4206a drm: Differentiate the lack of an interface from invalid parameter
If the ioctl is not supported on a particular piece of HW/driver
combination, report ENOTSUP (aka EOPNOTSUPP) so that it can be easily
distinguished from both the lack of the ioctl and from a regular invalid
parameter.

v2: Across all the kms ioctls we had a mixture of reporting EINVAL,
ENODEV and a few ENOTSUPP (most where EINVAL) for a failed
drm_core_check_feature(). Update everybody to report ENOTSUPP.

v3: ENOTSUPP is an internal errno! It's value (524) does not correspond
to a POSIX errno, the one we want is ENOTSUP. However,
uapi/asm-generic/errno.h doesn't include ENOTSUP but man errno says

	"ENOTSUP and EOPNOTSUPP have the same value on Linux,
	but according to POSIX.1 these error values should be
	distinct."

so use EOPNOTSUPP as its equivalent.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v2
Link: https://patchwork.freedesktop.org/patch/msgid/20180913192050.24812-1-chris@chris-wilson.co.uk
2018-09-14 17:29:47 +01:00
Daniel Vetter
72fdb40c1a drm: extract drm_atomic_uapi.c
This leaves all the commit/check and state handling in drm_atomic.c,
while pulling all the uapi glue and the huge ioctl itself into a
seprate file.

This seems to almost perfectly split the rather big drm_atomic.c file
into 2 equal sizes.

Also adjust the kerneldoc and type a very terse overview text.

v2: Rebase.

v3: Fix tiny typo.

v4:
- Fixup armada, newly converted atomic driver hooray!
- Fixup msm/dpu1, newly added too.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: intel-gfx@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180905135711.28370-7-daniel.vetter@ffwll.ch
2018-09-09 14:19:18 +02:00