[Bug 108355] Civilization VI - Artifacts in mouse cursor
https://bugs.freedesktop.org/show_bug.cgi?id=108355 Hadrien Nilsson changed: What|Removed |Added Component|Drivers/Gallium/radeonsi|Drivers/Gallium/softpipe Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org --- Comment #1 from Hadrien Nilsson --- I start thinking I'm not in the right Bugzilla section as OpenGL does not handle mouse cursors. Sorry if that the case, I would love to know the exact faulting component in the graphics stack. I do not know if this is a SDL, X11, drm, amdgpu or hardware problem, or the game itself (some kind of surface corruption). But Gnome screenshot program is able to correctly retrieve the cursor image as shown in the attachment. I wrote a small SDL program that changes my mouse cursor, as this is what Civ6 seems to use, but everything works fine. I contacted Aspyr support but their response was a dead-end : "AMD an Intel GPUs aren't supported". I tried to make the game use my own compiled SDL version with no luck. Steam games seem to use some kind of sandbox, LD_PRELOAD seems to be ignored. Or the game may not actually use SDL for the mouse cursor though the related symbols are referenced in the executable. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v3]
Jason Ekstrand writes: > You need to add this to anv_gem_stubs.c as well or else the unit tests > won't build. Sorry for not catching it earlier. I'm always missing this > too. Well, that's a bit hard to test as -Dbuild-tests=true fails in a bunch of glx tests, but I think I've got it. > With that fixed, the anv bits are > > Reviewed-by: Jason Ekstrand Thanks. I haven't heard from any radv developers, so I can either split the patch apart or wait for another day or two. In any case, I'll post v4 of the patch here with the anv_gem_reg_read addition made. -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v4]
Offers three clocks, device, clock monotonic and clock monotonic
raw. Could use some kernel support to reduce the deviation between
clock values.
v2:
Ensure deviation is at least as big as the GPU time interval.
v3:
Set device->lost when returning DEVICE_LOST.
Use MAX2 and DIV_ROUND_UP instead of open coding these.
Delete spurious TIMESTAMP in radv version.
Suggested-by: Jason Ekstrand
Suggested-by: Lionel Landwerlin
v4:
Add anv_gem_reg_read to anv_gem_stubs.c
Suggested-by: Jason Ekstrand
Signed-off-by: Keith Packard
---
src/amd/vulkan/radv_device.c | 81 +++
src/amd/vulkan/radv_extensions.py | 1 +
src/intel/vulkan/anv_device.c | 89 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_gem.c | 13 +
src/intel/vulkan/anv_gem_stubs.c | 7 +++
src/intel/vulkan/anv_private.h | 2 +
7 files changed, 194 insertions(+)
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 174922780fc..80050485e54 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -4955,3 +4955,84 @@ radv_GetDeviceGroupPeerMemoryFeatures(
VK_PEER_MEMORY_FEATURE_GENERIC_SRC_BIT |
VK_PEER_MEMORY_FEATURE_GENERIC_DST_BIT;
}
+
+static const VkTimeDomainEXT radv_time_domains[] = {
+ VK_TIME_DOMAIN_DEVICE_EXT,
+ VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT,
+ VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT,
+};
+
+VkResult radv_GetPhysicalDeviceCalibrateableTimeDomainsEXT(
+ VkPhysicalDevice physicalDevice,
+ uint32_t *pTimeDomainCount,
+ VkTimeDomainEXT *pTimeDomains)
+{
+ int d;
+ VK_OUTARRAY_MAKE(out, pTimeDomains, pTimeDomainCount);
+
+ for (d = 0; d < ARRAY_SIZE(radv_time_domains); d++) {
+ vk_outarray_append(, i) {
+ *i = radv_time_domains[d];
+ }
+ }
+
+ return vk_outarray_status();
+}
+
+static uint64_t
+radv_clock_gettime(clockid_t clock_id)
+{
+ struct timespec current;
+ int ret;
+
+ ret = clock_gettime(clock_id, );
+ if (ret < 0 && clock_id == CLOCK_MONOTONIC_RAW)
+ ret = clock_gettime(CLOCK_MONOTONIC, );
+ if (ret < 0)
+ return 0;
+
+ return (uint64_t) current.tv_sec * 10ULL + current.tv_nsec;
+}
+
+VkResult radv_GetCalibratedTimestampsEXT(
+ VkDevice _device,
+ uint32_t timestampCount,
+ const VkCalibratedTimestampInfoEXT *pTimestampInfos,
+ uint64_t *pTimestamps,
+ uint64_t *pMaxDeviation)
+{
+ RADV_FROM_HANDLE(radv_device, device, _device);
+ uint32_t clock_crystal_freq =
device->physical_device->rad_info.clock_crystal_freq;
+ int d;
+ uint64_t begin, end;
+
+ begin = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
+
+ for (d = 0; d < timestampCount; d++) {
+ switch (pTimestampInfos[d].timeDomain) {
+ case VK_TIME_DOMAIN_DEVICE_EXT:
+ pTimestamps[d] = device->ws->query_value(device->ws,
+
RADEON_TIMESTAMP);
+ break;
+ case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
+ pTimestamps[d] = radv_clock_gettime(CLOCK_MONOTONIC);
+ break;
+
+ case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
+ pTimestamps[d] = begin;
+ break;
+ default:
+ pTimestamps[d] = 0;
+ break;
+ }
+ }
+
+ end = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
+
+ uint64_t clock_period = end - begin;
+ uint64_t device_period = DIV_ROUND_UP(100, clock_crystal_freq);
+
+ *pMaxDeviation = MAX2(clock_period, device_period);
+
+ return VK_SUCCESS;
+}
diff --git a/src/amd/vulkan/radv_extensions.py
b/src/amd/vulkan/radv_extensions.py
index 5dcedae1c63..4c81d3f0068 100644
--- a/src/amd/vulkan/radv_extensions.py
+++ b/src/amd/vulkan/radv_extensions.py
@@ -92,6 +92,7 @@ EXTENSIONS = [
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
+Extension('VK_EXT_calibrated_timestamps', 1, True),
Extension('VK_EXT_conditional_rendering', 1, True),
Extension('VK_EXT_conservative_rasterization',1,
'device->rad_info.chip_class
[Bug 108381] amdgpu_bo_import and amdgpu_bo_free are not paired which leaks amdgpu_bo
https://bugs.freedesktop.org/show_bug.cgi?id=108381 --- Comment #1 from Yong Zhang --- Created attachment 142040 --> https://bugs.freedesktop.org/attachment.cgi?id=142040=edit possible fix to this issue attached possible fix to this issue -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108381] amdgpu_bo_import and amdgpu_bo_free are not paired which leaks amdgpu_bo
https://bugs.freedesktop.org/show_bug.cgi?id=108381 Bug ID: 108381 Summary: amdgpu_bo_import and amdgpu_bo_free are not paired which leaks amdgpu_bo Product: Mesa Version: 18.2 Hardware: All OS: Linux (All) Status: NEW Severity: major Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: zhangy...@lbesec.com QA Contact: dri-devel@lists.freedesktop.org Created attachment 142039 --> https://bugs.freedesktop.org/attachment.cgi?id=142039=edit sample code to reproduce the issue amdgpu_bo is reference counted, libdrm expects the number of calls to amdgpu_bo_import must be equal to amdgpu_bo_free. When importing prime fd into mesa, amdgpu_bo_from_handle will be called, which calls amdgpu_bo_import to get the unique handle (which in fact is a pointer to amdgpu_bo), then wrapped into amdgpu_winsys_bo object. The problem is amdgpu_winsys_bo is also reference counted, if one tries to import the same prime fd for multiple times, amdgpu_bo_import will be called for multiple times, but only one amdgpu_winsys_bo will be created, so amdgpu_bo_free will be called for only once, eventually leaks memory. I have attached a sample code to reproduce this issue. a possible solution is to call amdgpu_bo_free after amdgpu_bo_import if result.buf_handle can be found in ws->bo_export_table, which will balance the number of calls to amdgpu_bo_import and amdgpu_bo_free. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106177] overclocking doesn't work with 4.17-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=106177 --- Comment #8 from alvarex --- same effect as Christoph Haag mentions echoing pp_dpm_mclk and pp_mclk_od reports nothing changed. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106177] overclocking doesn't work with 4.17-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=106177 --- Comment #7 from alvarex --- I can confirm this behaviour with kernel 4.16 and point release 16 and 18 it works as I would expect, with kernel 4.17, 4.18.14 and 4.19 from drm fixes git it doesn't work. Setting feature mask have no effect. Polaris Rx 460 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108379] high idle power consumption
https://bugs.freedesktop.org/show_bug.cgi?id=108379 Bug ID: 108379 Summary: high idle power consumption Product: DRI Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: gr...@sub.red system: - Polaris10 (Ellesmere) - Linux 4.18.14 - WQHD (2560*1440 pixels) monitor with 144 Hz (DisplayPort) expected behaviour: GPU power states should be lowered to the minimum states while idling to save power, heat and noise. actual behaviour: - DC on (default): GPU stuck at highest SCLK DPM state, power consumption as per amdgpu_pm_info is ~24 Watts - amdgpu.dc=0: MCLK stuck at high DPM state, power consumption as per amdgpu_pm_info is ~26 Watts - Windows: correct (lowest) SCLK/MCLK DPM states, measured power consumption is ~16 Watts lower compared to Linux Note: https://bugzilla.kernel.org/show_bug.cgi?id=201275 fixed a regression causing *additional* power draw (SCLK and MCLK both at max, power consumption at ~38 Watts while idling). I can't tell if it's a regression as I've just recently got the GPU. Randomly tested kernels from within 4.17 to 4.18.14 all suffer from this issue. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/atomic_helper: Stop modesets on unregistered connectors harder
Unfortunately, it appears our fix in:
commit b5d29843d8ef ("drm/atomic_helper: Allow DPMS On<->Off changes
for unregistered connectors")
Which attempted to work around the problems introduced by:
commit 4d80273976bf ("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.
Fixes: b5d29843d8ef ("drm/atomic_helper: Allow DPMS On<->Off changes for
unregistered connectors")
Cc: Ville Syrjälä
Cc: Daniel Vetter
Cc: Rodrigo Vivi
Cc: sta...@vger.kernel.org
Cc: David Airlie
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_atomic_helper.c | 60 +
drivers/gpu/drm/drm_atomic_uapi.c | 21 -
drivers/gpu/drm/drm_connector.c | 10 ++---
drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++--
include/drm/drm_connector.h | 68 -
5 files changed, 127 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 6f66777dca4b..6cadeaf28ae4 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -529,6 +529,35 @@ mode_valid(struct drm_atomic_state *state)
return 0;
}
+static int
+unregistered_connector_check(struct drm_atomic_state *state,
+struct drm_connector *connector,
+struct drm_connector_state *old_conn_state,
+struct drm_connector_state *new_conn_state)
+{
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+
+ if (!drm_connector_unregistered(connector))
+ return 0;
+
+ crtc = new_conn_state->crtc;
+ if (!crtc)
+ return 0;
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ if (!crtc_state || !drm_atomic_crtc_needs_modeset(crtc_state))
+ return 0;
+
+ if (crtc_state->mode_changed) {
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] can't change mode on
unregistered connector\n",
+connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/**
* drm_atomic_helper_check_modeset - validate state object for modeset changes
* @dev: DRM device
@@ -684,18 +713,33 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
return ret;
}
- /*
-* Iterate over all connectors again, to make sure atomic_check()
-* has been called on them when a modeset is forced.
-*/
for_each_oldnew_connector_in_state(state, connector,
old_connector_state, new_connector_state, i) {
const struct drm_connector_helper_funcs *funcs =
connector->helper_private;
- if (connectors_mask & BIT(i))
- continue;
+ /* Make sure atomic_check() is called on any unchecked
+* connectors when a modeset has been forced
+*/
+ if (connectors_mask & BIT(i) && funcs->atomic_check) {
+ ret = funcs->atomic_check(connector,
+ new_connector_state);
+ if (ret)
+ return ret;
+ }
- if (funcs->atomic_check)
- ret = funcs->atomic_check(connector,
new_connector_state);
+ /*
+* Prevent userspace from turning on new displays or setting
+* new modes using connectors which have been removed from
+* userspace. This is racy since an unplug could happen at any
+* time including after this check, but that's OK: we only
+* care about preventing userspace from trying to set invalid
+* state using destroyed connectors that it's been notified
+* about. No one can save us after the atomic check completes
+* but ourselves.
+*/
+ ret = unregistered_connector_check(state,
+ connector,
+
Re: [PATCH v5 16/28] drm/i915/dsc: Define & Compute VESA DSC params
Ville/Jani,
Could you please look at the logistics of the patch and ACK this?
This has been validated and tested on the DSC panel.
Regards
Manasi
On Fri, Oct 05, 2018 at 04:22:54PM -0700, Manasi Navare wrote:
> From: Gaurav K Singh
>
> This patches does the following:
>
> 1. This patch defines all the DSC parameters as per the VESA
> DSC specification. These are stored in the encoder and used
> to compute the PPS parameters to be sent to the Sink.
> 2. Compute all the DSC parameters which are derived from DSC
> state of intel_crtc_state.
> 3. Compute all parameters that are VESA DSC specific
>
> This computation happens in the atomic check phase during
> compute_config() to validate if display stream compression
> can be enabled for the requested mode.
>
> v7: (From Manasi)
> * Dont use signed int for rc_range_params (Manasi)
> * Mask the range_bpg_offset to use only 6 bits
> * Add SPDX identifier (Chris Wilson)
> v6 (From Manasi):
> * Add a check for line_buf_depth return value (Anusha)
> * Remove DRM DSC constants to different patch (Manasi)
> v5 (From Manasi):
> * Add logic to limit the max line buf depth for DSC 1.1 to 13
> as per DSC 1.1 spec
> * Fix dim checkpatch warnings/checks
>
> v4 (From Gaurav):
> * Rebase on latest drm tip
> * rename variable name(Manasi)
> * Populate linebuf_depth variable(Manasi)
>
> v3 (From Gaurav):
> * Rebase my previous patches on top of Manasi's latest patch
> series
> * Using >>n rather than /2^n (Manasi)
> * Change the commit message to explain what the patch is doing(Gaurav)
>
> Fixed review comments from Ville:
> * Don't use macro TWOS_COMPLEMENT
> * Mention in comment about the source of RC params
> * Return directly from case statements
> * Using single asssignment for assigning rc_range_params
> * Using < about the fixed point numbers
>
> v2 (From Manasi):
> * Update logic for minor version to consider the dpcd value
> and what supported by the HW platform
> * Use DRM DSC config struct instead of intel_dp struct
> * Move the DSC constants to DRM DSC header file
> * Use u16, u8 where bigger data types not needed
> * * Compute the DSC parameters as part of DSC compute config
> since the computation can fail (Manasi)
>
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Anusha Srivatsa
> Cc: Gaurav K Singh
> Signed-off-by: Gaurav K Singh
> Signed-off-by: Manasi Navare
> Co-developed-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/intel_dp.c | 7 +
> drivers/gpu/drm/i915/intel_drv.h | 4 +
> drivers/gpu/drm/i915/intel_vdsc.c | 455 ++
> include/drm/drm_dp_helper.h | 3 +
> 5 files changed, 471 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_vdsc.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index ef1480c14e4e..127efb2948a7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -154,7 +154,8 @@ i915-y += dvo_ch7017.o \
> intel_sdvo.o \
> intel_tv.o \
> vlv_dsi.o \
> - vlv_dsi_pll.o
> + vlv_dsi_pll.o \
> + intel_vdsc.o
>
> # Post-mortem debug and GPU hang state capture
> i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 05300d82b202..89990a96263b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2073,6 +2073,13 @@ static bool intel_dp_dsc_compute_config(struct
> intel_dp *intel_dp,
> return false;
> }
> }
> + if (intel_dp_compute_dsc_params(intel_dp, pipe_config) < 0) {
> + DRM_ERROR("Cannot compute valid DSC parameters for Input Bpp =
> %d"
> + "Compressed BPP = %d\n",
> + pipe_config->pipe_bpp,
> + pipe_config->dsc_params.compressed_bpp);
> + return false;
> + }
> pipe_config->dsc_params.compression_enable = true;
> DRM_DEBUG_KMS("DP DSC computed with Input Bpp = %d "
> "Compressed Bpp = %d Slice Count = %d\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 443afe97423a..36089c77e493 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1760,6 +1760,10 @@ uint16_t intel_dp_dsc_get_output_bpp(int link_clock,
> uint8_t lane_count,
> uint8_t intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, int
> mode_clock,
>int mode_hdisplay);
>
> +/* intel_vdsc.c */
> +int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> + struct intel_crtc_state *pipe_config);
> +
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> {
> return ~((1 << lane_count) - 1) & 0xf;
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c
>
Re: [PATCH v5 13/28] drm/i915/dp: Compute DSC pipe config in atomic check
Hi Jani,
This patch has a verbal ACK from you when we went over the patch together,
This is rebased on top of edp fast/narrow optimized config like we discussed.
could you please review this?
Regards
Manasi
On Fri, Oct 05, 2018 at 04:22:51PM -0700, Manasi Navare wrote:
> DSC params like the enable, compressed bpp, slice ocunt and
> dsc_split are added to the intel_crtc_state. These parameters
> are set based on the requested mode and available link parameters
> during the pipe configuration in atomic check phase.
> These values are then later used to populate the remaining DSC
> and RC parameters before enbaling DSC in atomic commit.
>
> v9:
> * Rebase on top of drm-tip that now uses fast_narrow config
> for edp (Manasi)
> v8:
> * Check for DSC bpc not 0 (manasi)
>
> v7:
> * Fix indentation in compute_m_n (Manasi)
>
> v6 (From Gaurav):
> * Remove function call of intel_dp_compute_dsc_params() and
> invoke intel_dp_compute_dsc_params() in the patch where
> it is defined to fix compilation warning (Gaurav)
>
> v5:
> Add drm_dsc_cfg in intel_crtc_state (Manasi)
>
> v4:
> * Rebase on refactoring of intel_dp_compute_config on tip (Manasi)
> * Add a comment why we need to check PSR while enabling DSC (Gaurav)
>
> v3:
> * Check PPR > max_cdclock to use 2 VDSC instances (Ville)
>
> v2:
> * Add if-else for eDP/DP (Gaurav)
>
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Anusha Srivatsa
> Cc: Gaurav K Singh
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_display.c | 20 +++-
> drivers/gpu/drm/i915/intel_display.h | 3 +-
> drivers/gpu/drm/i915/intel_dp.c | 170 ++-
> drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
> 4 files changed, 155 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 36434c5359b1..4ebf7c83085c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6532,7 +6532,7 @@ static int ironlake_fdi_compute_config(struct
> intel_crtc *intel_crtc,
>
> pipe_config->fdi_lanes = lane;
>
> - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> + intel_link_compute_m_n(pipe_config->pipe_bpp, 0, lane, fdi_dotclock,
> link_bw, _config->fdi_m_n, false);
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> @@ -6767,17 +6767,25 @@ static void compute_m_n(unsigned int m, unsigned int
> n,
> }
>
> void
> -intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> +intel_link_compute_m_n(int bits_per_pixel, uint16_t compressed_bpp,
> +int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> bool constant_n)
> {
> m_n->tu = 64;
>
> - compute_m_n(bits_per_pixel * pixel_clock,
> - link_clock * nlanes * 8,
> - _n->gmch_m, _n->gmch_n,
> - constant_n);
> + /* For DSC, Data M/N calculation uses compressed BPP */
> + if (compressed_bpp)
> + compute_m_n(compressed_bpp * pixel_clock,
> + link_clock * nlanes * 8,
> + _n->gmch_m, _n->gmch_n,
> + constant_n);
> + else
> + compute_m_n(bits_per_pixel * pixel_clock,
> + link_clock * nlanes * 8,
> + _n->gmch_m, _n->gmch_n,
> + constant_n);
>
> compute_m_n(pixel_clock, link_clock,
> _n->link_m, _n->link_n,
> diff --git a/drivers/gpu/drm/i915/intel_display.h
> b/drivers/gpu/drm/i915/intel_display.h
> index 9fac67e31205..9eaba1bccae8 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -402,7 +402,8 @@ struct intel_link_m_n {
>(__i)++) \
> for_each_if(plane)
>
> -void intel_link_compute_m_n(int bpp, int nlanes,
> +void intel_link_compute_m_n(int bpp, uint16_t compressed_bpp,
> + int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> bool constant_n);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8e6891356d5b..05300d82b202 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -47,6 +47,8 @@
>
> /* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */
> #define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER 61440
> +#define DP_DSC_MIN_SUPPORTED_BPC 8
> +#define DP_DSC_MAX_SUPPORTED_BPC 10
>
> /* DP DSC throughput values used for slice count calculations KPixels/s */
> #define DP_DSC_PEAK_PIXEL_RATE 272
> @@ -1894,6 +1896,16 @@ static int intel_dp_compute_bpp(struct intel_dp
> *intel_dp,
> }
>
Re: [PATCH v5 11/28] drm/dsc: Add helpers for DSC picture parameter set infoframes
Hi Jani,
This patch adds the cpu_to_be16 macro and removes the bitfields and
uses macros instead for packing the infoframe as per your feedback
on the previous version of the patch.
Could you please review this patch?
Regards
Manasi
On Fri, Oct 05, 2018 at 04:22:49PM -0700, Manasi Navare wrote:
> According to Display Stream compression spec 1.2, the picture
> parameter set metadata is sent from source to sink device
> using the DP Secondary data packet. An infoframe is formed
> for the PPS SDP header and PPS SDP payload bytes.
> This patch adds helpers to fill the PPS SDP header
> and PPS SDP payload according to the DSC 1.2 specification.
>
> v6:
> * Use proper sequence points for breaking down the
> assignments (Chris Wilson)
> * Use SPDX identifier
> v5:
> Do not use bitfields for DRM structs (Jani N)
> v4:
> * Use DSC constants for params that dont change across
> configurations
> v3:
> * Add reference to added kernel-docs in Documentation/gpu/drm-kms-helpers.rst
> (Daniel Vetter)
>
> v2:
> * Add EXPORT_SYMBOL for the drm functions (Manasi)
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Anusha Srivatsa
> Cc: Harry Wentland
> Signed-off-by: Manasi Navare
> Acked-by: Harry Wentland
> ---
> Documentation/gpu/drm-kms-helpers.rst | 12 ++
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_dsc.c | 223 ++
> include/drm/drm_dsc.h | 22 +++
> 4 files changed, 258 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/drm_dsc.c
>
> diff --git a/Documentation/gpu/drm-kms-helpers.rst
> b/Documentation/gpu/drm-kms-helpers.rst
> index f9cfcdcdf024..50bb71712f82 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -223,6 +223,18 @@ MIPI DSI Helper Functions Reference
> .. kernel-doc:: drivers/gpu/drm/drm_mipi_dsi.c
> :export:
>
> +Display Stream Compression Helper Functions Reference
> +=
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_dsc.c
> + :doc: dsc helpers
> +
> +.. kernel-doc:: include/drm/drm_dsc.h
> + :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_dsc.c
> + :export:
> +
> Output Probing Helper Functions Reference
> =
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index bc6a16a3c36e..8e310fadb95d 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -32,7 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
> drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>
> -drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> +drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o
> drm_probe_helper.o \
> drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
> drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
> drm_simple_kms_helper.o drm_modeset_helper.o \
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> new file mode 100644
> index ..21ae8d015afd
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2018 Intel Corp
> + *
> + * Author:
> + * Manasi Navare
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * DOC: dsc helpers
> + *
> + * These functions contain some common logic and helpers to deal with VESA
> + * Display Stream Compression standard required for DSC on Display Port/eDP
> or
> + * MIPI display interfaces.
> + */
> +
> +/**
> + * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
> + * for DisplayPort as per the DP 1.4 spec.
> + * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
> + */
> +void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
> +{
> + memset(_sdp->pps_header, 0, sizeof(pps_sdp->pps_header));
> +
> + pps_sdp->pps_header.HB1 = DP_SDP_PPS;
> + pps_sdp->pps_header.HB2 = DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1;
> +}
> +EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
> +
> +/**
> + * drm_dsc_pps_infoframe_pack() - Populates the DSC PPS infoframe
> + * using the DSC configuration parameters in the order expected
> + * by the DSC Display Sink device. For the DSC, the sink device
> + * expects the PPS payload in the big endian format for the fields
> + * that span more than 1 byte.
> + *
> + * @pps_sdp:
> + * Secondary data packet for DSC Picture Parameter Set
> + * @dsc_cfg:
> + * DSC Configuration data filled by driver
> + */
> +void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
> + struct drm_dsc_config *dsc_cfg)
> +{
> + u8 i = 0;
> +
> + memset(_sdp->pps_payload, 0, sizeof(pps_sdp->pps_payload));
> +
> + /* PPS 0 */
> +
Re: [PATCH v5 26/28] drm/i915/dsc: Enable and disable appropriate power wells for VDSC
Hi Ville,
This adds a helper function to get the power well as per
the transcoder as per your suggestion.
Could you please review this one?
Regards
Manasi
On Fri, Oct 05, 2018 at 04:23:04PM -0700, Manasi Navare wrote:
> A separate power well 2 (PG2) is required for VDSC on eDP transcoder
> whereas all other transcoders use the power wells associated with the
> transcoders for VDSC.
> This patch adds a helper to obtain correct power domain depending on
> transcoder being used and enables/disables the power wells during
> VDSC enabling/disabling.
>
> Suggested-by: Ville Syrjala
> Cc: Ville Syrjala
> Cc: Imre Deak
> Cc: Rodrigo Vivi
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_vdsc.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c
> b/drivers/gpu/drm/i915/intel_vdsc.c
> index 4963e80a87f0..d2b4601459c3 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -581,6 +581,23 @@ int intel_dp_compute_dsc_params(struct intel_dp
> *intel_dp,
> return 0;
> }
>
> +static enum intel_display_power_domain
> +intel_dsc_get_power_domains(struct intel_crtc_state *crtc_state)
> +{
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +
> + /*
> + * On ICL+ PW2/ POWER_DOMAIN_VDSC_PIPE_A is required for
> + * VDSC/joining for eDP transcoder.
> + * For any other transcoder, VDSC/joining uses the power well
> associated
> + * with the pipe/transcoder in use.
> + */
> + if (cpu_transcoder == TRANSCODER_EDP)
> + return POWER_DOMAIN_VDSC_PIPE_A;
> + else
> + return POWER_DOMAIN_TRANSCODER(cpu_transcoder);
> +}
> +
> static void intel_configure_pps_for_dsc_encoder(struct intel_encoder
> *encoder,
> struct intel_crtc_state
> *crtc_state)
> {
> @@ -1019,6 +1036,10 @@ void intel_dsc_enable(struct intel_encoder *encoder,
> if (!crtc_state->dsc_params.compression_enable)
> return;
>
> + /* Enable Power wells for VDSC/joining */
> + intel_display_power_get(dev_priv,
> + intel_dsc_get_power_domains(crtc_state));
> +
> intel_configure_pps_for_dsc_encoder(encoder, crtc_state);
>
> intel_dp_send_dsc_pps_sdp(encoder, crtc_state);
> @@ -1073,4 +1094,8 @@ void intel_dsc_disable(struct intel_encoder *encoder,
> RIGHT_BRANCH_VDSC_ENABLE);
> I915_WRITE(dss_ctl2_reg, dss_ctl2_val);
>
> + /* Disable Power wells for VDSC/joining */
> + intel_display_power_put(dev_priv,
> + intel_dsc_get_power_domains(old_crtc_state));
> +
> }
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 19/28] drm/i915/dsc: Add a power domain for VDSC on eDP/MIPI DSI
Hi Imre/Ville,
This patch adds the power domain as per our discussion and feedback
on previous patch set.
Could you please take a look at this?
Manasi
On Fri, Oct 05, 2018 at 04:22:57PM -0700, Manasi Navare wrote:
> On Icelake, a separate power well PG2 is created for
> VDSC engine used for eDP/MIPI DSI. This patch adds a new
> display power domain for Power well 2.
>
> v2:
> * Fix the power well mismatch CI error (Ville)
> * Rename as VDSC_PIPE_A (Imre)
> * Fix a whitespace (Anusha)
> * Fix Comments (Imre)
>
> Cc: Ville Syrjala
> Cc: Rodrigo Vivi
> Cc: Imre Deak
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_display.h| 1 +
> drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.h
> b/drivers/gpu/drm/i915/intel_display.h
> index 9eaba1bccae8..4c513169960c 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -256,6 +256,7 @@ enum intel_display_power_domain {
> POWER_DOMAIN_MODESET,
> POWER_DOMAIN_GT_IRQ,
> POWER_DOMAIN_INIT,
> + POWER_DOMAIN_VDSC_PIPE_A,
>
> POWER_DOMAIN_NUM,
> };
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 3cf8533e0834..3ed0a3a1015a 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum
> intel_display_power_domain domain)
> return "MODESET";
> case POWER_DOMAIN_GT_IRQ:
> return "GT_IRQ";
> + case POWER_DOMAIN_VDSC_PIPE_A:
> + return "VDSC_PIPE_A";
> default:
> MISSING_CASE(domain);
> return "?";
> @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private
> *dev_priv,
>*/
> #define ICL_PW_2_POWER_DOMAINS ( \
> ICL_PW_3_POWER_DOMAINS |\
> + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \
> BIT_ULL(POWER_DOMAIN_INIT))
> /*
> - * - eDP/DSI VDSC
>* - KVMR (HW control)
>*/
> #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v3]
On Mon, Oct 15, 2018 at 6:05 PM Keith Packard wrote:
> Offers three clocks, device, clock monotonic and clock monotonic
> raw. Could use some kernel support to reduce the deviation between
> clock values.
>
> v2:
> Ensure deviation is at least as big as the GPU time interval.
>
> v3:
> Set device->lost when returning DEVICE_LOST.
> Use MAX2 and DIV_ROUND_UP instead of open coding these.
> Delete spurious TIMESTAMP in radv version.
> Suggested-by: Jason Ekstrand
> Suggested-by: Lionel Landwerlin
>
> Signed-off-by: Keith Packard
> ---
> src/amd/vulkan/radv_device.c | 81 +++
> src/amd/vulkan/radv_extensions.py | 1 +
> src/intel/vulkan/anv_device.c | 89 ++
> src/intel/vulkan/anv_extensions.py | 1 +
> src/intel/vulkan/anv_gem.c | 13 +
> src/intel/vulkan/anv_private.h | 2 +
> 6 files changed, 187 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 174922780fc..80050485e54 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -4955,3 +4955,84 @@ radv_GetDeviceGroupPeerMemoryFeatures(
>VK_PEER_MEMORY_FEATURE_GENERIC_SRC_BIT |
>VK_PEER_MEMORY_FEATURE_GENERIC_DST_BIT;
> }
> +
> +static const VkTimeDomainEXT radv_time_domains[] = {
> + VK_TIME_DOMAIN_DEVICE_EXT,
> + VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT,
> + VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT,
> +};
> +
> +VkResult radv_GetPhysicalDeviceCalibrateableTimeDomainsEXT(
> + VkPhysicalDevice physicalDevice,
> + uint32_t *pTimeDomainCount,
> + VkTimeDomainEXT *pTimeDomains)
> +{
> + int d;
> + VK_OUTARRAY_MAKE(out, pTimeDomains, pTimeDomainCount);
> +
> + for (d = 0; d < ARRAY_SIZE(radv_time_domains); d++) {
> + vk_outarray_append(, i) {
> + *i = radv_time_domains[d];
> + }
> + }
> +
> + return vk_outarray_status();
> +}
> +
> +static uint64_t
> +radv_clock_gettime(clockid_t clock_id)
> +{
> + struct timespec current;
> + int ret;
> +
> + ret = clock_gettime(clock_id, );
> + if (ret < 0 && clock_id == CLOCK_MONOTONIC_RAW)
> + ret = clock_gettime(CLOCK_MONOTONIC, );
> + if (ret < 0)
> + return 0;
> +
> + return (uint64_t) current.tv_sec * 10ULL + current.tv_nsec;
> +}
> +
> +VkResult radv_GetCalibratedTimestampsEXT(
> + VkDevice _device,
> + uint32_t timestampCount,
> + const VkCalibratedTimestampInfoEXT *pTimestampInfos,
> + uint64_t *pTimestamps,
> + uint64_t *pMaxDeviation)
> +{
> + RADV_FROM_HANDLE(radv_device, device, _device);
> + uint32_t clock_crystal_freq =
> device->physical_device->rad_info.clock_crystal_freq;
> + int d;
> + uint64_t begin, end;
> +
> + begin = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
> +
> + for (d = 0; d < timestampCount; d++) {
> + switch (pTimestampInfos[d].timeDomain) {
> + case VK_TIME_DOMAIN_DEVICE_EXT:
> + pTimestamps[d] =
> device->ws->query_value(device->ws,
> +
> RADEON_TIMESTAMP);
> + break;
> + case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
> + pTimestamps[d] =
> radv_clock_gettime(CLOCK_MONOTONIC);
> + break;
> +
> + case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
> + pTimestamps[d] = begin;
> + break;
> + default:
> + pTimestamps[d] = 0;
> + break;
> + }
> + }
> +
> + end = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
> +
> + uint64_t clock_period = end - begin;
> + uint64_t device_period = DIV_ROUND_UP(100, clock_crystal_freq);
> +
> + *pMaxDeviation = MAX2(clock_period, device_period);
> +
> + return VK_SUCCESS;
> +}
> diff --git a/src/amd/vulkan/radv_extensions.py
> b/src/amd/vulkan/radv_extensions.py
> index 5dcedae1c63..4c81d3f0068 100644
> --- a/src/amd/vulkan/radv_extensions.py
> +++ b/src/amd/vulkan/radv_extensions.py
> @@ -92,6 +92,7 @@ EXTENSIONS = [
> Extension('VK_KHR_display', 23,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
> Extension('VK_EXT_direct_mode_display', 1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
> Extension('VK_EXT_acquire_xlib_display', 1,
> 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
> +Extension('VK_EXT_calibrated_timestamps', 1, True),
> Extension('VK_EXT_conditional_rendering',
[PATCH libdrm 2/2] gitignore: add _build
This is the directory used by meson/autotools (at least in the .gitlab-ci configuration) so ignore the whole dir. Signed-off-by: Lucas De Marchi --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 49cced50..54365c7c 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ Makefile Makefile.in TAGS +_build aclocal.m4 autom4te.cache bsd-core/*/@ -- 2.19.1.1.g8c3cf03f71 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/2] gitignore: sort file
LANG=C sort -u .gitignore | sponge .gitignore This way it's easier to keep track of the entries. Signed-off-by: Lucas De Marchi --- .gitignore | 56 +++--- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index d51e619b..49cced50 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,3 @@ -bsd-core/*/@ -bsd-core/*/machine -*~ *.1 *.3 *.5 @@ -17,17 +14,20 @@ bsd-core/*/machine *.o.cmd *.sw? *.trs +*~ +.*check* +.*install* .depend .deps .libs .tmp_versions -.*check* -.*install* Makefile Makefile.in TAGS aclocal.m4 autom4te.cache +bsd-core/*/@ +bsd-core/*/machine build-aux bus_if.h compile @@ -47,21 +47,22 @@ drm_pciids.h export_syms i915.kld install-sh -libdrm/config.h.in libdrm.pc +libdrm/config.h.in +libdrm_amdgpu.pc +libdrm_etnaviv.pc +libdrm_exynos.pc +libdrm_freedreno.pc libdrm_intel.pc libdrm_nouveau.pc -libdrm_radeon.pc libdrm_omap.pc -libdrm_exynos.pc -libdrm_freedreno.pc -libdrm_amdgpu.pc +libdrm_radeon.pc libdrm_vc4.pc -libdrm_etnaviv.pc libkms.pc libtool ltmain.sh mach64.kld +man/*.3 man/.man_fixup mga.kld missing @@ -74,35 +75,34 @@ savage.kld sis.kld stamp-h1 tdfx.kld -via.kld -tests/auth tests/amdgpu/amdgpu_test +tests/auth tests/dristat tests/drmdevice tests/drmsl tests/drmstat +tests/etnaviv/etnaviv_2d_test +tests/etnaviv/etnaviv_bo_cache_test +tests/etnaviv/etnaviv_cmd_stream_test +tests/exynos/exynos_fimg2d_event +tests/exynos/exynos_fimg2d_perf +tests/exynos/exynos_fimg2d_test tests/getclient tests/getstats tests/getversion tests/hash +tests/kms/kms-steal-crtc +tests/kms/kms-universal-planes +tests/kmstest/kmstest tests/lock -tests/openclose -tests/random -tests/setversion -tests/updatedraw tests/modeprint/modeprint tests/modetest/modetest tests/name_from_fd +tests/openclose tests/proptest/proptest -tests/kms/kms-steal-crtc -tests/kms/kms-universal-planes -tests/kmstest/kmstest -tests/vbltest/vbltest tests/radeon/radeon_ttm -tests/exynos/exynos_fimg2d_event -tests/exynos/exynos_fimg2d_perf -tests/exynos/exynos_fimg2d_test -tests/etnaviv/etnaviv_2d_test -tests/etnaviv/etnaviv_cmd_stream_test -tests/etnaviv/etnaviv_bo_cache_test -man/*.3 +tests/random +tests/setversion +tests/updatedraw +tests/vbltest/vbltest +via.kld -- 2.19.1.1.g8c3cf03f71 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: analogix_dp: Fix misleading indentation reported by smatch
Hi Enric,
Thank you for the patch.
On Saturday, 13 October 2018 14:18:44 EEST Enric Balletbo i Serra wrote:
> This patch avoids that building the bridge/analogix source code with
> smatch triggers complaints about inconsistent indenting.
>
> Signed-off-by: Enric Balletbo i Serra
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index
> d68986cea132..75e2649d7515 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1219,12 +1219,12 @@ static int analogix_dp_bridge_attach(struct
> drm_bridge *bridge) * plat_data->attch return, that's why we record the
> connector
>* point after plat attached.
>*/
> - if (dp->plat_data->attach) {
> - ret = dp->plat_data->attach(dp->plat_data, bridge, connector);
> - if (ret) {
> - DRM_ERROR("Failed at platform attch func\n");
> - return ret;
> - }
> + if (dp->plat_data->attach) {
> + ret = dp->plat_data->attach(dp->plat_data, bridge, connector);
> + if (ret) {
> + DRM_ERROR("Failed at platform attch func\n");
> + return ret;
> + }
> }
This looks good to me. While at it, could you s/attch/attach/ ? With that
fixed,
Reviewed-by: Laurent Pinchart
> if (dp->plat_data->panel) {
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v3]
Offers three clocks, device, clock monotonic and clock monotonic
raw. Could use some kernel support to reduce the deviation between
clock values.
v2:
Ensure deviation is at least as big as the GPU time interval.
v3:
Set device->lost when returning DEVICE_LOST.
Use MAX2 and DIV_ROUND_UP instead of open coding these.
Delete spurious TIMESTAMP in radv version.
Suggested-by: Jason Ekstrand
Suggested-by: Lionel Landwerlin
Signed-off-by: Keith Packard
---
src/amd/vulkan/radv_device.c | 81 +++
src/amd/vulkan/radv_extensions.py | 1 +
src/intel/vulkan/anv_device.c | 89 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_gem.c | 13 +
src/intel/vulkan/anv_private.h | 2 +
6 files changed, 187 insertions(+)
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 174922780fc..80050485e54 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -4955,3 +4955,84 @@ radv_GetDeviceGroupPeerMemoryFeatures(
VK_PEER_MEMORY_FEATURE_GENERIC_SRC_BIT |
VK_PEER_MEMORY_FEATURE_GENERIC_DST_BIT;
}
+
+static const VkTimeDomainEXT radv_time_domains[] = {
+ VK_TIME_DOMAIN_DEVICE_EXT,
+ VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT,
+ VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT,
+};
+
+VkResult radv_GetPhysicalDeviceCalibrateableTimeDomainsEXT(
+ VkPhysicalDevice physicalDevice,
+ uint32_t *pTimeDomainCount,
+ VkTimeDomainEXT *pTimeDomains)
+{
+ int d;
+ VK_OUTARRAY_MAKE(out, pTimeDomains, pTimeDomainCount);
+
+ for (d = 0; d < ARRAY_SIZE(radv_time_domains); d++) {
+ vk_outarray_append(, i) {
+ *i = radv_time_domains[d];
+ }
+ }
+
+ return vk_outarray_status();
+}
+
+static uint64_t
+radv_clock_gettime(clockid_t clock_id)
+{
+ struct timespec current;
+ int ret;
+
+ ret = clock_gettime(clock_id, );
+ if (ret < 0 && clock_id == CLOCK_MONOTONIC_RAW)
+ ret = clock_gettime(CLOCK_MONOTONIC, );
+ if (ret < 0)
+ return 0;
+
+ return (uint64_t) current.tv_sec * 10ULL + current.tv_nsec;
+}
+
+VkResult radv_GetCalibratedTimestampsEXT(
+ VkDevice _device,
+ uint32_t timestampCount,
+ const VkCalibratedTimestampInfoEXT *pTimestampInfos,
+ uint64_t *pTimestamps,
+ uint64_t *pMaxDeviation)
+{
+ RADV_FROM_HANDLE(radv_device, device, _device);
+ uint32_t clock_crystal_freq =
device->physical_device->rad_info.clock_crystal_freq;
+ int d;
+ uint64_t begin, end;
+
+ begin = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
+
+ for (d = 0; d < timestampCount; d++) {
+ switch (pTimestampInfos[d].timeDomain) {
+ case VK_TIME_DOMAIN_DEVICE_EXT:
+ pTimestamps[d] = device->ws->query_value(device->ws,
+
RADEON_TIMESTAMP);
+ break;
+ case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
+ pTimestamps[d] = radv_clock_gettime(CLOCK_MONOTONIC);
+ break;
+
+ case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
+ pTimestamps[d] = begin;
+ break;
+ default:
+ pTimestamps[d] = 0;
+ break;
+ }
+ }
+
+ end = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
+
+ uint64_t clock_period = end - begin;
+ uint64_t device_period = DIV_ROUND_UP(100, clock_crystal_freq);
+
+ *pMaxDeviation = MAX2(clock_period, device_period);
+
+ return VK_SUCCESS;
+}
diff --git a/src/amd/vulkan/radv_extensions.py
b/src/amd/vulkan/radv_extensions.py
index 5dcedae1c63..4c81d3f0068 100644
--- a/src/amd/vulkan/radv_extensions.py
+++ b/src/amd/vulkan/radv_extensions.py
@@ -92,6 +92,7 @@ EXTENSIONS = [
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
+Extension('VK_EXT_calibrated_timestamps', 1, True),
Extension('VK_EXT_conditional_rendering', 1, True),
Extension('VK_EXT_conservative_rasterization',1,
'device->rad_info.chip_class >= GFX9'),
Extension('VK_EXT_display_surface_counter', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
diff --git
Re: [PATCH 4/5] drm: rcar-du: Add R8A7744 support
Hi Fabrizio,
Thank you for the patch.
On Friday, 21 September 2018 21:08:30 EEST Fabrizio Castro wrote:
> From: Biju Das
>
> Add support for the R8A7744 DU (which is very similar to the R8A7743 DU);
> it has 1 DPAD (RGB) output and 1 LVDS output.
>
> Signed-off-by: Biju Das
> Reviewed-by: Fabrizio Castro
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index c07d3f1..2c3d0e5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -321,6 +321,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a77970_info = {
>
> static const struct of_device_id rcar_du_of_table[] = {
> { .compatible = "renesas,du-r8a7743", .data = _du_r8a7743_info },
> + { .compatible = "renesas,du-r8a7744", .data = _du_r8a7743_info },
> { .compatible = "renesas,du-r8a7745", .data = _du_r8a7745_info },
> { .compatible = "renesas,du-r8a77470", .data = _du_r8a77470_info },
> { .compatible = "renesas,du-r8a7779", .data = _du_r8a7779_info },
This looks good to me. I would also apply this change:
@@ -41,7 +41,7 @@ static const struct rcar_du_device_info rzg1_du_r8a7743_info
= {
.channels_mask = BIT(1) | BIT(0),
.routes = {
/*
-* R8A7743 has one RGB output and one LVDS output
+* R8A774[34] has one RGB output and one LVDS output
*/
[RCAR_DU_OUTPUT_DPAD0] = {
.possible_crtcs = BIT(1) | BIT(0),
With this,
Reviewed-by: Laurent Pinchart
There's no need to resubmit, I've applied the patch to my tree with the above
change.
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: rcar-du: Add r8a77470 support
Hi Fabrizio,
Thank you for the patch.
On Friday, 21 September 2018 21:08:29 EEST Fabrizio Castro wrote:
> Add RZ/G1C (a.k.a. r8a77470) support to the R-Car DU driver.
>
> Signed-off-by: Fabrizio Castro
> Reviewed-by: Biju Das
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 26 ++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 02aee6c..c07d3f1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -77,6 +77,31 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7745_info = { },
> };
>
> +static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
> + .gen = 2,
> + .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + | RCAR_DU_FEATURE_EXT_CTRL_REGS,
The DU driver has recently gained two new feature flags,
RCAR_DU_FEATURE_INTERLACED and RCAR_DU_FEATURE_TVM_SYNC. I believe that both
should be set here. Could you please confirm (and test) ?
> + .channels_mask = BIT(1) | BIT(0),
> + .routes = {
> + /*
> + * R8A77470 has two RGB outputs, one LVDS output, and
> + * one analog video output (unsupported)
I'd write this "and one (currently unsupported) analog video output" to match
the other device entries. With this fixed,
Reviewed-by: Laurent Pinchart
> + */
> + [RCAR_DU_OUTPUT_DPAD0] = {
> + .possible_crtcs = BIT(0),
> + .port = 0,
> + },
> + [RCAR_DU_OUTPUT_DPAD1] = {
> + .possible_crtcs = BIT(1),
> + .port = 1,
> + },
> + [RCAR_DU_OUTPUT_LVDS0] = {
> + .possible_crtcs = BIT(0) | BIT(1),
> + .port = 2,
> + },
> + },
> +};
> +
> static const struct rcar_du_device_info rcar_du_r8a7779_info = {
> .gen = 2,
> .features = 0,
> @@ -297,6 +322,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a77970_info = { static const struct of_device_id
> rcar_du_of_table[] = {
> { .compatible = "renesas,du-r8a7743", .data = _du_r8a7743_info },
> { .compatible = "renesas,du-r8a7745", .data = _du_r8a7745_info },
> + { .compatible = "renesas,du-r8a77470", .data = _du_r8a77470_info },
> { .compatible = "renesas,du-r8a7779", .data = _du_r8a7779_info },
> { .compatible = "renesas,du-r8a7790", .data = _du_r8a7790_info },
> { .compatible = "renesas,du-r8a7791", .data = _du_r8a7791_info },
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] dt-bindings: display: renesas: du: Document the r8a7744 bindings
Hi Fabrizio, Thank you for the patch. On Friday, 21 September 2018 21:08:28 EEST Fabrizio Castro wrote: > From: Biju Das > > Document the RZ/G1N (R8A7744) SoC in the R-Car DU bindings. > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro Reviewed-by: Laurent Pinchart and taken in my tree. > --- > Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt > b/Documentation/devicetree/bindings/display/renesas,du.txt index > 2b28ae2..aad9cf1 100644 > --- a/Documentation/devicetree/bindings/display/renesas,du.txt > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt > @@ -4,6 +4,7 @@ Required Properties: > >- compatible: must be one of the following. > - "renesas,du-r8a7743" for R8A7743 (RZ/G1M) compatible DU > +- "renesas,du-r8a7744" for R8A7744 (RZ/G1N) compatible DU > - "renesas,du-r8a7745" for R8A7745 (RZ/G1E) compatible DU > - "renesas,du-r8a77470" for R8A77470 (RZ/G1C) compatible DU > - "renesas,du-r8a7779" for R8A7779 (R-Car H1) compatible DU > @@ -51,6 +52,7 @@ corresponding to each DU output. > Port0 Port1 Port2 Port3 > --- > -- R8A7743 (RZ/G1M) DPAD 0 LVDS 0 - - + > R8A7744 (RZ/G1N) DPAD 0 LVDS 0 - - > R8A7745 (RZ/G1E) DPAD 0 DPAD 1 - - > R8A77470 (RZ/G1C) DPAD 0 DPAD 1 LVDS 0 - > R8A7779 (R-Car H1) DPAD 0 DPAD 1 - - -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] dt-bindings: display: renesas: du: Document the r8a77470 bindings
Hi Fabrizio, Thank you for the patch. On Friday, 21 September 2018 21:08:27 EEST Fabrizio Castro wrote: > Document the RZ/G1C (r8a77470) SoC in R-Car DU bindings. > > Signed-off-by: Fabrizio Castro > Reviewed-by: Biju Das Reviewed-by: Laurent Pinchart and taken in my tree. > --- > Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt > b/Documentation/devicetree/bindings/display/renesas,du.txt index > ec9d34be..2b28ae2 100644 > --- a/Documentation/devicetree/bindings/display/renesas,du.txt > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt > @@ -5,6 +5,7 @@ Required Properties: >- compatible: must be one of the following. > - "renesas,du-r8a7743" for R8A7743 (RZ/G1M) compatible DU > - "renesas,du-r8a7745" for R8A7745 (RZ/G1E) compatible DU > +- "renesas,du-r8a77470" for R8A77470 (RZ/G1C) compatible DU > - "renesas,du-r8a7779" for R8A7779 (R-Car H1) compatible DU > - "renesas,du-r8a7790" for R8A7790 (R-Car H2) compatible DU > - "renesas,du-r8a7791" for R8A7791 (R-Car M2-W) compatible DU > @@ -51,6 +52,7 @@ corresponding to each DU output. > --- > -- R8A7743 (RZ/G1M) DPAD 0 LVDS 0 - - > R8A7745 (RZ/G1E) DPAD 0 DPAD 1 - - + > R8A77470 (RZ/G1C) DPAD 0 DPAD 1 LVDS 0 - > R8A7779 (R-Car H1) DPAD 0 DPAD 1 - - > R8A7790 (R-Car H2) DPAD 0 LVDS 0 LVDS 1 - > R8A7791 (R-Car M2-W) DPAD 0 LVDS 0 - - -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 02/18] drm: Add a new helper to validate damage during atomic_check
On Wed, Oct 10, 2018 at 05:16:41PM -0700, Deepak Rawat wrote:
> This helper function makes sure that damage from plane state is
> discarded for full modeset cycle. For some reason, which makes damage
> irrelevant, driver might want to do a full plane update for e.g. full
> modeset. Such cases must be checked here.
>
> Cc: ville.syrj...@linux.intel.com
> Cc: Daniel Vetter
> Cc: Pekka Paalanen
> Cc: Daniel Stone
> Signed-off-by: Deepak Rawat
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 3 +++
> drivers/gpu/drm/drm_damage_helper.c | 38 +
> include/drm/drm_damage_helper.h | 2 ++
> 3 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 35395577ca86..41dabb817c57 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "drm_crtc_helper_internal.h"
> @@ -828,6 +829,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>
> drm_atomic_helper_plane_changed(state, old_plane_state,
> new_plane_state, plane);
>
> + drm_atomic_helper_check_plane_damage(state, new_plane_state);
> +
> if (!funcs || !funcs->atomic_check)
> continue;
>
> diff --git a/drivers/gpu/drm/drm_damage_helper.c
> b/drivers/gpu/drm/drm_damage_helper.c
> index 8dc906a489a9..c130514bbb21 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -29,6 +29,7 @@
> *
> **/
>
> +#include
> #include
>
> /**
> @@ -81,3 +82,40 @@ void drm_plane_enable_fb_damage_clips(struct drm_plane
> *plane)
> 0);
> }
> EXPORT_SYMBOL(drm_plane_enable_fb_damage_clips);
> +
> +/**
> + * drm_atomic_helper_check_plane_damage - Verify plane damage on
> atomic_check.
> + * @state: The driver state object.
> + * @plane_state: Plane state for which to verify damage.
> + *
> + * This helper function makes sure that damage from plane state is discarded
> + * for full modeset cycle. For some reason, which makes damage irrelevant,
remove "cycle" ^ ^Reads weird.. maybe...
"If there are more reasons a driver would want to do a full plane
update rather than processing individual damage regions, then
those cases should be taken care of here."
> + * driver might want to do a full plane update for e.g. full modeset. Such
> + * cases must be checked here. Note that NULL
> _plane_state.fb_damage_clips
^ drm_plane_state.fb_damage_clips
== NULL
> + * in plane state means that full update should happen. It also ensue helper
^ ensure(?)
that the
> + * iterator to return _plane_state.src as damage.
^s/to/will
> + *
> + * Currently this helper discard damage during full modeset only. This is
^ discards
The rest of the paragraph reads kind of strange, and pretty much repeats
what has already been said, so maybe just remove it?
> + * because, presently only vmwgfx exposes damage interface, which need full
> + * plane update during full modeset only. As more driver add damage support,
> + * should any state change need full plane update, must be added here.
> + */
> +void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state,
> + struct drm_plane_state *plane_state)
> +{
> + struct drm_crtc_state *crtc_state;
> +
> + if (plane_state->crtc) {
> + crtc_state = drm_atomic_get_new_crtc_state(state,
> +plane_state->crtc);
> +
> + if (WARN_ON(!crtc_state))
> + return;
> +
> + if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> + drm_property_blob_put(plane_state->fb_damage_clips);
> + plane_state->fb_damage_clips = NULL;
> + }
> + }
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage);
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> index 4947c614fff9..59584cbf3d40 100644
> --- a/include/drm/drm_damage_helper.h
> +++ b/include/drm/drm_damage_helper.h
> @@ -35,5 +35,7 @@
> #include
>
> void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> +void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state,
> + struct drm_plane_state *plane_state);
>
> #endif
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108317] [Polaris] Black Textures only on Polaris in Cemu Zelda Breath of the Wild
https://bugs.freedesktop.org/show_bug.cgi?id=108317 --- Comment #8 from John Galt --- I've found evidence of Polaris users without this issue on llvm 6 + mesa mild. However, my attempts at downgrading and building with llvm 6 and current mesa haven't gone well yet. At least we know this is probably a regression in llvm 7+ -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v2]
On 15/10/2018 22:22, Keith Packard wrote: +#define TIMESTAMP 0x2358 + +VkResult radv_GetCalibratedTimestampsEXT( Heh, I think you copied that define over from Anv ;) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v2]
On Mon, Oct 15, 2018 at 4:22 PM Keith Packard wrote:
> Offers three clocks, device, clock monotonic and clock monotonic
> raw. Could use some kernel support to reduce the deviation between
> clock values.
>
> v2:
> Ensure deviation is at least as big as the GPU time interval.
>
> Signed-off-by: Keith Packard
> ---
> src/amd/vulkan/radv_device.c | 84
> src/amd/vulkan/radv_extensions.py | 1 +
> src/intel/vulkan/anv_device.c | 88 ++
> src/intel/vulkan/anv_extensions.py | 1 +
> src/intel/vulkan/anv_gem.c | 13 +
> src/intel/vulkan/anv_private.h | 2 +
> 6 files changed, 189 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 174922780fc..29f0afbc69b 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -4955,3 +4955,87 @@ radv_GetDeviceGroupPeerMemoryFeatures(
>VK_PEER_MEMORY_FEATURE_GENERIC_SRC_BIT |
>VK_PEER_MEMORY_FEATURE_GENERIC_DST_BIT;
> }
> +
> +static const VkTimeDomainEXT radv_time_domains[] = {
> + VK_TIME_DOMAIN_DEVICE_EXT,
> + VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT,
> + VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT,
> +};
> +
> +VkResult radv_GetPhysicalDeviceCalibrateableTimeDomainsEXT(
> + VkPhysicalDevice physicalDevice,
> + uint32_t *pTimeDomainCount,
> + VkTimeDomainEXT *pTimeDomains)
> +{
> + int d;
> + VK_OUTARRAY_MAKE(out, pTimeDomains, pTimeDomainCount);
> +
> + for (d = 0; d < ARRAY_SIZE(radv_time_domains); d++) {
> + vk_outarray_append(, i) {
> + *i = radv_time_domains[d];
> + }
> + }
> +
> + return vk_outarray_status();
> +}
> +
> +static uint64_t
> +radv_clock_gettime(clockid_t clock_id)
> +{
> + struct timespec current;
> + int ret;
> +
> + ret = clock_gettime(clock_id, );
> + if (ret < 0 && clock_id == CLOCK_MONOTONIC_RAW)
> + ret = clock_gettime(CLOCK_MONOTONIC, );
> + if (ret < 0)
> + return 0;
> +
> + return (uint64_t) current.tv_sec * 10ULL + current.tv_nsec;
> +}
> +
> +#define TIMESTAMP 0x2358
> +
> +VkResult radv_GetCalibratedTimestampsEXT(
> + VkDevice _device,
> + uint32_t timestampCount,
> + const VkCalibratedTimestampInfoEXT *pTimestampInfos,
> + uint64_t *pTimestamps,
> + uint64_t *pMaxDeviation)
> +{
> + RADV_FROM_HANDLE(radv_device, device, _device);
> + uint32_t clock_crystal_freq =
> device->physical_device->rad_info.clock_crystal_freq;
> + int d;
> + uint64_t begin, end;
> +
> + begin = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
> +
> + for (d = 0; d < timestampCount; d++) {
> + switch (pTimestampInfos[d].timeDomain) {
> + case VK_TIME_DOMAIN_DEVICE_EXT:
> + /* XXX older kernels don't support this interface.
> */
> + pTimestamps[d] =
> device->ws->query_value(device->ws,
> +
> RADEON_TIMESTAMP);
> + break;
> + case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
> + pTimestamps[d] =
> radv_clock_gettime(CLOCK_MONOTONIC);
> + break;
> +
> + case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
> + pTimestamps[d] = begin;
> + break;
> + default:
> + pTimestamps[d] = 0;
> + break;
> + }
> + }
> +
> + end = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
> +
> + uint64_t clock_period = end - begin;
> + uint64_t device_period = (100 + clock_crystal_freq - 1) /
> clock_crystal_freq;
> +
> + *pMaxDeviation = clock_period > device_period ? clock_period :
> device_period;
> +
> + return VK_SUCCESS;
> +}
> diff --git a/src/amd/vulkan/radv_extensions.py
> b/src/amd/vulkan/radv_extensions.py
> index 5dcedae1c63..4c81d3f0068 100644
> --- a/src/amd/vulkan/radv_extensions.py
> +++ b/src/amd/vulkan/radv_extensions.py
> @@ -92,6 +92,7 @@ EXTENSIONS = [
> Extension('VK_KHR_display', 23,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
> Extension('VK_EXT_direct_mode_display', 1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
> Extension('VK_EXT_acquire_xlib_display', 1,
> 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
> +Extension('VK_EXT_calibrated_timestamps', 1, True),
> Extension('VK_EXT_conditional_rendering', 1, True),
> Extension('VK_EXT_conservative_rasterization',1,
>
[PATCH] drm/msm/gpu: Don't map command buffers with nr_relocs equal to 0
If a command buffer doesn't have any relocs assigned to it there then
is no need to map it in the kernel address space.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/msm_gem_submit.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index aca7379b9c2e..1b05b8481c31 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -317,6 +317,9 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct msm_gem_object *ob
uint32_t *ptr;
int ret = 0;
+ if (!nr_relocs)
+ return 0;
+
if (offset % 4) {
DRM_ERROR("non-aligned cmdstream buffer: %u\n", offset);
return -EINVAL;
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Add tests for VK_EXT_calibrated_timestamps [v2]
Jason Ekstrand writes: > We're using MRs for crucible. Please create one and make sure you check > the "Allow commits from members who can merge to the target branch" so it > can be rebased through the UI by someone other than yourself. OOo. Shiny! -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vulkan: Add VK_EXT_calibrated_timestamps extension (radv and anv) [v2]
Offers three clocks, device, clock monotonic and clock monotonic
raw. Could use some kernel support to reduce the deviation between
clock values.
v2:
Ensure deviation is at least as big as the GPU time interval.
Signed-off-by: Keith Packard
---
src/amd/vulkan/radv_device.c | 84
src/amd/vulkan/radv_extensions.py | 1 +
src/intel/vulkan/anv_device.c | 88 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_gem.c | 13 +
src/intel/vulkan/anv_private.h | 2 +
6 files changed, 189 insertions(+)
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 174922780fc..29f0afbc69b 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -4955,3 +4955,87 @@ radv_GetDeviceGroupPeerMemoryFeatures(
VK_PEER_MEMORY_FEATURE_GENERIC_SRC_BIT |
VK_PEER_MEMORY_FEATURE_GENERIC_DST_BIT;
}
+
+static const VkTimeDomainEXT radv_time_domains[] = {
+ VK_TIME_DOMAIN_DEVICE_EXT,
+ VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT,
+ VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT,
+};
+
+VkResult radv_GetPhysicalDeviceCalibrateableTimeDomainsEXT(
+ VkPhysicalDevice physicalDevice,
+ uint32_t *pTimeDomainCount,
+ VkTimeDomainEXT *pTimeDomains)
+{
+ int d;
+ VK_OUTARRAY_MAKE(out, pTimeDomains, pTimeDomainCount);
+
+ for (d = 0; d < ARRAY_SIZE(radv_time_domains); d++) {
+ vk_outarray_append(, i) {
+ *i = radv_time_domains[d];
+ }
+ }
+
+ return vk_outarray_status();
+}
+
+static uint64_t
+radv_clock_gettime(clockid_t clock_id)
+{
+ struct timespec current;
+ int ret;
+
+ ret = clock_gettime(clock_id, );
+ if (ret < 0 && clock_id == CLOCK_MONOTONIC_RAW)
+ ret = clock_gettime(CLOCK_MONOTONIC, );
+ if (ret < 0)
+ return 0;
+
+ return (uint64_t) current.tv_sec * 10ULL + current.tv_nsec;
+}
+
+#define TIMESTAMP 0x2358
+
+VkResult radv_GetCalibratedTimestampsEXT(
+ VkDevice _device,
+ uint32_t timestampCount,
+ const VkCalibratedTimestampInfoEXT *pTimestampInfos,
+ uint64_t *pTimestamps,
+ uint64_t *pMaxDeviation)
+{
+ RADV_FROM_HANDLE(radv_device, device, _device);
+ uint32_t clock_crystal_freq =
device->physical_device->rad_info.clock_crystal_freq;
+ int d;
+ uint64_t begin, end;
+
+ begin = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
+
+ for (d = 0; d < timestampCount; d++) {
+ switch (pTimestampInfos[d].timeDomain) {
+ case VK_TIME_DOMAIN_DEVICE_EXT:
+ /* XXX older kernels don't support this interface. */
+ pTimestamps[d] = device->ws->query_value(device->ws,
+
RADEON_TIMESTAMP);
+ break;
+ case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
+ pTimestamps[d] = radv_clock_gettime(CLOCK_MONOTONIC);
+ break;
+
+ case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
+ pTimestamps[d] = begin;
+ break;
+ default:
+ pTimestamps[d] = 0;
+ break;
+ }
+ }
+
+ end = radv_clock_gettime(CLOCK_MONOTONIC_RAW);
+
+ uint64_t clock_period = end - begin;
+ uint64_t device_period = (100 + clock_crystal_freq - 1) /
clock_crystal_freq;
+
+ *pMaxDeviation = clock_period > device_period ? clock_period :
device_period;
+
+ return VK_SUCCESS;
+}
diff --git a/src/amd/vulkan/radv_extensions.py
b/src/amd/vulkan/radv_extensions.py
index 5dcedae1c63..4c81d3f0068 100644
--- a/src/amd/vulkan/radv_extensions.py
+++ b/src/amd/vulkan/radv_extensions.py
@@ -92,6 +92,7 @@ EXTENSIONS = [
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
+Extension('VK_EXT_calibrated_timestamps', 1, True),
Extension('VK_EXT_conditional_rendering', 1, True),
Extension('VK_EXT_conservative_rasterization',1,
'device->rad_info.chip_class >= GFX9'),
Extension('VK_EXT_display_surface_counter', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index a2551452eb1..6a6539c9685 100644
---
Re: [PATCH] Add tests for VK_EXT_calibrated_timestamps [v2]
We're using MRs for crucible. Please create one and make sure you check
the "Allow commits from members who can merge to the target branch" so it
can be rebased through the UI by someone other than yourself.
--Jason
On Mon, Oct 15, 2018 at 4:15 PM Keith Packard wrote:
> Five tests:
>
> 1) Check for non-null function pointers
> 2) Check for in-range time domains
> 3) Check monotonic domains for correct values
> 4) Check correlation between monotonic and device domains
> 5) Check to make sure times in device domain match queue times
>
> Signed-off-by: Keith Packard
> ---
> Makefile.am| 1 +
> src/tests/func/calibrated-timestamps.c | 442 +
> 2 files changed, 443 insertions(+)
> create mode 100644 src/tests/func/calibrated-timestamps.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 0ca35bd..ba98c60 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -113,6 +113,7 @@ bin_crucible_SOURCES = \
> src/tests/stress/lots-of-surface-state.c \
> src/tests/stress/buffer_limit.c \
> src/tests/self/concurrent-output.c \
> + src/tests/func/calibrated-timestamps.c \
> src/util/cru_cleanup.c \
> src/util/cru_format.c \
> src/util/cru_image.c \
> diff --git a/src/tests/func/calibrated-timestamps.c
> b/src/tests/func/calibrated-timestamps.c
> new file mode 100644
> index 000..a98150b
> --- /dev/null
> +++ b/src/tests/func/calibrated-timestamps.c
> @@ -0,0 +1,442 @@
> +/*
> + * Copyright © 2018 Keith Packard
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +
> +#include "tapi/t.h"
> +#include
> +#include
> +#include
> +
> +#define GET_DEVICE_FUNCTION_PTR(name) \
> +PFN_vk##name name = (PFN_vk##name)vkGetDeviceProcAddr(t_device,
> "vk"#name)
> +
> +#define GET_INSTANCE_FUNCTION_PTR(name) \
> +PFN_vk##name name = (PFN_vk##name)vkGetInstanceProcAddr(t_instance,
> "vk"#name)
> +
> +/* Test 1: Make sure the function pointers promised by the extension
> + * are valid
> + */
> +static void
> +test_funcs(void)
> +{
> +t_require_ext("VK_EXT_calibrated_timestamps");
> +
> +
> GET_INSTANCE_FUNCTION_PTR(GetPhysicalDeviceCalibrateableTimeDomainsEXT);
> +GET_DEVICE_FUNCTION_PTR(GetCalibratedTimestampsEXT);
> +
> +t_assert(GetPhysicalDeviceCalibrateableTimeDomainsEXT != NULL);
> +t_assert(GetCalibratedTimestampsEXT != NULL);
> +}
> +
> +test_define {
> +.name = "func.calibrated-timestamps.funcs",
> +.start = test_funcs,
> +.no_image = true,
> +};
> +
> +/* Test 2: Make sure all of the domains offered by the driver are in range
> + */
> +static void
> +test_domains(void)
> +{
> +t_require_ext("VK_EXT_calibrated_timestamps");
> +
> +
> GET_INSTANCE_FUNCTION_PTR(GetPhysicalDeviceCalibrateableTimeDomainsEXT);
> +GET_DEVICE_FUNCTION_PTR(GetCalibratedTimestampsEXT);
> +
> +t_assert(GetPhysicalDeviceCalibrateableTimeDomainsEXT != NULL);
> +t_assert(GetCalibratedTimestampsEXT != NULL);
> +
> +VkResult result;
> +
> +uint32_t timeDomainCount;
> +result = GetPhysicalDeviceCalibrateableTimeDomainsEXT(
> +t_physical_dev,
> +,
> +NULL);
> +t_assert(result == VK_SUCCESS);
> +t_assert(timeDomainCount > 0);
> +
> +VkTimeDomainEXT *timeDomains = calloc(timeDomainCount, sizeof
> (VkTimeDomainEXT));
> +t_assert(timeDomains != NULL);
> +
> +result = GetPhysicalDeviceCalibrateableTimeDomainsEXT(
> +t_physical_dev,
> +,
> +timeDomains);
> +
> +t_assert(result == VK_SUCCESS);
> +
> +/* Make sure all reported domains are valid */
> +for (uint32_t d = 0; d < timeDomainCount; d++) {
> +t_assert(VK_TIME_DOMAIN_BEGIN_RANGE_EXT <= timeDomains[d] &&
> + timeDomains[d] <= VK_TIME_DOMAIN_END_RANGE_EXT);
> +}
> +}
> +
> +test_define {
> +.name = "func.calibrated-timestamps.domains",
> +.start = test_domains,
> +.no_image = true,
> +};
> +
> +static uint64_t
> +crucible_clock_gettime(VkTimeDomainEXT domain)
> +{
> +struct timespec current;
> +int ret;
> +clockid_t clock_id;
> +
> +switch (domain) {
> +case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
> +clock_id = CLOCK_MONOTONIC;
> +break;
> +case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
> +clock_id = CLOCK_MONOTONIC_RAW;
> +break;
> +default:
> +t_assert(0);
> +return 0;
> +}
> +
> +ret = clock_gettime(clock_id, );
> +t_assert (ret >= 0);
> +if
[PATCH] Add tests for VK_EXT_calibrated_timestamps [v2]
Five tests:
1) Check for non-null function pointers
2) Check for in-range time domains
3) Check monotonic domains for correct values
4) Check correlation between monotonic and device domains
5) Check to make sure times in device domain match queue times
Signed-off-by: Keith Packard
---
Makefile.am| 1 +
src/tests/func/calibrated-timestamps.c | 442 +
2 files changed, 443 insertions(+)
create mode 100644 src/tests/func/calibrated-timestamps.c
diff --git a/Makefile.am b/Makefile.am
index 0ca35bd..ba98c60 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -113,6 +113,7 @@ bin_crucible_SOURCES = \
src/tests/stress/lots-of-surface-state.c \
src/tests/stress/buffer_limit.c \
src/tests/self/concurrent-output.c \
+ src/tests/func/calibrated-timestamps.c \
src/util/cru_cleanup.c \
src/util/cru_format.c \
src/util/cru_image.c \
diff --git a/src/tests/func/calibrated-timestamps.c
b/src/tests/func/calibrated-timestamps.c
new file mode 100644
index 000..a98150b
--- /dev/null
+++ b/src/tests/func/calibrated-timestamps.c
@@ -0,0 +1,442 @@
+/*
+ * Copyright © 2018 Keith Packard
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include "tapi/t.h"
+#include
+#include
+#include
+
+#define GET_DEVICE_FUNCTION_PTR(name) \
+PFN_vk##name name = (PFN_vk##name)vkGetDeviceProcAddr(t_device, "vk"#name)
+
+#define GET_INSTANCE_FUNCTION_PTR(name) \
+PFN_vk##name name = (PFN_vk##name)vkGetInstanceProcAddr(t_instance,
"vk"#name)
+
+/* Test 1: Make sure the function pointers promised by the extension
+ * are valid
+ */
+static void
+test_funcs(void)
+{
+t_require_ext("VK_EXT_calibrated_timestamps");
+
+GET_INSTANCE_FUNCTION_PTR(GetPhysicalDeviceCalibrateableTimeDomainsEXT);
+GET_DEVICE_FUNCTION_PTR(GetCalibratedTimestampsEXT);
+
+t_assert(GetPhysicalDeviceCalibrateableTimeDomainsEXT != NULL);
+t_assert(GetCalibratedTimestampsEXT != NULL);
+}
+
+test_define {
+.name = "func.calibrated-timestamps.funcs",
+.start = test_funcs,
+.no_image = true,
+};
+
+/* Test 2: Make sure all of the domains offered by the driver are in range
+ */
+static void
+test_domains(void)
+{
+t_require_ext("VK_EXT_calibrated_timestamps");
+
+GET_INSTANCE_FUNCTION_PTR(GetPhysicalDeviceCalibrateableTimeDomainsEXT);
+GET_DEVICE_FUNCTION_PTR(GetCalibratedTimestampsEXT);
+
+t_assert(GetPhysicalDeviceCalibrateableTimeDomainsEXT != NULL);
+t_assert(GetCalibratedTimestampsEXT != NULL);
+
+VkResult result;
+
+uint32_t timeDomainCount;
+result = GetPhysicalDeviceCalibrateableTimeDomainsEXT(
+t_physical_dev,
+,
+NULL);
+t_assert(result == VK_SUCCESS);
+t_assert(timeDomainCount > 0);
+
+VkTimeDomainEXT *timeDomains = calloc(timeDomainCount, sizeof
(VkTimeDomainEXT));
+t_assert(timeDomains != NULL);
+
+result = GetPhysicalDeviceCalibrateableTimeDomainsEXT(
+t_physical_dev,
+,
+timeDomains);
+
+t_assert(result == VK_SUCCESS);
+
+/* Make sure all reported domains are valid */
+for (uint32_t d = 0; d < timeDomainCount; d++) {
+t_assert(VK_TIME_DOMAIN_BEGIN_RANGE_EXT <= timeDomains[d] &&
+ timeDomains[d] <= VK_TIME_DOMAIN_END_RANGE_EXT);
+}
+}
+
+test_define {
+.name = "func.calibrated-timestamps.domains",
+.start = test_domains,
+.no_image = true,
+};
+
+static uint64_t
+crucible_clock_gettime(VkTimeDomainEXT domain)
+{
+struct timespec current;
+int ret;
+clockid_t clock_id;
+
+switch (domain) {
+case VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT:
+clock_id = CLOCK_MONOTONIC;
+break;
+case VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT:
+clock_id = CLOCK_MONOTONIC_RAW;
+break;
+default:
+t_assert(0);
+return 0;
+}
+
+ret = clock_gettime(clock_id, );
+t_assert (ret >= 0);
+if (ret < 0)
+return 0;
+
+return (uint64_t) current.tv_sec * 10ULL + current.tv_nsec;
+}
+
+/* Test 3: Make sure any monotonic domains return accurate data
+ */
+static void
+test_monotonic(void)
+{
+t_require_ext("VK_EXT_calibrated_timestamps");
+
+GET_INSTANCE_FUNCTION_PTR(GetPhysicalDeviceCalibrateableTimeDomainsEXT);
+GET_DEVICE_FUNCTION_PTR(GetCalibratedTimestampsEXT);
+
+t_assert(GetPhysicalDeviceCalibrateableTimeDomainsEXT != NULL);
+t_assert(GetCalibratedTimestampsEXT != NULL);
+
+VkResult result;
+
+uint32_t
Re: [PATCH] drm/radeon: change SPDX identifier to MIT
On Mon, Oct 15, 2018 at 12:53 AM Jonathan Gray wrote:
>
> Commit b24413180f5600bcb3bb70fbed5cf186b60864bd added
> "SPDX-License-Identifier: GPL-2.0" to files which previously had no
> license, change this to MIT for radeon matching the license text of the
> other radeon files.
>
> Signed-off-by: Jonathan Gray
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/radeon/mkregtable.c | 2 +-
> drivers/gpu/drm/radeon/r100_track.h | 2 +-
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> drivers/gpu/drm/radeon/radeon_legacy_tv.c| 2 +-
> drivers/gpu/drm/radeon/radeon_trace.h| 2 +-
> drivers/gpu/drm/radeon/radeon_trace_points.c | 2 +-
> include/drm/drm_pciids.h | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/mkregtable.c
> b/drivers/gpu/drm/radeon/mkregtable.c
> index ba704633b072..52a7246fed9e 100644
> --- a/drivers/gpu/drm/radeon/mkregtable.c
> +++ b/drivers/gpu/drm/radeon/mkregtable.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: MIT
> /* utility to create the register check tables
> * this includes inlined list.h safe for userspace.
> *
> diff --git a/drivers/gpu/drm/radeon/r100_track.h
> b/drivers/gpu/drm/radeon/r100_track.h
> index ad16a925f8d5..57e2b09784be 100644
> --- a/drivers/gpu/drm/radeon/r100_track.h
> +++ b/drivers/gpu/drm/radeon/r100_track.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: MIT */
>
> #define R100_TRACK_MAX_TEXTURE 3
> #define R200_TRACK_MAX_TEXTURE 6
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index f920be236cc9..84b3ad2172a3 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: MIT
>
> #include
> #include
> diff --git a/drivers/gpu/drm/radeon/radeon_legacy_tv.c
> b/drivers/gpu/drm/radeon/radeon_legacy_tv.c
> index 611cf934b211..4278272e3191 100644
> --- a/drivers/gpu/drm/radeon/radeon_legacy_tv.c
> +++ b/drivers/gpu/drm/radeon/radeon_legacy_tv.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: MIT
> #include
> #include
> #include "radeon.h"
> diff --git a/drivers/gpu/drm/radeon/radeon_trace.h
> b/drivers/gpu/drm/radeon/radeon_trace.h
> index bc26efd1793e..0d84b8aafab3 100644
> --- a/drivers/gpu/drm/radeon/radeon_trace.h
> +++ b/drivers/gpu/drm/radeon/radeon_trace.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: MIT */
> #if !defined(_RADEON_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
> #define _RADEON_TRACE_H_
>
> diff --git a/drivers/gpu/drm/radeon/radeon_trace_points.c
> b/drivers/gpu/drm/radeon/radeon_trace_points.c
> index 66b3d5084662..65e92302f974 100644
> --- a/drivers/gpu/drm/radeon/radeon_trace_points.c
> +++ b/drivers/gpu/drm/radeon/radeon_trace_points.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: MIT
> /* Copyright Red Hat Inc 2010.
> * Author : Dave Airlie
> */
> diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
> index 683742826511..b7e899ce44f0 100644
> --- a/include/drm/drm_pciids.h
> +++ b/include/drm/drm_pciids.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: MIT */
> #define radeon_PCI_IDS \
> {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
> {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: correct SPDX identifier in amdgpu_trace_points.c
On Mon, Oct 15, 2018 at 12:52 AM Jonathan Gray wrote: > > Commit b24413180f5600bcb3bb70fbed5cf186b60864bd > 'License cleanup: add SPDX GPL-2.0 license identifier to files with no > license' > incorrectly added "SPDX-License-Identifier: GPL-2.0" to a file with MIT > license text. Change the SPDX identifier to match the license text. > > Signed-off-by: Jonathan Gray Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c > index b160b958e5fe..f212402570a5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c > @@ -1,4 +1,4 @@ > -// SPDX-License-Identifier: GPL-2.0 > +// SPDX-License-Identifier: MIT > /* Copyright Red Hat Inc 2010. > * > * Permission is hereby granted, free of charge, to any person obtaining a > -- > 2.19.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107823] [amdgpu/displayport] Blackscreen on native resolution
https://bugs.freedesktop.org/show_bug.cgi?id=107823 --- Comment #6 from Drexler --- Hi, was wondering if you can share the EDID of this NEC EA223WM monitor? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: fix handling of cmdstream offset
Userspace hasn't used submit cmds with submit_offset != 0 for a while,
but this starts cropping up again with cmdstream sub-buffer-allocation
in libdrm_freedreno.
Doesn't do much good to increment the buf ptr before assigning it.
Fixes: 78b8e5b847b4 drm/msm: dump a rd GPUADDR header for all buffers in the
command
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_rd.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
index cca933458439..0c2c8d2c631f 100644
--- a/drivers/gpu/drm/msm/msm_rd.c
+++ b/drivers/gpu/drm/msm/msm_rd.c
@@ -316,10 +316,11 @@ static void snapshot_buf(struct msm_rd_state *rd,
uint64_t iova, uint32_t size)
{
struct msm_gem_object *obj = submit->bos[idx].obj;
+ unsigned offset = 0;
const char *buf;
if (iova) {
- buf += iova - submit->bos[idx].iova;
+ offset = iova - submit->bos[idx].iova;
} else {
iova = submit->bos[idx].iova;
size = obj->base.size;
@@ -340,6 +341,8 @@ static void snapshot_buf(struct msm_rd_state *rd,
if (IS_ERR(buf))
return;
+ buf += offset;
+
rd_write_section(rd, RD_BUFFER_CONTENTS, buf, size);
msm_gem_put_vaddr(>base);
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107261] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us * 10 tries - optc1_lock line:628
https://bugs.freedesktop.org/show_bug.cgi?id=107261 --- Comment #8 from Vedran Miletić --- I get these errors on Ryzen 7 2700U 03:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. [AMD/ATI] Raven Ridge [Radeon Vega Series / Radeon Vega Mobile Series] [1002:15dd] (rev c3) (prog-if 00 [VGA controller]) Subsystem: Acer Incorporated [ALI] Device [1025:1259] Flags: bus master, fast devsel, latency 0, IRQ 54 Memory at d000 (64-bit, prefetchable) [size=256M] Memory at e000 (64-bit, prefetchable) [size=2M] I/O ports at 1000 [size=256] Memory at e080 (32-bit, non-prefetchable) [size=512K] Capabilities: Kernel driver in use: amdgpu Kernel modules: amdgpu when running Valley. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/drm_vblank: Change EINVAL by the correct errno
For historical reason, the function drm_wait_vblank_ioctl always return -EINVAL if something gets wrong. This scenario limits the flexibility for the userspace make detailed verification of the problem and take some action. In particular, the validation of “if (!dev->irq_enabled)” in the drm_wait_vblank_ioctl is responsible for checking if the driver support vblank or not. If the driver does not support VBlank, the function drm_wait_vblank_ioctl returns EINVAL which does not represent the real issue; this patch changes this behavior by return EOPNOTSUPP. Additionally, some operations are unsupported by this function, and returns EINVAL; this patch also changes the return value to EOPNOTSUPP in this case. Lastly, the function drm_wait_vblank_ioctl is invoked by libdrm, which is used by many compositors; because of this, it is important to check if this change breaks any compositor. In this sense, the following projects were examined: * Drm-hwcomposer * Kwin * Sway * Wlroots * Wayland-core * Weston * Xorg (67 different drivers) For each repository the verification happened in three steps: * Update the main branch * Look for any occurrence "drmWaitVBlank" with the command: git grep -n "drmWaitVBlank" * Look in the git history of the project with the command: git log -SdrmWaitVBlank Finally, none of the above projects validate the use of EINVAL which make safe, at least for these projects, to change the return values. Change since V1: Daniel Vetter and Chris Wilson - Replace ENOTTY by EOPNOTSUPP - Return EINVAL if the parameters are wrong Signed-off-by: Rodrigo Siqueira --- drivers/gpu/drm/drm_vblank.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 98e091175921..80f5a3bb427e 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -1533,10 +1533,10 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, unsigned int flags, pipe, high_pipe; if (!dev->irq_enabled) - return -EINVAL; + return -EOPNOTSUPP; if (vblwait->request.type & _DRM_VBLANK_SIGNAL) - return -EINVAL; + return -EOPNOTSUPP; if (vblwait->request.type & ~(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK | -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Allow fd.o to join forces with X.Org
On October 15, 2018 2:50:13 PM UTC, Harry Wentland
wrote:
> The leadership of freedesktop.org (fd.o) has recently expressed
> interest
> in having an elected governing body. Given the tight connection
> between
> fd.o and X.Org and the fact that X.Org has such a governing body it
> seemed obvious to consider extending X.Org's mandate to fd.o.
>
> Quite a bit of background on fd.o leading up to this has been covered
> by
> Daniel Stone at XDC 2018 and was covered really well by Jake Edge of
> LWN [1].
If you'd like to watch Daniel's presentation, the recording is available on
YouTube:
https://youtu.be/s22B3E7rUTs
The slides are linked in the description.
>
> One question that is briefly addressed in the LWN article and was
> thoroughly discussed by members of the X.Org boards, Daniel Stone, and
> others in hallway discussions is the question of whether to extend the
> X.Org membership to projects hosted on fd.o but outside the purpose of
> the X.Org foundation as enacted in its bylaws.
>
> Most people I talked to would prefer not to dilute X.Org's mission and
> extend membership only to contributors of projects that follow X.Org's
> purpose as enacted in its bylaws. Other projects can continue to be
> hosted on fd.o but won't receive X.Org membership for the mere reason
> of
> being hosted on fd.o.
With my member hat on, I think this is the best choice.
Acked-by: Eric Engestrom
>
> [1] https://lwn.net/Articles/767258/
>
> v2:
> - Subject line that better describes the intention
> - Briefly describe reasons behind this change
> - Drop expanding membership eligibility
> ---
>
> We're looking for feedback and comments on this patch. If it's not
> widely controversial the final version of the patch will be put to a
> vote at the 2019 X.Org elections.
>
> The patch applies to the X.Org bylaws git repo, which can be found at
> https://gitlab.freedesktop.org/xorgfoundation/bylaws
>
> Happy commenting.
>
> Harry
>
> bylaws.tex | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/bylaws.tex b/bylaws.tex
> index 4ab35a4f7745..44ff4745963b 100644
> --- a/bylaws.tex
> +++ b/bylaws.tex
> @@ -14,7 +14,7 @@ BE IT ENACTED AND IT IS HEREBY ENACTED as a By-law
> of the X.Org Foundation
>
> The purpose of the X.Org Foundation shall be to:
> \begin{enumerate}[(i)\hspace{.2cm}]
> - \item Research, develop, support, organize, administrate,
> standardize,
> + \item \label{1} Research, develop, support, organize, administrate,
> standardize,
> promote, and defend a free and open accelerated graphics stack. This
> includes, but is not limited to, the following projects: DRM, Mesa,
> Wayland and the X Window System,
> @@ -24,6 +24,11 @@ The purpose of the X.Org Foundation shall be to:
>
> \item Support and educate the general community of users of this
> graphics stack.
> +
> + \item Support free and open source projects through the
> freedesktop.org
> + infrastructure. For projects outside the scope of item (\ref{1})
> support
> + extends to project hosting only.
> +
> \end{enumerate}
>
> \article{INTERPRETATION}
> --
> 2.19.1
>
> ___
> memb...@foundation.x.org: X.Org Foundation Members
> Archives: https://foundation.x.org/cgi-bin/mailman/private/members
> Info: https://foundation.x.org/cgi-bin/mailman/listinfo/members
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging: ion: Rework ion_map_dma_buf() to minimize re-mapping
On Sat, Oct 13, 2018 at 11:01 PM, Liam Mark wrote: > On Fri, 12 Oct 2018, Laura Abbott wrote: >> I thought there might have been some Qualcomm >> stuff that did that (Liam? Todd?) > > Yes we have a form of "lazy mapping", which clients can opt into using, > which results in iommu page table mapping not being unmaped on dma-unamp. > Instead they are unmapped when the buffer is destroyed. > > It is important to note that in our "lazy mapping" implementation cache > maintenance is still applied on dma-map and dma-unmap. > If you need a full description of this feature I can provide it. > >> >> I suspect most of the cost of the dma_map/dma_unmap is from the >> cache flushing and not the actual mapping operations. If this >> is the case, another option might be to figure out how to >> incorporate dma_attrs so drivers can use DMA_ATTR_SKIP_CPU_SYNC >> to decide when they actually want to sync. > > We have support for this locally on our 4.14 branch. > > We have added a dma_map_attrs field to the dma_buf_attachment struct, > clients can then specify dma-attributes here such as > DMA_ATTR_SKIP_CPU_SYNC before dma-mapping the buffer, then we ensure that > these dma attributes are passed to dma_map_sg_attrs when ion_map_dma_buf > is called (same for ion_unmap_dma_buf). Do you happen to have a pointer to the tree if its public? thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging: ion: Rework ion_map_dma_buf() to minimize re-mapping
On Fri, Oct 12, 2018 at 10:51 AM, Laura Abbott wrote:
> On 10/10/2018 04:33 PM, John Stultz wrote:
>>
>> Since 4.12, much later narrowed down to commit 2a55e7b5e544
>> ("staging: android: ion: Call dma_map_sg for syncing and mapping"),
>> we have seen graphics performance issues on the HiKey960.
>>
>> This was initially confounded by the fact that the out-of-tree
>> DRM driver was using HiSi custom ION heap which broke with the
>> 4.12 ION abi changes, so there was lots of suspicion that the
>> performance problems were due to switching to a somewhat simple
>> cma based DRM driver for HiKey960. Additionally, as no
>> performance regression was seen w/ the original HiKey board
>> (which is SMP, not big.LITTLE as w/ HiKey960), there was some
>> thought that the out-of-tree EAS code wasn't quite optimized.
>>
>> But after chasing a number of other leads, I found that
>> reverting the ION code to 4.11-era got the majority of the
>> graphics performance back (there may yet be further EAS tweaks
>> needed), which lead me to the dma_map_sg change.
>>
>> In talking w/ Laura and Liam, it was suspected that the extra
>> cache operations were causing the trouble. Additionally, I found
>> that part of the reason we didn't see this w/ the original
>> HiKey board is that its (proprietary blob) GL code uses ion_mmap
>> and ion_map_dma_buf is called very rarely, where as with
>> HiKey960, the (also proprietary blob) GL code calls
>> ion_map_dma_buf much more frequently via the kernel driver.
>>
>> Anyway, with the cause of the performance regression isolated,
>> I've tried to find a way to improve the performance of the
>> current code.
>>
>> This approach, which I've mostly copied from the drm_prime
>> implementation is to try to track the direction we're mapping
>> the buffers so we can avoid calling dma_map/unmap_sg on every
>> ion_map_dma_buf/ion_unmap_dma_buf call, and instead try to do
>> the work in attach/detach paths.
>>
>> I'm not 100% sure of the correctness here, so close review would
>> be good, but it gets the performance back to being similar to
>> reverting the ION code to the 4.11-era.
>>
>> Feedback would be greatly appreciated!
>>
...
>> @@ -264,7 +291,6 @@ static void ion_unmap_dma_buf(struct
>> dma_buf_attachment *attachment,
>> struct sg_table *table,
>> enum dma_data_direction direction)
>> {
>> - dma_unmap_sg(attachment->dev, table->sgl, table->nents,
>> direction);
>
>
> This changes the semantics so that the only time a buffer
> gets unmapped is on detach. I don't think we want to restrict
> Ion to that behavior but I also don't know if anyone else
> is relying on that. I thought there might have been some Qualcomm
> stuff that did that (Liam? Todd?)
>
> I suspect most of the cost of the dma_map/dma_unmap is from the
> cache flushing and not the actual mapping operations. If this
> is the case, another option might be to figure out how to
> incorporate dma_attrs so drivers can use DMA_ATTR_SKIP_CPU_SYNC
> to decide when they actually want to sync.
Ok. Thanks so much for the feedback and the suggestion. I'll try to
look into dma_attrs here shortly.
thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v3 04/18] drm/selftest: Add drm damage helper selftest
> On Wed, Oct 10, 2018 at 05:16:43PM -0700, Deepak Rawat wrote:
> > Selftest for drm damage helper iterator functions.
> >
> > Cc: ville.syrj...@linux.intel.com
> > Cc: Daniel Vetter
> > Cc: Pekka Paalanen
> > Cc: Daniel Stone
> > Cc: intel-...@lists.freedesktop.org
> > Cc: igt-...@lists.freedesktop.org
> > Cc: petri.latv...@intel.com
> > Cc: ch...@chris-wilson.co.uk
> > Signed-off-by: Deepak Rawat
> > ---
> > drivers/gpu/drm/selftests/Makefile| 3 +-
> > .../selftests/drm_damage_helper_selftests.h | 22 +
> > .../drm/selftests/test-drm_damage_helper.c| 844
> ++
> > 3 files changed, 868 insertions(+), 1 deletion(-)
> > create mode 100644
> drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > create mode 100644 drivers/gpu/drm/selftests/test-
> drm_damage_helper.c
> >
> > diff --git a/drivers/gpu/drm/selftests/Makefile
> b/drivers/gpu/drm/selftests/Makefile
> > index 9fc349fa18e9..88ac216f5962 100644
> > --- a/drivers/gpu/drm/selftests/Makefile
> > +++ b/drivers/gpu/drm/selftests/Makefile
> > @@ -1 +1,2 @@
> > -obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm-
> helper.o
> > +obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm-
> helper.o \
> > + test-drm_damage_helper.o
>
> With the testcase intagrated into the test-drm-helper.ko module, for
> patches 1-4 in this series:
>
> Reviewed-by: Daniel Vetter
>
> Obviously needs some adjusting on the igt side too, since we seem to be
> missing the igt scaffolding for tests-drm-helper.ko.
> -Daniel
Hi Daniel,
Thanks for the review. I am a little confused here. Should we have single
kernel module for drm plane helper selftest and damage helper selftest?
Also shall I rename the kernel selfttest to kms_*?
For user-space igt test it should be it makes sense to rename to kms_selftets?
>
> > diff --git a/drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> b/drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > new file mode 100644
> > index ..3a1cbe05bef0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +selftest(damage_iter_no_damage, igt_damage_iter_no_damage)
> > +selftest(damage_iter_no_damage_fractional_src,
> igt_damage_iter_no_damage_fractional_src)
> > +selftest(damage_iter_no_damage_src_moved,
> igt_damage_iter_no_damage_src_moved)
> > +selftest(damage_iter_no_damage_fractional_src_moved,
> igt_damage_iter_no_damage_fractional_src_moved)
> > +selftest(damage_iter_no_damage_not_visible,
> igt_damage_iter_no_damage_not_visible)
> > +selftest(damage_iter_no_damage_no_crtc,
> igt_damage_iter_no_damage_no_crtc)
> > +selftest(damage_iter_no_damage_no_fb,
> igt_damage_iter_no_damage_no_fb)
> > +selftest(damage_iter_simple_damage,
> igt_damage_iter_simple_damage)
> > +selftest(damage_iter_single_damage, igt_damage_iter_single_damage)
> > +selftest(damage_iter_single_damage_intersect_src,
> igt_damage_iter_single_damage_intersect_src)
> > +selftest(damage_iter_single_damage_outside_src,
> igt_damage_iter_single_damage_outside_src)
> > +selftest(damage_iter_single_damage_fractional_src,
> igt_damage_iter_single_damage_fractional_src)
> > +selftest(damage_iter_single_damage_intersect_fractional_src,
> igt_damage_iter_single_damage_intersect_fractional_src)
> > +selftest(damage_iter_single_damage_outside_fractional_src,
> igt_damage_iter_single_damage_outside_fractional_src)
> > +selftest(damage_iter_single_damage_src_moved,
> igt_damage_iter_single_damage_src_moved)
> > +selftest(damage_iter_single_damage_fractional_src_moved,
> igt_damage_iter_single_damage_fractional_src_moved)
> > +selftest(damage_iter_damage, igt_damage_iter_damage)
> > +selftest(damage_iter_damage_one_intersect,
> igt_damage_iter_damage_one_intersect)
> > +selftest(damage_iter_damage_one_outside,
> igt_damage_iter_damage_one_outside)
> > +selftest(damage_iter_damage_src_moved,
> igt_damage_iter_damage_src_moved)
> > +selftest(damage_iter_damage_not_visible,
> igt_damage_iter_damage_not_visible)
> > diff --git a/drivers/gpu/drm/selftests/test-drm_damage_helper.c
> b/drivers/gpu/drm/selftests/test-drm_damage_helper.c
> > new file mode 100644
> > index ..17754734c47a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/selftests/test-drm_damage_helper.c
> > @@ -0,0 +1,844 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Test case for drm_damage_helper functions
> > + */
> > +
> > +#define pr_fmt(fmt) "drm_damage_helper: " fmt
> > +
> > +#include
> > +#include
> > +
> > +#define TESTS "drm_damage_helper_selftests.h"
> > +#include "drm_selftest.h"
> > +
> > +#define FAIL(test, msg, ...) \
> > + do { \
> > + if (test) { \
> > + pr_err("%s/%u: " msg, __FUNCTION__, __LINE__,
> ##__VA_ARGS__); \
> > + return -EINVAL; \
> > + } \
> > + } while (0)
> > +
> > +#define FAIL_ON(x) FAIL((x), "%s",
Re: [PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC
On 10/15/2018 09:57 AM, Pekka Paalanen wrote: On Fri, 12 Oct 2018 08:58:23 -0400 "Kazlauskas, Nicholas" wrote: On 10/12/2018 07:20 AM, Koenig, Christian wrote: Am 12.10.2018 um 11:21 schrieb Pekka Paalanen: On Fri, 12 Oct 2018 07:35:57 + "Koenig, Christian" wrote: Am 12.10.2018 um 09:23 schrieb Pekka Paalanen: On Wed, 10 Oct 2018 09:35:50 -0400 "Kazlauskas, Nicholas" wrote: The patches I've put out target this use case mostly because of the benefit for a relatively simple interface. VRR can and has been used in more ways that this, however. An example usecase that differs from this would actually be video playback. The monitor's refresh rate likely doesn't align with the video content rate. An API that exposes direct control over VRR (like the range, refresh duration, presentation timestamp) could allow the application to specify the content rate directly to deliver a smoother playback experience. For example, if you had a 24fps video and the VRR range was 40-60Hz you could target 48Hz via some API here. The way that has been discussed to be implemented is that DRM page flips would carry a target timestamp, which the driver will then meet as good as it can. It is better to define an absolute target timestamp than a frequency, because a timestamp can be used to synchronize with audio and more. Mario Kleiner can tell you all about scientific use cases that require accurate display timing, not just frequency. To summarize what information should be provided by the driver stack to make applications happy: 1. The minimum time a frame can be displayed, in other words the maximum frame rate. 2. The maximum time a frame can be displayed, in other words the minimum frame rate. 3. How much change of frame timing is allowed between frames to avoid luminescence flickering. Number 1 and 2 can also be used to signal the availability of VRR to applications, e.g. if they are identical we don't support VRR at all. Hi Christian, "the maximum time a frame can be displayed" is perhaps not an unambiguous definition. A frame can be shown indefinitely in any case. Good point. Please also note that I'm not an expert on the display stuff in general. My background comes more from implementing the VDPAU mediaplayer interface in mesa. So just throwing some ideas in here from the point of view of an userspace developer which wants to write a media player :) Excellent! The CRTC will simply start scanning out the same frame again if there is no new one. I understand what you want to say, but perhaps some different words will be in order. I do wonder if applications really want to know the maximum acceptable slew rate in timings... maybe that should be left for the drivers to apply. What I'm thinking is that we have the page flip timestamp with the page flip events to tell when the new FB became active. That information could be extended with a time range on when the very next flip could take place. Applications are already computing that prediction from the flip timestamp and fixed refresh rate, but it might be nice to give them the driver's opinion explicitly. Maybe the tolerable slew rate is not a constant. Well it depends. I agree that the kernel should probably enforce the slew rate to avoid flickering for the user. That's what I was hoping if the monitor hardware does not do that already, but now it sounds like it's not possible. Another failed assumption from my side. But it might be beneficial for the application to know things like this. Applications should know when they could likely flip, my question is how to tell them. Is the acceptable slew rate a constant for a video mode, or does it depend on the previous refresh interval. What the application definitely needs to know is when a frame was actually displayed. E.g. application says I want to display this at point X and at some point later the kernel returns saying the frame was displayed at point N where in the ideal case N=X. You already get this timestamp with the DRM page flip events. We also have Wayland and X11 extensions to get it to apps. Additional to that let's please use 64bit values and nanoseconds for every value we use here, and NOT fps, line numbers or similar :) Seconded! I'd really favour absolute timestamps even. Regards, Christian. Flickering will really depend on the panel itself. A wider ranger is more likely to exhibit the issue, but factors like topology, pixel density and other display technology can affect this. It can be hard for a driver to guess at all of this. For many panels it'd be difficult to notice unless it's consistently jumping between the min and max range. Do you mean that there is no way to know and get that information from the monitor itself? Nothing in EDID that could even be used as a default and quirk the monitors that got it wrong? The variable refresh range is exposed, but that's about it. There's certainly no simple algorithm you can feed monitor
[Bug 108350] amd-staging-drm-next-git witcher3 crashes and graphical oddities vega10 radv
https://bugs.freedesktop.org/show_bug.cgi?id=108350 --- Comment #2 from Michel Dänzer --- Can you bisect the kernel? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108098] Acer Swift 3 Ryzen 7 2700U, amdgpu, graphics freezes on 4.15.0-34
https://bugs.freedesktop.org/show_bug.cgi?id=108098 --- Comment #3 from Antonio Chirizzi --- Hello, after a few days without problems, I got a complete freeze today. The laptop was really stuck, not ssh access was possible. In the mean time I managed to install the latest LinuxMint 19 Cinnamon, and also reinstalled the latest kernel 4.18.0-041800-generic from Ubuntu. I have no info in any log file or any core available. The computer just froze. What can I do to collect useful data the next time it freezes? Thanks -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
On Mon, Oct 15, 2018 at 5:24 PM Thomas Hellstrom wrote: > On 10/15/2018 04:47 PM, Daniel Vetter wrote: > > On Mon, Oct 15, 2018 at 2:30 PM Thomas Hellstrom > > wrote: > >> On 10/15/2018 12:55 PM, Koenig, Christian wrote: > >>> Am 15.10.2018 um 12:42 schrieb Thomas Hellstrom: > On 10/15/2018 10:43 AM, Koenig, Christian wrote: > > Hi Thomas, > > > >> 1. You need the a lock with the order lock->mmap_sem, cause otherwise > >> you run into trouble when drm_vma_node_unmap() needs to be called. > >> Why is this? unmap_mapping_range() never takes the mmap_sem. It takes > >> a finer-level per-address-space lock. > > Well, that is interesting. Where do we actually then have the order of > > reservation->mmap_sem defined? > TTM doesn't. But some drivers using TTM do, which prompted the wording > in the fault handler comments. > But as Daniel mentioned, to be able to share buffers throughout DRM, > we at some point need to agree on a locking order, and when we do it's > important that we consider all relevant use-cases and workarounds. > > To me it certainly looks like the vm_ops implementations would be more > straightforward if we were to use mmap_sem->bo_reserve, but that would > again require a number of drivers to rework their copy_*_user code. > If those drivers, however, insisted on keeping their copy_*_user code, > they probably need to fix the recursive bo reservation up anyway, for > example by making sure not to copy from a vma that has VM_IO set. > >>> Yeah, completely agree. If you ask me using copy_(from|to)_user while a > >>> BO is reserved is currently completely forbidden. > >> So if we all agree that this should be completely forbidden, and this is > >> what requires the bo_reserve->mmap_sem locking order, then I guess the > >> logical follow up is trying to enforce mmap_sem->bo_reserve and pursuade > >> driver maintainers to convert. Perhaps as a configurable lockdep option? > > The problem with adding that under e.g. CONFIG_PROVE_LOCKING or > > similar is that we have drivers violating it right now. And last time > > around this entire discussion came up no one wanted to do the work to > > convert drivers, since it means you get to add a slowpath to the most > > complex ioctl they have. > > -Daniel > > Hmm. I actually had something like CONFIG_DRM_PROVE_LOCKING in mind; To > be enabled only for DRM developers. dma-buf is shared outside of DRM, imo that's where all the troubles lie. Especially once we have Christian's work merged to dynamically pin/unpin shared dma-bufs. We really need everyone to follow the locking rules around dma-buf/reservation_obj, or things won't really work. > But regardless, that slowpath is not hit frequently enough to affect > real-life performance, right? So it's more a matter of coding effort > rather than valid performance concerns? Validating it was the big concern for i915 at least. We typed tons of tests to make sure it keeps working, plus clever tricks to make sure we can actually test it - we heuristically prefault, which reduces the real-world hit rate for the slow-path to 0. Perfect for hiding a security bug :-/ -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
On 10/15/2018 04:47 PM, Daniel Vetter wrote: On Mon, Oct 15, 2018 at 2:30 PM Thomas Hellstrom wrote: On 10/15/2018 12:55 PM, Koenig, Christian wrote: Am 15.10.2018 um 12:42 schrieb Thomas Hellstrom: On 10/15/2018 10:43 AM, Koenig, Christian wrote: Hi Thomas, 1. You need the a lock with the order lock->mmap_sem, cause otherwise you run into trouble when drm_vma_node_unmap() needs to be called. Why is this? unmap_mapping_range() never takes the mmap_sem. It takes a finer-level per-address-space lock. Well, that is interesting. Where do we actually then have the order of reservation->mmap_sem defined? TTM doesn't. But some drivers using TTM do, which prompted the wording in the fault handler comments. But as Daniel mentioned, to be able to share buffers throughout DRM, we at some point need to agree on a locking order, and when we do it's important that we consider all relevant use-cases and workarounds. To me it certainly looks like the vm_ops implementations would be more straightforward if we were to use mmap_sem->bo_reserve, but that would again require a number of drivers to rework their copy_*_user code. If those drivers, however, insisted on keeping their copy_*_user code, they probably need to fix the recursive bo reservation up anyway, for example by making sure not to copy from a vma that has VM_IO set. Yeah, completely agree. If you ask me using copy_(from|to)_user while a BO is reserved is currently completely forbidden. So if we all agree that this should be completely forbidden, and this is what requires the bo_reserve->mmap_sem locking order, then I guess the logical follow up is trying to enforce mmap_sem->bo_reserve and pursuade driver maintainers to convert. Perhaps as a configurable lockdep option? The problem with adding that under e.g. CONFIG_PROVE_LOCKING or similar is that we have drivers violating it right now. And last time around this entire discussion came up no one wanted to do the work to convert drivers, since it means you get to add a slowpath to the most complex ioctl they have. -Daniel Hmm. I actually had something like CONFIG_DRM_PROVE_LOCKING in mind; To be enabled only for DRM developers. But regardless, that slowpath is not hit frequently enough to affect real-life performance, right? So it's more a matter of coding effort rather than valid performance concerns? /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] dt-bindings: display: renesas: du: Document the r8a7744 bindings
On Fri, 21 Sep 2018 19:08:28 +0100, Fabrizio Castro wrote: > From: Biju Das > > Document the RZ/G1N (R8A7744) SoC in the R-Car DU bindings. > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro > --- > Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] dt-bindings: display: renesas: du: Document the r8a77470 bindings
On Fri, 21 Sep 2018 19:08:27 +0100, Fabrizio Castro wrote: > Document the RZ/G1C (r8a77470) SoC in R-Car DU bindings. > > Signed-off-by: Fabrizio Castro > Reviewed-by: Biju Das > --- > Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/9] PM / OPP: dt-bindings: Add opp-interconnect-bw
On Mon, Oct 15, 2018 at 09:34:20AM -0500, Rob Herring wrote:
> On Mon, Aug 27, 2018 at 09:11:09AM -0600, Jordan Crouse wrote:
> > Add the "opp-interconnect-bw" property to specify the
> > average and peak bandwidth for an interconnect path for
> > a specific operating power point. A separate bandwidth
> > pair can be specified for each of the interconnects
> > defined for the device by appending the interconnect
> > name to the property.
> >
> > Signed-off-by: Jordan Crouse
> > ---
> > Documentation/devicetree/bindings/opp/opp.txt | 36 +++
> > 1 file changed, 36 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt
> > b/Documentation/devicetree/bindings/opp/opp.txt
> > index c396c4c0af92..d714c084f36d 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -170,6 +170,11 @@ Optional properties:
> >functioning of the current device at the current OPP (where this
> > property is
> >present).
> >
> > +- opp-interconnect-bw-: This is an array of pairs specifying the
> > average
> > + and peak bandwidth in bytes per second for the interconnect path known by
> > + 'name'. This should match the name(s) specified by interconnect-names
> > in the
> > + device definition.
> > +
>
> I don't think this is good design with the name defined in one node and
> then used in the OPP table. First, '*-names' is typically the a name
> local to that node/device. If you had 2 instances of a device with a
> shared OPP table for the 2 instances, then you are going to have to
> make the names unique. Second, how exactly would having multiple b/w
> entries work? A given OPP frequency supports the sum of the b/w entries?
> What if some devices for b/w entries aren't currently active?
For device that is fully scalable a given OPP frequency could need a
different quota for multiple interconnect entries - each one would be
programmed individually, something like this:
set_opp() {
set_device_frequency(opp);
for each interconnect name {
get_device_bw(opp, name);
icc_set(name, bw);
}
}
And the naming scheme gives you the flexibility to choose the icc names
you need. For example if port0 was always fixed and port1 scaled you could
specify the b/w for port1 without having to also have a dummy vote for port0.
The main reason I included the names was that I felt it had a potential to
be confusing if we used absolute indexes or some other implied scheme - at
least for the driver developer it makes slightly more sense to look up the
bandwidth for "port0" with the same name you used to register the interconnect.
> This also seems like a mixture of using OPP table for setting GPU's
> bandwidth/freq and then the interconnect binding to set the
> interconnect's bandwidth. Perhaps we need a more unified approach. Not
> sure what that would look like though.
In a perfect world the interconnect(s) would be left to scale on their own
and they wouldn't be tied to an individual device frequency. But that would
involve significantly more infrastructure for a given device.
Also in RE to our other discussion with Viresh on the qcom,level for the
GPU; the GPU frequency vote is done by the microcontroller but the
interconnect bandwidth is set by the CPU on the sdm845 so we're already
not unified by design. There could be something out there that uses
magic and callbacks but I also don't know what that looks like.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Planes enumeration with sun4i-drm driver
On Mon, Oct 15, 2018 at 10:29 AM Maxime Ripard wrote: > On Fri, Oct 12, 2018 at 06:47:13PM +0200, Paul Kocialkowski wrote: > > I'm looking at the sun4i DRM driver these days (especially for > > mainlining the VPU tiled format support through the frontend). > > > > The way things are done currently, all the possibly-supported plane > > formats are listed in sun4i_backend_layer_formats and exposed as-is to > > userspace for every plane. However, some of these formats cannot be > > used on all planes at the same time and will be rejected when checking > > the atomic commit. > > > > I find this a bit confusing from a userspace perspective. > > > > Not all constraints can be provided to userspace (e.g. the number of > > planes we can effectively scale), but when it comes to formats, we have > > that possibility. So perhaps it would make sense to only enumerate > > formats as many times as we can support them. > > > > For instance, it could look like: > > # plane 0: primary, RGB only > > # plane 1: overlay, RGB + backend YUV formats > > # plane 2: overlay, RGB + frontend YUV formats > > # plane 3: overlay, RGB only > > > > That's not perfect either, because e.g. requesting a scaled RGB plane > > will require the frontend and thus make it impossible to use the > > frontend YUV formats that would still be described. But it feels like > > it would be closer to representing hardware capabilities than our > > current situation. > > That's really arguable. The hardware capabilities are that you can use > any of those formats or features, on any plane, *but* on only one > plane at the same time. What you describe isn't closer to it, it's > just a way to workaround the issue we are facing (ie being able to > show those constraints to userspace), and an imperfect workaround. > > > I am really not sure about the DRM subsystem policy about this, though. > > Perhaps it was already decided that it's fine to deal with everything > > at commit checking time and report more formats than the hardware can > > really handle. > > It doesn't really matter what the DRM policy is here. Any change in > the direction you suggest would be a regression from the userspace > point of view and therefore would have to be reverted. How exactly can this cause a regression? Do you have some userspace that card-codes it's allocation of planes, which would then fail here and worked beforehand? > IIRC Weston tries to discover those constraints by brute-forcing > atomic_check and figuring out a combination that works, and we would > surely benefit some kind of API to expose composition constraints. The current hints we have is to limit the features each plane exposes. Currently there's no proposal to do stuff like "only x of y planes can do $feature" at all. So yeah, Paul's idea doesn't seem entirely out of hand to me, and for the "bug regressions" we need a concrete example of what will break. Since we're fine-tuning the drm api all the time, within the limits of what userspace expects :-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] Allow fd.o to join forces with X.Org
The leadership of freedesktop.org (fd.o) has recently expressed interest
in having an elected governing body. Given the tight connection between
fd.o and X.Org and the fact that X.Org has such a governing body it
seemed obvious to consider extending X.Org's mandate to fd.o.
Quite a bit of background on fd.o leading up to this has been covered by
Daniel Stone at XDC 2018 and was covered really well by Jake Edge of LWN [1].
One question that is briefly addressed in the LWN article and was
thoroughly discussed by members of the X.Org boards, Daniel Stone, and
others in hallway discussions is the question of whether to extend the
X.Org membership to projects hosted on fd.o but outside the purpose of
the X.Org foundation as enacted in its bylaws.
Most people I talked to would prefer not to dilute X.Org's mission and
extend membership only to contributors of projects that follow X.Org's
purpose as enacted in its bylaws. Other projects can continue to be
hosted on fd.o but won't receive X.Org membership for the mere reason of
being hosted on fd.o.
[1] https://lwn.net/Articles/767258/
v2:
- Subject line that better describes the intention
- Briefly describe reasons behind this change
- Drop expanding membership eligibility
---
We're looking for feedback and comments on this patch. If it's not
widely controversial the final version of the patch will be put to a
vote at the 2019 X.Org elections.
The patch applies to the X.Org bylaws git repo, which can be found at
https://gitlab.freedesktop.org/xorgfoundation/bylaws
Happy commenting.
Harry
bylaws.tex | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/bylaws.tex b/bylaws.tex
index 4ab35a4f7745..44ff4745963b 100644
--- a/bylaws.tex
+++ b/bylaws.tex
@@ -14,7 +14,7 @@ BE IT ENACTED AND IT IS HEREBY ENACTED as a By-law of the
X.Org Foundation
The purpose of the X.Org Foundation shall be to:
\begin{enumerate}[(i)\hspace{.2cm}]
- \item Research, develop, support, organize, administrate, standardize,
+ \item \label{1} Research, develop, support, organize, administrate,
standardize,
promote, and defend a free and open accelerated graphics stack. This
includes, but is not limited to, the following projects: DRM, Mesa,
Wayland and the X Window System,
@@ -24,6 +24,11 @@ The purpose of the X.Org Foundation shall be to:
\item Support and educate the general community of users of this
graphics stack.
+
+ \item Support free and open source projects through the freedesktop.org
+ infrastructure. For projects outside the scope of item (\ref{1}) support
+ extends to project hosting only.
+
\end{enumerate}
\article{INTERPRETATION}
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
On Mon, Oct 15, 2018 at 2:30 PM Thomas Hellstrom wrote: > > On 10/15/2018 12:55 PM, Koenig, Christian wrote: > > Am 15.10.2018 um 12:42 schrieb Thomas Hellstrom: > >> On 10/15/2018 10:43 AM, Koenig, Christian wrote: > >>> Hi Thomas, > >>> > 1. You need the a lock with the order lock->mmap_sem, cause otherwise > you run into trouble when drm_vma_node_unmap() needs to be called. > Why is this? unmap_mapping_range() never takes the mmap_sem. It takes > a finer-level per-address-space lock. > >>> Well, that is interesting. Where do we actually then have the order of > >>> reservation->mmap_sem defined? > >> TTM doesn't. But some drivers using TTM do, which prompted the wording > >> in the fault handler comments. > >> But as Daniel mentioned, to be able to share buffers throughout DRM, > >> we at some point need to agree on a locking order, and when we do it's > >> important that we consider all relevant use-cases and workarounds. > >> > >> To me it certainly looks like the vm_ops implementations would be more > >> straightforward if we were to use mmap_sem->bo_reserve, but that would > >> again require a number of drivers to rework their copy_*_user code. > >> If those drivers, however, insisted on keeping their copy_*_user code, > >> they probably need to fix the recursive bo reservation up anyway, for > >> example by making sure not to copy from a vma that has VM_IO set. > > Yeah, completely agree. If you ask me using copy_(from|to)_user while a > > BO is reserved is currently completely forbidden. > > So if we all agree that this should be completely forbidden, and this is > what requires the bo_reserve->mmap_sem locking order, then I guess the > logical follow up is trying to enforce mmap_sem->bo_reserve and pursuade > driver maintainers to convert. Perhaps as a configurable lockdep option? The problem with adding that under e.g. CONFIG_PROVE_LOCKING or similar is that we have drivers violating it right now. And last time around this entire discussion came up no one wanted to do the work to convert drivers, since it means you get to add a slowpath to the most complex ioctl they have. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On Mon, Oct 15, 2018 at 03:33:27PM +0530, Viresh Kumar wrote: > On 11-10-18, 08:54, Jordan Crouse wrote: > > I understand what you are trying to say Jordan and I agree with those > expectations. But what I am looking for is consistency across Qcom > code using the same feature. Which enables better support for the code > going forward, etc. And if we are going to go a different way, there > must be a very good reason to do that. I agree that consistency is good. But the GPU is by design outside of the control of the genpd universe so it is by design not using the same features. It unfortunately does happen to use a similar number in an OPP binding to construct the level mapping but since we can't read the cmd-db from the GMU space this is a necessary evil. > But let me try to understand the hardware a bit first.. > > > The GPU domain is completely controlled by the GMU hardware and is powered > > independently of the CPU. For various reasons the GMU can't come up with > > the vote itself so we need to construct a vote and send it during > > initialization. > > So it is the kernel which needs to send this vote on behalf of the GMU > to RPM ? Who does the vote aggregation in this case ? I thought there > has to be a single vote going from CPU side to the RPM. Isn't the GMU > vote part of that ? For clarity the GMU and GPU are in different domains. The GMU (which is controlled by the CPU) is responsible for generating and sending the vote on behalf of the GPU. From an RPMh perspective The GPU is considered to be separate from the CPU vote. Also, I probably confused you by saying that the kernel constructs the vote for the GPU. It actually constructs the mapping that the GMU can use to send the vote. This is equivalent to rpmhpd_update_level_mapping() in the rpmhpd driver if that helps. > > For this we duplicate the code that rmphd does to query the > > cmd-db and build the values using qcom,level as a guide. This is necessary > > copypasta as the alternative would be to add the hooks into genpd or add a > > side hook into the rpmhd to get the values we need and none of that is worth > > it for a few lines of walking an array. > > Initially when I was designing this qcom,level or generically called > "performance-state" thingy, I kept the values directly in the OPP node > of the consumer (the way you have done it), but then there were > discussions which forced us to move this to the genpd level. For > example, what will you do if you also need to pass voltage/current in > addition to performance-state to the RPM? StephenB actually said we > may or may not know these values and we must support both the cases. I agree that genpd has many responsibilities because it has to deal with many different devices. The GMU / GPU is built for a single purpose and the hardware has been designed accordingly. For the foreseeable future we will not need to know anything beyond the level mapping to operate the GPU. > The opp-microvolt properties of the consumer device (GPU here) should > be used to handle the regulators which are controlled by kernel itself > and so they can't additionally handle the RPMs data. And so we created > separate OPP table for the RPM and represented that as a genpd and we > now handle the aggregation in genpd core itself on behalf of all the > consumers. Yes and it should continue to be that way. This just happens to be a situation where one of the consumers is out of your area of control by design. > > qcom,level serves the purpose for us in this case because we can get the > > value > > we need and construct the vote. If we move to using required-opp, thats just > > another step of parsing for the driver and > > The OPP core shall have all the infrastructure to parse these things > and we should keep all such code there to avoid duplication. Using required-opp to look up another opp to look up the qcom,level is by definition additional parsing. It doesn't imply that there would be code duplication. > > it establishes a relationship with > > rmphd on the CPU that shouldn't exist. > > Sure. I am not suggesting that the representation in software should > be different from what the hardware is, maybe I didn't understood the > hardware design well. > > > I do see a good argument for using the symbolic bindings (i.e. > > RPMH_REGULATOR_LEVEL_TURBO_L1) and we can do that easily but beyond that I > > don't think that we need the extra parsing step. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: VSDB yCBCr420 Deep Color mode bit definitions
Regards
Shashank
On 10/15/2018 4:39 PM, Jani Nikula wrote:
On Mon, 15 Oct 2018, Jani Nikula wrote:
On Fri, 05 Oct 2018, clinton.a.tay...@intel.com wrote:
From: Clint Taylor
HDMI Forum VSDB YCBCR420 deep color capability bits are 2:0. Correct
definitions in the header for the mask to work correctly.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107893
Signed-off-by: Clint Taylor
When posting fixes like this, please do git blame on the stuff you're
fixing, and add Fixes: tag and a bunch of Cc's. It'll help us propagate
the fix to stable kernels and get feedback from the authors and
reviewers. 'dim fixes' will help you with this:
$ dim fixes e6a9a2c3dc437
Fixes: e6a9a2c3dc43 ("drm/edid: parse ycbcr 420 deep color information")
Cc: Ville Syrjälä
Cc: Jose Abreu
Cc: Shashank Sharma
Cc: Gustavo Padovan
Cc: Maarten Lankhorst
Cc: Sean Paul
Cc: David Airlie
Cc: dri-devel@lists.freedesktop.org
Cc: # v4.14+
Anyway this looks sane to me,
Reviewed-by: Jani Nikula
but I'm wondering if there was some deeper meaning to the original |= in
there.
Honestly, I was considering new blocks in HDMI 2.1 spec for dc, and
parsing of those before this block, keeping |= required.
But we can always do other way around, or will take care of it when we
add code for it.
Just cross checked with the spec too,
Reviewed-by: Shashank Sharma
- Shashank
PS. It'll be useful to repost this Cc: intel-gfx just to get the CI as
we seem to be the only consumer of the stuff being fixed.
BR,
Jani.
---
drivers/gpu/drm/drm_edid.c | 2 +-
include/drm/drm_edid.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1e2b940..ff0bfc6 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4282,7 +4282,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct
drm_connector *connector,
struct drm_hdmi_info *hdmi = >display_info.hdmi;
dc_mask = db[7] & DRM_EDID_YCBCR420_DC_MASK;
- hdmi->y420_dc_modes |= dc_mask;
+ hdmi->y420_dc_modes = dc_mask;
}
static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b25d12e..e3c4048 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -214,9 +214,9 @@ struct detailed_timing {
#define DRM_EDID_HDMI_DC_Y444 (1 << 3)
/* YCBCR 420 deep color modes */
-#define DRM_EDID_YCBCR420_DC_48 (1 << 6)
-#define DRM_EDID_YCBCR420_DC_36 (1 << 5)
-#define DRM_EDID_YCBCR420_DC_30 (1 << 4)
+#define DRM_EDID_YCBCR420_DC_48 (1 << 2)
+#define DRM_EDID_YCBCR420_DC_36 (1 << 1)
+#define DRM_EDID_YCBCR420_DC_30 (1 << 0)
#define DRM_EDID_YCBCR420_DC_MASK (DRM_EDID_YCBCR420_DC_48 | \
DRM_EDID_YCBCR420_DC_36 | \
DRM_EDID_YCBCR420_DC_30)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/9] PM / OPP: dt-bindings: Add opp-interconnect-bw
On Mon, Aug 27, 2018 at 09:11:09AM -0600, Jordan Crouse wrote: > Add the "opp-interconnect-bw" property to specify the > average and peak bandwidth for an interconnect path for > a specific operating power point. A separate bandwidth > pair can be specified for each of the interconnects > defined for the device by appending the interconnect > name to the property. > > Signed-off-by: Jordan Crouse > --- > Documentation/devicetree/bindings/opp/opp.txt | 36 +++ > 1 file changed, 36 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt > b/Documentation/devicetree/bindings/opp/opp.txt > index c396c4c0af92..d714c084f36d 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -170,6 +170,11 @@ Optional properties: >functioning of the current device at the current OPP (where this property > is >present). > > +- opp-interconnect-bw-: This is an array of pairs specifying the > average > + and peak bandwidth in bytes per second for the interconnect path known by > + 'name'. This should match the name(s) specified by interconnect-names in > the > + device definition. > + I don't think this is good design with the name defined in one node and then used in the OPP table. First, '*-names' is typically the a name local to that node/device. If you had 2 instances of a device with a shared OPP table for the 2 instances, then you are going to have to make the names unique. Second, how exactly would having multiple b/w entries work? A given OPP frequency supports the sum of the b/w entries? What if some devices for b/w entries aren't currently active? This also seems like a mixture of using OPP table for setting GPU's bandwidth/freq and then the interconnect binding to set the interconnect's bandwidth. Perhaps we need a more unified approach. Not sure what that would look like though. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC
On Fri, 12 Oct 2018 08:58:23 -0400 "Kazlauskas, Nicholas" wrote: > On 10/12/2018 07:20 AM, Koenig, Christian wrote: > > Am 12.10.2018 um 11:21 schrieb Pekka Paalanen: > >> On Fri, 12 Oct 2018 07:35:57 + > >> "Koenig, Christian" wrote: > >> > >>> Am 12.10.2018 um 09:23 schrieb Pekka Paalanen: > On Wed, 10 Oct 2018 09:35:50 -0400 > "Kazlauskas, Nicholas" wrote: > > > The patches I've put out target this use case mostly because of the > > benefit for a relatively simple interface. VRR can and has been used in > > more ways that this, however. > > > > An example usecase that differs from this would actually be video > > playback. The monitor's refresh rate likely doesn't align with the video > > content rate. An API that exposes direct control over VRR (like the > > range, refresh duration, presentation timestamp) could allow the > > application to specify the content rate directly to deliver a smoother > > playback experience. For example, if you had a 24fps video and the VRR > > range was 40-60Hz you could target 48Hz via some API here. > The way that has been discussed to be implemented is that DRM page flips > would carry a target timestamp, which the driver will then meet as good > as it can. It is better to define an absolute target timestamp than a > frequency, because a timestamp can be used to synchronize with audio > and more. Mario Kleiner can tell you all about scientific use cases > that require accurate display timing, not just frequency. > >>> To summarize what information should be provided by the driver stack to > >>> make applications happy: > >>> > >>> 1. The minimum time a frame can be displayed, in other words the maximum > >>> frame rate. > >>> 2. The maximum time a frame can be displayed, in other words the minimum > >>> frame rate. > >>> 3. How much change of frame timing is allowed between frames to avoid > >>> luminescence flickering. > >>> > >>> Number 1 and 2 can also be used to signal the availability of VRR to > >>> applications, e.g. if they are identical we don't support VRR at all. > >> Hi Christian, > >> > >> "the maximum time a frame can be displayed" is perhaps not an > >> unambiguous definition. A frame can be shown indefinitely in any case. > > > > Good point. Please also note that I'm not an expert on the display stuff > > in general. > > > > My background comes more from implementing the VDPAU mediaplayer > > interface in mesa. > > > > So just throwing some ideas in here from the point of view of an > > userspace developer which wants to write a media player :) Excellent! > >> The CRTC will simply start scanning out the same frame again if there > >> is no new one. I understand what you want to say, but perhaps some > >> different words will be in order. > >> > >> I do wonder if applications really want to know the maximum acceptable > >> slew rate in timings... maybe that should be left for the drivers to > >> apply. What I'm thinking is that we have the page flip timestamp with > >> the page flip events to tell when the new FB became active. That > >> information could be extended with a time range on when the very next > >> flip could take place. Applications are already computing that > >> prediction from the flip timestamp and fixed refresh rate, but it might > >> be nice to give them the driver's opinion explicitly. Maybe the > >> tolerable slew rate is not a constant. > > > > Well it depends. I agree that the kernel should probably enforce the > > slew rate to avoid flickering for the user. That's what I was hoping if the monitor hardware does not do that already, but now it sounds like it's not possible. Another failed assumption from my side. > > But it might be beneficial for the application to know things like this. Applications should know when they could likely flip, my question is how to tell them. Is the acceptable slew rate a constant for a video mode, or does it depend on the previous refresh interval. > > > > What the application definitely needs to know is when a frame was > > actually displayed. E.g. application says I want to display this at > > point X and at some point later the kernel returns saying the frame was > > displayed at point N where in the ideal case N=X. You already get this timestamp with the DRM page flip events. We also have Wayland and X11 extensions to get it to apps. > > > > Additional to that let's please use 64bit values and nanoseconds for > > every value we use here, and NOT fps, line numbers or similar :) Seconded! I'd really favour absolute timestamps even. > > > > Regards, > > Christian. > > Flickering will really depend on the panel itself. A wider ranger is > more likely to exhibit the issue, but factors like topology, pixel > density and other display technology can affect this. > > It can be hard for a driver to guess at all of this. For many panels > it'd be difficult to notice
[PATCH v2] drm: Get ref on CRTC commit object when waiting for flip_done
From: Leo Li
This fixes a general protection fault, caused by accessing the contents
of a flip_done completion object that has already been freed. It occurs
due to the preemption of a non-blocking commit worker thread W by
another commit thread X. X continues to clear its atomic state at the
end, destroying the CRTC commit object that W still needs. Switching
back to W and accessing the commit objects then leads to bad results.
Worker W becomes preemptable when waiting for flip_done to complete. At
this point, a frequently occurring commit thread X can take over. Here's
an example where W is a worker thread that flips on both CRTCs, and X
does a legacy cursor update on both CRTCs:
...
1. W does flip work
2. W runs commit_hw_done()
3. W waits for flip_done on CRTC 1
4. > flip_done for CRTC 1 completes
5. W finishes waiting for CRTC 1
6. W waits for flip_done on CRTC 2
7. > Preempted by X
8. > flip_done for CRTC 2 completes
9. X atomic_check: hw_done and flip_done are complete on all CRTCs
10. X updates cursor on both CRTCs
11. X destroys atomic state
12. X done
13. > Switch back to W
14. W waits for flip_done on CRTC 2
15. W raises general protection fault
The error looks like so:
general protection fault: [#1] PREEMPT SMP PTI
**snip**
Call Trace:
lock_acquire+0xa2/0x1b0
_raw_spin_lock_irq+0x39/0x70
wait_for_completion_timeout+0x31/0x130
drm_atomic_helper_wait_for_flip_done+0x64/0x90 [drm_kms_helper]
amdgpu_dm_atomic_commit_tail+0xcae/0xdd0 [amdgpu]
commit_tail+0x3d/0x70 [drm_kms_helper]
process_one_work+0x212/0x650
worker_thread+0x49/0x420
kthread+0xfb/0x130
ret_from_fork+0x3a/0x50
Modules linked in: x86_pkg_temp_thermal amdgpu(O) chash(O)
gpu_sched(O) drm_kms_helper(O) syscopyarea sysfillrect sysimgblt
fb_sys_fops ttm(O) drm(O)
Note that i915 has this issue masked, since hw_done is signaled after
waiting for flip_done. Doing so will block the cursor update from
happening until hw_done is signaled, preventing the cursor commit from
destroying the state.
v2: The reference on the commit object needs to be obtained before
hw_done() is signaled, since that's the point where another commit
is allowed to modify the state. Assuming that the
new_crtc_state->commit object still exists within flip_done() is
incorrect.
Fix by getting a reference in setup_commit(), and releasing it
during default_clear().
Signed-off-by: Leo Li
---
Sending again, this time to the correct mailing list :)
Leo
drivers/gpu/drm/drm_atomic.c| 5 +
drivers/gpu/drm/drm_atomic_helper.c | 12
include/drm/drm_atomic.h| 11 +++
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3eb061e..12ae917 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -174,6 +174,11 @@ void drm_atomic_state_default_clear(struct
drm_atomic_state *state)
state->crtcs[i].state = NULL;
state->crtcs[i].old_state = NULL;
state->crtcs[i].new_state = NULL;
+
+ if (state->crtcs[i].commit) {
+ drm_crtc_commit_put(state->crtcs[i].commit);
+ state->crtcs[i].commit = NULL;
+ }
}
for (i = 0; i < config->num_total_plane; i++) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 80be74d..1bb4c31 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1408,15 +1408,16 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
{
- struct drm_crtc_state *new_crtc_state;
struct drm_crtc *crtc;
int i;
- for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
- struct drm_crtc_commit *commit = new_crtc_state->commit;
+ for (i = 0; i < dev->mode_config.num_crtc; i++) {
+ struct drm_crtc_commit *commit = old_state->crtcs[i].commit;
int ret;
- if (!commit)
+ crtc = old_state->crtcs[i].ptr;
+
+ if (!crtc || !commit)
continue;
ret = wait_for_completion_timeout(>flip_done, 10 * HZ);
@@ -1934,6 +1935,9 @@ int drm_atomic_helper_setup_commit(struct
drm_atomic_state *state,
drm_crtc_commit_get(commit);
commit->abort_completion = true;
+
+ state->crtcs[i].commit = commit;
+ drm_crtc_commit_get(commit);
}
for_each_oldnew_connector_in_state(state, conn, old_conn_state,
new_conn_state, i) {
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows
https://bugs.freedesktop.org/show_bug.cgi?id=106175 --- Comment #28 from Nicholas Kazlauskas --- (In reply to tempel.julian from comment #27) > Is this commit related to it? > https://lists.freedesktop.org/archives/amd-gfx/2018-October/027726.html It shouldn't be. You would likely be experiencing a driver hang in this case because of the fault. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 18/18] drm/vmwgfx: Don't clear mode::type anymore
On 10/11/2018 02:16 AM, Deepak Rawat wrote:
With kernel commit 4f09c77b5c3b7, no need to clear mode::type for
user-space bug.
That commit-id will change as soon as we put the commit on vmwgfx-next.
I'd recomment citing the commit title instead.
/Thomas
Signed-off-by: Deepak Rawat
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 --
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 3 ---
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +-
5 files changed, 3 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 21dcb46a233b..12ce38bb6846 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2812,28 +2812,6 @@ vmw_kms_create_implicit_placement_property(struct
vmw_private *dev_priv,
}
-
-/**
- * vmw_kms_set_config - Wrapper around drm_atomic_helper_set_config
- *
- * @set: The configuration to set.
- *
- * The vmwgfx Xorg driver doesn't assign the mode::type member, which
- * when drm_mode_set_crtcinfo is called as part of the configuration setting
- * causes it to return incorrect crtc dimensions causing severe problems in
- * the vmwgfx modesetting. So explicitly clear that member before calling
- * into drm_atomic_helper_set_config.
- */
-int vmw_kms_set_config(struct drm_mode_set *set,
- struct drm_modeset_acquire_ctx *ctx)
-{
- if (set && set->mode)
- set->mode->type = 0;
-
- return drm_atomic_helper_set_config(set, ctx);
-}
-
-
/**
* vmw_kms_suspend - Save modesetting state and turn modesetting off.
*
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 3e8b8b3d33aa..bc5bccf1db42 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -572,9 +572,6 @@ int vmw_kms_stdu_dma(struct vmw_private *dev_priv,
bool interruptible,
struct drm_crtc *crtc);
-int vmw_kms_set_config(struct drm_mode_set *set,
- struct drm_modeset_acquire_ctx *ctx);
-
int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
/**
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index 723578117191..b909fbc540ff 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -233,7 +233,7 @@ static const struct drm_crtc_funcs vmw_legacy_crtc_funcs = {
.reset = vmw_du_crtc_reset,
.atomic_duplicate_state = vmw_du_crtc_duplicate_state,
.atomic_destroy_state = vmw_du_crtc_destroy_state,
- .set_config = vmw_kms_set_config,
+ .set_config = drm_atomic_helper_set_config,
};
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 77521075e803..09118ca199fa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -353,7 +353,7 @@ static const struct drm_crtc_funcs
vmw_screen_object_crtc_funcs = {
.reset = vmw_du_crtc_reset,
.atomic_duplicate_state = vmw_du_crtc_duplicate_state,
.atomic_destroy_state = vmw_du_crtc_destroy_state,
- .set_config = vmw_kms_set_config,
+ .set_config = drm_atomic_helper_set_config,
.page_flip = vmw_sou_crtc_page_flip,
};
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index ae9063482141..b5245a9b5608 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -990,7 +990,7 @@ static const struct drm_crtc_funcs vmw_stdu_crtc_funcs = {
.reset = vmw_du_crtc_reset,
.atomic_duplicate_state = vmw_du_crtc_duplicate_state,
.atomic_destroy_state = vmw_du_crtc_destroy_state,
- .set_config = vmw_kms_set_config,
+ .set_config = drm_atomic_helper_set_config,
.page_flip = vmw_stdu_crtc_page_flip,
};
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 10/18] drm/vmwgfx: Updated comment for stdu plane update
On 10/11/2018 02:16 AM, Deepak Rawat wrote:
Update the commet to sync with code.
Minor typo above^
Signed-off-by: Deepak Rawat
---
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 64d11af2b81b..1821c4be0ef4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1653,7 +1653,6 @@ static int vmw_stdu_plane_update_surface(struct
vmw_private *dev_priv,
/**
* vmw_stdu_primary_plane_atomic_update - formally switches STDU to new plane
- *
* @plane: display plane
* @old_state: Only used to get crtc info
*
@@ -1674,10 +1673,7 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane
*plane,
struct vmw_private *dev_priv;
int ret;
- /*
-* We cannot really fail this function, so if we do, then output an
-* error and maintain consistent atomic state.
-*/
+ /* If somehow gets a device error, maintain consistent atomic state */
Perhaps: "If we somehow get", or "In case of"
if (crtc && plane->state->fb) {
struct vmw_framebuffer *vfb =
vmw_framebuffer_to_vfb(plane->state->fb);
@@ -1706,12 +1702,7 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane
*plane,
stdu = vmw_crtc_to_stdu(crtc);
dev_priv = vmw_priv(crtc->dev);
- /*
-* When disabling a plane, CRTC and FB should always be NULL
-* together, otherwise it's an error.
-* Here primary plane is being disable so blank the screen
-* target display unit, if not already done.
-*/
+ /* Blank STDU when fb and crtc are NULL */
if (!stdu->defined)
return;
@@ -1726,11 +1717,8 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane *plane,
return;
}
+ /* In case of error vblank event is sent in vmw_du_crtc_atomic_flush */
event = crtc->state->event;
- /*
-* In case of failure and other cases, vblank event will be sent in
-* vmw_du_crtc_atomic_flush.
-*/
if (event && fence) {
struct drm_file *file_priv = event->base.file_priv;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/18] drm: Add a new plane property to send damage during plane update
With some nitpicks sent out previously
Reviewed-by: Thomas Hellstrom
On 10/11/2018 02:16 AM, Deepak Rawat wrote:
From: Lukasz Spintzyk
FB_DAMAGE_CLIPS is an optional plane property to mark damaged regions
on the plane in framebuffer coordinates of the framebuffer attached to
the plane.
The layout of blob data is simply an array of "struct drm_mode_rect".
Unlike plane src coordinates, damage clips are not in 16.16 fixed point.
As plane src in framebuffer cannot be negative so are damage clips. In
damage clip, x1/y1 are inclusive and x2/y2 are exclusive.
This patch also exports the kernel internal drm_rect to userspace as
drm_mode_rect. This is because "struct drm_clip_rect" is not sufficient
to represent damage for current plane size.
Driver which are interested in enabling FB_DAMAGE_CLIPS property for a
plane should enable this property using drm_plane_enable_damage_clips.
v2:
- Input validation on damage clips against framebuffer size.
- Doc update, other minor changes.
Cc: ville.syrj...@linux.intel.com
Cc: Daniel Vetter
Cc: Pekka Paalanen
Cc: Daniel Stone
Signed-off-by: Lukasz Spintzyk
Signed-off-by: Deepak Rawat
---
Documentation/gpu/drm-kms.rst | 12 +
drivers/gpu/drm/Makefile| 3 +-
drivers/gpu/drm/drm_atomic.c| 22
drivers/gpu/drm/drm_atomic_helper.c | 3 ++
drivers/gpu/drm/drm_atomic_uapi.c | 13 +
drivers/gpu/drm/drm_damage_helper.c | 83 +
drivers/gpu/drm/drm_mode_config.c | 6 +++
include/drm/drm_damage_helper.h | 39 ++
include/drm/drm_mode_config.h | 9
include/drm/drm_plane.h | 40 ++
include/uapi/drm/drm_mode.h | 19 +++
11 files changed, 248 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_damage_helper.c
create mode 100644 include/drm/drm_damage_helper.h
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..6c3e89e324f8 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -554,6 +554,18 @@ Plane Composition Properties
.. kernel-doc:: drivers/gpu/drm/drm_blend.c
:export:
+FB_DAMAGE_CLIPS
+~~~
+
+.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
+ :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
+ :export:
+
+.. kernel-doc:: include/drm/drm_damage_helper.h
+ :internal:
+
Color Management Properties
---
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index bc6a16a3c36e..2ed4c66ca849 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -36,7 +36,8 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
drm_simple_kms_helper.o drm_modeset_helper.o \
- drm_scdc_helper.o drm_gem_framebuffer_helper.o
+ drm_scdc_helper.o drm_gem_framebuffer_helper.o \
+ drm_damage_helper.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 2870ae205237..ff488b47b5bb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -516,6 +516,8 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
unsigned int fb_width, fb_height;
+ struct drm_mode_rect *clips;
+ uint32_t num_clips;
int ret;
/* either *both* CRTC and FB must be set, or neither */
@@ -585,6 +587,26 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
return -ENOSPC;
}
+ clips = drm_plane_get_damage_clips(state);
+ num_clips = drm_plane_get_damage_clips_count(state);
+
+ /* Make sure damage clips are valid and inside the fb. */
+ while (num_clips > 0) {
+ if (clips->x1 >= clips->x2 ||
+ clips->y1 >= clips->y2 ||
+ clips->x1 < 0 ||
+ clips->y1 < 0 ||
+ clips->x2 > fb_width ||
+ clips->y2 > fb_height) {
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] invalid damage clip %d %d %d
%d\n",
+plane->base.id, plane->name, clips->x1,
+clips->y1, clips->x2, clips->y2);
+ return -EINVAL;
+ }
+ clips++;
+ num_clips--;
+ }
+
if (plane_switching_crtc(state->state, plane, state)) {
DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
plane->base.id, plane->name);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
[Bug 108322] RX580 Display flickering after waking from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=108322 --- Comment #13 from Nicholas Kazlauskas --- (In reply to bmilreu from comment #10) > https://www.phoronix.com/scan.php?page=news_item=AMD-V4-Adaptive-VRR- > FreeSync > Is this maybe related? This would be unrelated - those patches are still in review and not part of any tree yet. Luminance flickering is also a bit different from typical flickering. It is a more subtle effect and doesn't have any sort of visual corruption associated with it - lines, the screen displaying black, etc. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 15/18] drm/vmwgfx: Update comments for sou plane update function
On 10/11/2018 02:16 AM, Deepak Rawat wrote:
Update comments to sync with code.
Signed-off-by: Deepak Rawat
---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 1cef622a779e..14bcd4db4f9c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -771,6 +771,7 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane *plane,
struct vmw_fence_obj *fence = NULL;
int ret;
+ /* If somehow gets a device error maintain consistent atomic state */
Same here, "If we somehow get" or "In case of"
if (crtc && plane->state->fb) {
struct vmw_private *dev_priv = vmw_priv(crtc->dev);
struct vmw_framebuffer *vfb =
@@ -783,28 +784,15 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane
*plane,
ret = vmw_sou_plane_update_surface(dev_priv, plane,
old_state, vfb,
);
-
- /*
-* We cannot really fail this function, so if we do, then output
-* an error and maintain consistent atomic state.
-*/
if (ret != 0)
DRM_ERROR("Failed to update screen.\n");
} else {
- /*
-* When disabling a plane, CRTC and FB should always be NULL
-* together, otherwise it's an error.
-* Here primary plane is being disable so should really blank
-* the screen object display unit, if not already done.
-*/
+ /* Do nothing when fb and crtc is NULL (blank crtc) */
return;
}
+ /* For error case vblank event is sent from vmw_du_crtc_atomic_flush */
event = crtc->state->event;
- /*
-* In case of failure and other cases, vblank event will be sent in
-* vmw_du_crtc_atomic_flush.
-*/
if (event && fence) {
struct drm_file *file_priv = event->base.file_priv;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
On 10/15/2018 12:55 PM, Koenig, Christian wrote: Am 15.10.2018 um 12:42 schrieb Thomas Hellstrom: On 10/15/2018 10:43 AM, Koenig, Christian wrote: Hi Thomas, 1. You need the a lock with the order lock->mmap_sem, cause otherwise you run into trouble when drm_vma_node_unmap() needs to be called. Why is this? unmap_mapping_range() never takes the mmap_sem. It takes a finer-level per-address-space lock. Well, that is interesting. Where do we actually then have the order of reservation->mmap_sem defined? TTM doesn't. But some drivers using TTM do, which prompted the wording in the fault handler comments. But as Daniel mentioned, to be able to share buffers throughout DRM, we at some point need to agree on a locking order, and when we do it's important that we consider all relevant use-cases and workarounds. To me it certainly looks like the vm_ops implementations would be more straightforward if we were to use mmap_sem->bo_reserve, but that would again require a number of drivers to rework their copy_*_user code. If those drivers, however, insisted on keeping their copy_*_user code, they probably need to fix the recursive bo reservation up anyway, for example by making sure not to copy from a vma that has VM_IO set. Yeah, completely agree. If you ask me using copy_(from|to)_user while a BO is reserved is currently completely forbidden. So if we all agree that this should be completely forbidden, and this is what requires the bo_reserve->mmap_sem locking order, then I guess the logical follow up is trying to enforce mmap_sem->bo_reserve and pursuade driver maintainers to convert. Perhaps as a configurable lockdep option? [SNIP] /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/stm: ltdc: Solve issue on pixel clock & data enable polarity
Le lun. 24 sept. 2018 à 14:05, Yannick Fertré a écrit :
>
> Wrong flags used for set the pixel clock & data enable polarities.
> Add trace for polarities of hsync, vsync, data enabled & pixel clock.
>
> Signed-off-by: Yannick Fertré
Reviewed-by: Benjamin Gaignard
> ---
> drivers/gpu/drm/stm/ltdc.c | 23 +++
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 808d9fb..f671abc 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -517,7 +517,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> struct videomode vm;
> u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h;
> u32 total_width, total_height;
> - u32 val;
> + u32 val = 0;
>
> drm_display_mode_to_videomode(mode, );
>
> @@ -538,7 +538,22 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc
> *crtc)
> total_height = accum_act_h + vm.vfront_porch;
>
> /* Configures the HS, VS, DE and PC polarities. Default Active Low */
> - val = 0;
> + if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
> + DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is
> active low");
> + if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
> + DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is
> active high");
> + if (vm.flags & DISPLAY_FLAGS_VSYNC_LOW)
> + DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active
> low");
> + if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
> + DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active
> high");
> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> + DRM_DEBUG_DRIVER("Data Enable polarity is active low");
> + if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> + DRM_DEBUG_DRIVER("Data Enable polarity is active high");
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + DRM_DEBUG_DRIVER("Pixel clock polarity is active low");
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> + DRM_DEBUG_DRIVER("Pixel clock polarity is active high");
>
> if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
> val |= GCR_HSPOL;
> @@ -546,10 +561,10 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc
> *crtc)
> if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
> val |= GCR_VSPOL;
>
> - if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> val |= GCR_DEPOL;
>
> - if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> val |= GCR_PCPOL;
>
> reg_update_bits(ldev->regs, LTDC_GCR,
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable
Le lun. 24 sept. 2018 à 13:59, Yannick Fertré a écrit :
>
> Add missing flags for pixel clock & data enable polarities.
> These flags are similar to other synchronization signals (hsync, vsync...).
>
> Signed-off-by: Yannick Fertré
Reviewed-by: Benjamin Gaignard
> ---
> drivers/gpu/drm/drm_modes.c | 19 ++-
> include/uapi/drm/drm_mode.h | 6 ++
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 02db9ac..596f8b3 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
> * according to the hdisplay, vdisplay, vrefresh.
> * It is based from the VESA(TM) Coordinated Video Timing Generator by
> * Graham Loveridge April 9, 2003 available at
> - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> *
> * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
> * What I have done is to translate it by using integer calculation.
> @@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct
> videomode *vm,
> dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> + dmode->flags |= DRM_MODE_FLAG_PPIXCLK;
> + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + dmode->flags |= DRM_MODE_FLAG_NPIXCLK;
> + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> + dmode->flags |= DRM_MODE_FLAG_PDATAEN;
> + else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> + dmode->flags |= DRM_MODE_FLAG_NDE;
> +
> drm_mode_set_name(dmode);
> }
> EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
> @@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct
> drm_display_mode *dmode,
> vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
> if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
> vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
> + if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
> + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> + else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
> + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> + if (dmode->flags & DRM_MODE_FLAG_PDE)
> + vm->flags |= DISPLAY_FLAGS_DE_HIGH;
> + else if (dmode->flags & DRM_MODE_FLAG_NDE)
> + vm->flags |= DISPLAY_FLAGS_DE_LOW;
> }
> EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index d3e0fe3..b335a17 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -89,6 +89,12 @@ extern "C" {
> #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
> #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF(8<<14)
>
> +/* flags for polarity clock & data enable polarities */
> +#define DRM_MODE_FLAG_PPIXDATA (1 << 19)
> +#define DRM_MODE_FLAG_NPIXDATA (1 << 20)
> +#define DRM_MODE_FLAG_PDE (1 << 21)
> +#define DRM_MODE_FLAG_NDE (1 << 22)
> +
> /* Picture aspect ratio options */
> #define DRM_MODE_PICTURE_ASPECT_NONE 0
> #define DRM_MODE_PICTURE_ASPECT_4_31
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: VSDB yCBCr420 Deep Color mode bit definitions
On Mon, 15 Oct 2018, Jani Nikula wrote:
> On Fri, 05 Oct 2018, clinton.a.tay...@intel.com wrote:
>> From: Clint Taylor
>>
>> HDMI Forum VSDB YCBCR420 deep color capability bits are 2:0. Correct
>> definitions in the header for the mask to work correctly.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107893
>> Signed-off-by: Clint Taylor
>
> When posting fixes like this, please do git blame on the stuff you're
> fixing, and add Fixes: tag and a bunch of Cc's. It'll help us propagate
> the fix to stable kernels and get feedback from the authors and
> reviewers. 'dim fixes' will help you with this:
>
> $ dim fixes e6a9a2c3dc437
> Fixes: e6a9a2c3dc43 ("drm/edid: parse ycbcr 420 deep color information")
> Cc: Ville Syrjälä
> Cc: Jose Abreu
> Cc: Shashank Sharma
> Cc: Gustavo Padovan
> Cc: Maarten Lankhorst
> Cc: Sean Paul
> Cc: David Airlie
> Cc: dri-devel@lists.freedesktop.org
> Cc: # v4.14+
>
> Anyway this looks sane to me,
>
> Reviewed-by: Jani Nikula
>
> but I'm wondering if there was some deeper meaning to the original |= in
> there.
PS. It'll be useful to repost this Cc: intel-gfx just to get the CI as
we seem to be the only consumer of the stuff being fixed.
>
>
> BR,
> Jani.
>
>
>> ---
>> drivers/gpu/drm/drm_edid.c | 2 +-
>> include/drm/drm_edid.h | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 1e2b940..ff0bfc6 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4282,7 +4282,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct
>> drm_connector *connector,
>> struct drm_hdmi_info *hdmi = >display_info.hdmi;
>>
>> dc_mask = db[7] & DRM_EDID_YCBCR420_DC_MASK;
>> -hdmi->y420_dc_modes |= dc_mask;
>> +hdmi->y420_dc_modes = dc_mask;
>> }
>>
>> static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index b25d12e..e3c4048 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -214,9 +214,9 @@ struct detailed_timing {
>> #define DRM_EDID_HDMI_DC_Y444 (1 << 3)
>>
>> /* YCBCR 420 deep color modes */
>> -#define DRM_EDID_YCBCR420_DC_48 (1 << 6)
>> -#define DRM_EDID_YCBCR420_DC_36 (1 << 5)
>> -#define DRM_EDID_YCBCR420_DC_30 (1 << 4)
>> +#define DRM_EDID_YCBCR420_DC_48 (1 << 2)
>> +#define DRM_EDID_YCBCR420_DC_36 (1 << 1)
>> +#define DRM_EDID_YCBCR420_DC_30 (1 << 0)
>> #define DRM_EDID_YCBCR420_DC_MASK (DRM_EDID_YCBCR420_DC_48 | \
>> DRM_EDID_YCBCR420_DC_36 | \
>> DRM_EDID_YCBCR420_DC_30)
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: VSDB yCBCr420 Deep Color mode bit definitions
On Fri, 05 Oct 2018, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor
>
> HDMI Forum VSDB YCBCR420 deep color capability bits are 2:0. Correct
> definitions in the header for the mask to work correctly.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107893
> Signed-off-by: Clint Taylor
When posting fixes like this, please do git blame on the stuff you're
fixing, and add Fixes: tag and a bunch of Cc's. It'll help us propagate
the fix to stable kernels and get feedback from the authors and
reviewers. 'dim fixes' will help you with this:
$ dim fixes e6a9a2c3dc437
Fixes: e6a9a2c3dc43 ("drm/edid: parse ycbcr 420 deep color information")
Cc: Ville Syrjälä
Cc: Jose Abreu
Cc: Shashank Sharma
Cc: Gustavo Padovan
Cc: Maarten Lankhorst
Cc: Sean Paul
Cc: David Airlie
Cc: dri-devel@lists.freedesktop.org
Cc: # v4.14+
Anyway this looks sane to me,
Reviewed-by: Jani Nikula
but I'm wondering if there was some deeper meaning to the original |= in
there.
BR,
Jani.
> ---
> drivers/gpu/drm/drm_edid.c | 2 +-
> include/drm/drm_edid.h | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 1e2b940..ff0bfc6 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4282,7 +4282,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct
> drm_connector *connector,
> struct drm_hdmi_info *hdmi = >display_info.hdmi;
>
> dc_mask = db[7] & DRM_EDID_YCBCR420_DC_MASK;
> - hdmi->y420_dc_modes |= dc_mask;
> + hdmi->y420_dc_modes = dc_mask;
> }
>
> static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index b25d12e..e3c4048 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -214,9 +214,9 @@ struct detailed_timing {
> #define DRM_EDID_HDMI_DC_Y444 (1 << 3)
>
> /* YCBCR 420 deep color modes */
> -#define DRM_EDID_YCBCR420_DC_48(1 << 6)
> -#define DRM_EDID_YCBCR420_DC_36(1 << 5)
> -#define DRM_EDID_YCBCR420_DC_30(1 << 4)
> +#define DRM_EDID_YCBCR420_DC_48(1 << 2)
> +#define DRM_EDID_YCBCR420_DC_36(1 << 1)
> +#define DRM_EDID_YCBCR420_DC_30(1 << 0)
> #define DRM_EDID_YCBCR420_DC_MASK (DRM_EDID_YCBCR420_DC_48 | \
> DRM_EDID_YCBCR420_DC_36 | \
> DRM_EDID_YCBCR420_DC_30)
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
Am 15.10.2018 um 12:42 schrieb Thomas Hellstrom: > On 10/15/2018 10:43 AM, Koenig, Christian wrote: >> Hi Thomas, >> >>> 1. You need the a lock with the order lock->mmap_sem, cause otherwise >>> you run into trouble when drm_vma_node_unmap() needs to be called. >>> Why is this? unmap_mapping_range() never takes the mmap_sem. It takes >>> a finer-level per-address-space lock. >> Well, that is interesting. Where do we actually then have the order of >> reservation->mmap_sem defined? > > TTM doesn't. But some drivers using TTM do, which prompted the wording > in the fault handler comments. > But as Daniel mentioned, to be able to share buffers throughout DRM, > we at some point need to agree on a locking order, and when we do it's > important that we consider all relevant use-cases and workarounds. > > To me it certainly looks like the vm_ops implementations would be more > straightforward if we were to use mmap_sem->bo_reserve, but that would > again require a number of drivers to rework their copy_*_user code. > If those drivers, however, insisted on keeping their copy_*_user code, > they probably need to fix the recursive bo reservation up anyway, for > example by making sure not to copy from a vma that has VM_IO set. Yeah, completely agree. If you ask me using copy_(from|to)_user while a BO is reserved is currently completely forbidden. [SNIP] >> We need one lock which is held while the BO backing store is replaced to >> do faults and other stuff. >> >> And we need another lock which is held while we allocate backing store >> for the BO, do command submission and prepare moves. >> >> As far as I can see the first one should be a simple mutex while the >> second is the reservation object. > > So why can't we use reservation for both, like today? We are going to need to sooner or later anyway for recoverable GPU faults. E.g. in such a handler you only need the current location of a BO and not add fences or move it around. Additional to that copy_(from|to)_user can definitely take the mmap_sem in some code paths. So if we really want to allow that while BOs are reserved we need to split up the lock anyway. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/4] drm: Add vrr_enabled property to drm CRTC
Am 15.10.2018 um 12:06 schrieb Michel Dänzer: > [SNIP] > Apart from the above, another issue is that this would give direct > control to the client on whether or not VRR should be used. But we want > to allow the user to disable VRR even if a client wants to use it, via > an RandR output property. This requires that the Xorg driver can control > whether or not VRR can actually be used, via the CRTC property added by > this patch. Oh, that is a really good argument! No objection from my side to this then, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
On 10/15/2018 10:43 AM, Koenig, Christian wrote: Hi Thomas, 1. You need the a lock with the order lock->mmap_sem, cause otherwise you run into trouble when drm_vma_node_unmap() needs to be called. Why is this? unmap_mapping_range() never takes the mmap_sem. It takes a finer-level per-address-space lock. Well, that is interesting. Where do we actually then have the order of reservation->mmap_sem defined? TTM doesn't. But some drivers using TTM do, which prompted the wording in the fault handler comments. But as Daniel mentioned, to be able to share buffers throughout DRM, we at some point need to agree on a locking order, and when we do it's important that we consider all relevant use-cases and workarounds. To me it certainly looks like the vm_ops implementations would be more straightforward if we were to use mmap_sem->bo_reserve, but that would again require a number of drivers to rework their copy_*_user code. If those drivers, however, insisted on keeping their copy_*_user code, they probably need to fix the recursive bo reservation up anyway, for example by making sure not to copy from a vma that has VM_IO set. Do you mean to temporarily pin the bo while page-table-entries are set up? That is basically what we already do by taking the reservation lock. Otherwise whatever lock we introduce in the fault handler will also need to block the bo from being moved.. Correct, let me explain it a bit differently: We need one lock which is held while the BO backing store is replaced to do faults and other stuff. And we need another lock which is held while we allocate backing store for the BO, do command submission and prepare moves. As far as I can see the first one should be a simple mutex while the second is the reservation object. So why can't we use reservation for both, like today? Thanks, Thomas Regards, Christian. Am 15.10.2018 um 10:20 schrieb Thomas Hellstrom: Hi! Interesting disscussion. Some comments below. On 10/15/2018 09:41 AM, Koenig, Christian wrote: Now amdgpu switched over Well exactly that's the point. amdgpu has *NOT* switched the order over. like i915 did yearsearlier I actually consider this a problem in i915 which should be fixed sooner or later. The locking in the fault handler as exposed by TTM is actually pointing out a deeper issue which nobody seems to have correctly understood so far. Let me explain a bit: 1. You need the a lock with the order lock->mmap_sem, cause otherwise you run into trouble when drm_vma_node_unmap() needs to be called. Why is this? unmap_mapping_range() never takes the mmap_sem. It takes a finer-level per-address-space lock. 2. But for CPU fault processing you also need a lock to determine the actual address where a BO is currently located Do you mean to temporarily pin the bo while page-table-entries are set up? i915 has solved this by separating the locks and using the reservation lock as fault processing lock. Problem is that this clashes with filling reservation objects on demand when another driver wants to use it. Could you elaborate a bit on this, what locking scenarios are conflicting etc. So what we should probably do is to fix i915 to not use the reservation object as fault processing lock any more, then separate the TTM handling into two locks as well. This sounds a bit strange to me. What we want to do in the fault handler is to temporarily pin down the object so that we can set up page-table entries, which is exactly what we accomplish with reservation objects. Otherwise whatever lock we introduce in the fault handler will also need to block the bo from being moved.. Thanks, Thomas. And last fix the the reservation object logic to allow pinning DMA-bufs on demand. Christian. Am 15.10.2018 um 08:55 schrieb Daniel Vetter: On Sun, Oct 14, 2018 at 8:20 PM Koenig, Christian wrote: Hi Thomas, that the access() handler took a shortcut when the new locking order was established There is no new locking order, the access handler is just for debugging and ignoring the correct locking order between mmap_sem and bo_reserve. That this is throwing a lockdep warning is perfectly possible. We should probably move that to a trylock instead. bo_reserve() copy_to_user() / copy_from_user() bo_unreserve() That one is illegal for a completely different reason. The address accessed by copy_to_user()/copy_from_user() could be a BO itself, so to resolve this we could end up locking a BO twice. Adding a might_lock() to the beginning of ttm_bo_vm_fault as you suggested doesn't work either, because at this point the mmap_sem is still locked. So lockdep would complain about the incorrect bo_reserve and mmap_sem order. I think Thomas' point is the one below: Christian. Am 13.10.2018 um 21:04 schrieb Thomas Hellstrom: Hi, Christian, On 10/13/2018 07:36 PM, Christian König wrote: Hi Thomas, bo_reserve() copy_to_user() / copy_from_user() bo_unreserve() That pattern is illegal for a number of
[Bug 108356] AMD DC: Mullins APU: Possible race condition between vblank interrupt and atomic pageflip
https://bugs.freedesktop.org/show_bug.cgi?id=108356 --- Comment #3 from Michel Dänzer --- FWIW, if drm_hwcomposer uses the vertical blank interrupt for flip completion, that's a drm_hwcomposer bug. Page flips generate their own events signalling completion. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/4] drm: Add vrr_enabled property to drm CRTC
On 2018-10-15 11:47 a.m., Christian König wrote: > Am 15.10.2018 um 11:40 schrieb Michel Dänzer: >> On 2018-10-13 7:38 p.m., Christian König wrote: >>> Am 12.10.2018 um 18:44 schrieb Nicholas Kazlauskas: This patch introduces the 'vrr_enabled' CRTC property to allow dynamic control over variable refresh rate support for a CRTC. This property should be treated like a content hint to the driver - if the hardware or driver is not capable of driving variable refresh timings then this is not considered an error. Capability for variable refresh rate support should be determined by querying the vrr_capable drm connector property. It is worth noting that while the property is intended for atomic use it isn't filtered from legacy userspace queries. This allows for Xorg userspace drivers to implement support. >>> I'm honestly still not convinced that a per CRTC property is actually >>> the right approach. >>> >>> What we should rather do instead is to implement a target timestamp for >>> the page flip. >> You'd have to be more specific about how the latter could completely >> replace the former. I don't see how it could. > > Each flip request send by an application gets a timestamp when the flip > should be displayed. > > If I'm not completely mistaken we already have something like that in > both DRI2 as well as DRI3. Certainly not DRI2, but for now we're not enabling VRR with that anyway. While the X11 Present extension specifies PresentOptionUST for this, support for it isn't implemented yet in xserver (as in setting the option has no effect, the X server interprets the target value as an MSC anyway). So this couldn't work before the next major release of xserver, which based on recent history could be at least about one year away. In contrast, the CRTC property based solution for the gaming use-case can work even with older xserver versions. > So as far as I can see we only need to add an extra flag that those > information are trust worth in the context of VRR as well. > > If we don't set this flag we always get the always working fallback > behavior, e.g. VRR is disabled and we have a fixed refresh rate. > > If we set this flag and the timestamp is in the past we get the VRR > behavior to display the next frame as soon as possible. > > If we set this flag and specific a specific timestamp then we get the > VRR behavior to display the frame as close as possible to the specified > timestamp. Apart from the above, another issue is that this would give direct control to the client on whether or not VRR should be used. But we want to allow the user to disable VRR even if a client wants to use it, via an RandR output property. This requires that the Xorg driver can control whether or not VRR can actually be used, via the CRTC property added by this patch. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/4] drm: Add vrr_enabled property to drm CRTC
Am 15.10.2018 um 11:40 schrieb Michel Dänzer: On 2018-10-13 7:38 p.m., Christian König wrote: Am 12.10.2018 um 18:44 schrieb Nicholas Kazlauskas: This patch introduces the 'vrr_enabled' CRTC property to allow dynamic control over variable refresh rate support for a CRTC. This property should be treated like a content hint to the driver - if the hardware or driver is not capable of driving variable refresh timings then this is not considered an error. Capability for variable refresh rate support should be determined by querying the vrr_capable drm connector property. It is worth noting that while the property is intended for atomic use it isn't filtered from legacy userspace queries. This allows for Xorg userspace drivers to implement support. I'm honestly still not convinced that a per CRTC property is actually the right approach. What we should rather do instead is to implement a target timestamp for the page flip. You'd have to be more specific about how the latter could completely replace the former. I don't see how it could. Each flip request send by an application gets a timestamp when the flip should be displayed. If I'm not completely mistaken we already have something like that in both DRI2 as well as DRI3. So as far as I can see we only need to add an extra flag that those information are trust worth in the context of VRR as well. If we don't set this flag we always get the always working fallback behavior, e.g. VRR is disabled and we have a fixed refresh rate. If we set this flag and the timestamp is in the past we get the VRR behavior to display the next frame as soon as possible. If we set this flag and specific a specific timestamp then we get the VRR behavior to display the frame as close as possible to the specified timestamp. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/4] drm: Add vrr_enabled property to drm CRTC
On 2018-10-13 7:38 p.m., Christian König wrote: > Am 12.10.2018 um 18:44 schrieb Nicholas Kazlauskas: >> This patch introduces the 'vrr_enabled' CRTC property to allow >> dynamic control over variable refresh rate support for a CRTC. >> >> This property should be treated like a content hint to the driver - >> if the hardware or driver is not capable of driving variable refresh >> timings then this is not considered an error. >> >> Capability for variable refresh rate support should be determined >> by querying the vrr_capable drm connector property. >> >> It is worth noting that while the property is intended for atomic use >> it isn't filtered from legacy userspace queries. This allows for Xorg >> userspace drivers to implement support. > > I'm honestly still not convinced that a per CRTC property is actually > the right approach. > > What we should rather do instead is to implement a target timestamp for > the page flip. You'd have to be more specific about how the latter could completely replace the former. I don't see how it could. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/8] media: sti/bdisp: don't pass GFP_DMA32 to dma_alloc_attrs
Le sam. 13 oct. 2018 à 17:18, Christoph Hellwig a écrit : > > The DMA API does its own zone decisions based on the coherent_dma_mask. > > Signed-off-by: Christoph Hellwig Reviewed-by: Benjamin Gaignard > --- > drivers/media/platform/sti/bdisp/bdisp-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c > b/drivers/media/platform/sti/bdisp/bdisp-hw.c > index 26d9fa7aeb5f..4372abbb5950 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c > @@ -510,7 +510,7 @@ int bdisp_hw_alloc_filters(struct device *dev) > > /* Allocate all the filters within a single memory page */ > size = (BDISP_HF_NB * NB_H_FILTER) + (BDISP_VF_NB * NB_V_FILTER); > - base = dma_alloc_attrs(dev, size, , GFP_KERNEL | GFP_DMA, > + base = dma_alloc_attrs(dev, size, , GFP_KERNEL, >DMA_ATTR_WRITE_COMBINE); > if (!base) > return -ENOMEM; > -- > 2.19.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/8] drm: sti: don't pass GFP_DMA32 to dma_alloc_wc
Le sam. 13 oct. 2018 à 17:17, Christoph Hellwig a écrit :
>
> The DMA API does its own zone decisions based on the coherent_dma_mask.
>
> Signed-off-by: Christoph Hellwig
Reviewed-by: Benjamin Gaignard
> ---
> drivers/gpu/drm/sti/sti_gdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> index c32de6cbf061..cdf0a1396e00 100644
> --- a/drivers/gpu/drm/sti/sti_gdp.c
> +++ b/drivers/gpu/drm/sti/sti_gdp.c
> @@ -517,7 +517,7 @@ static void sti_gdp_init(struct sti_gdp *gdp)
> /* Allocate all the nodes within a single memory page */
> size = sizeof(struct sti_gdp_node) *
> GDP_NODE_PER_FIELD * GDP_NODE_NB_BANK;
> - base = dma_alloc_wc(gdp->dev, size, _addr, GFP_KERNEL | GFP_DMA);
> + base = dma_alloc_wc(gdp->dev, size, _addr, GFP_KERNEL);
>
> if (!base) {
> DRM_ERROR("Failed to allocate memory for GDP node\n");
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/7] drm/amdgpu: update version for timeline syncobj support in amdgpu
I'm on sick leave today. But I will see what I can do later in the afternoon, Christian. Am 15.10.2018 um 11:01 schrieb Zhou, David(ChunMing): > Ping... > Christian, Could I get your RB on the series? And help me to push to drm-misc? > After that I can rebase libdrm header file based on drm-next. > > Thanks, > David Zhou > >> -Original Message- >> From: amd-gfx On Behalf Of >> Chunming Zhou >> Sent: Monday, October 15, 2018 4:56 PM >> To: dri-devel@lists.freedesktop.org >> Cc: Zhou, David(ChunMing) ; amd- >> g...@lists.freedesktop.org >> Subject: [PATCH 7/7] drm/amdgpu: update version for timeline syncobj >> support in amdgpu >> >> Signed-off-by: Chunming Zhou >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> index 6870909da926..58cba492ba55 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> @@ -70,9 +70,10 @@ >>* - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). >>* - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. >>* - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. >> + * - 3.28.0 - Add syncobj timeline support to AMDGPU_CS. >>*/ >> #define KMS_DRIVER_MAJOR 3 >> -#define KMS_DRIVER_MINOR27 >> +#define KMS_DRIVER_MINOR28 >> #define KMS_DRIVER_PATCHLEVEL 0 >> >> int amdgpu_vram_limit = 0; >> -- >> 2.17.1 >> >> ___ >> amd-gfx mailing list >> amd-...@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 7/7] drm/amdgpu: update version for timeline syncobj support in amdgpu
Ping... Christian, Could I get your RB on the series? And help me to push to drm-misc? After that I can rebase libdrm header file based on drm-next. Thanks, David Zhou > -Original Message- > From: amd-gfx On Behalf Of > Chunming Zhou > Sent: Monday, October 15, 2018 4:56 PM > To: dri-devel@lists.freedesktop.org > Cc: Zhou, David(ChunMing) ; amd- > g...@lists.freedesktop.org > Subject: [PATCH 7/7] drm/amdgpu: update version for timeline syncobj > support in amdgpu > > Signed-off-by: Chunming Zhou > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 6870909da926..58cba492ba55 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -70,9 +70,10 @@ > * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). > * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. > * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. > + * - 3.28.0 - Add syncobj timeline support to AMDGPU_CS. > */ > #define KMS_DRIVER_MAJOR 3 > -#define KMS_DRIVER_MINOR 27 > +#define KMS_DRIVER_MINOR 28 > #define KMS_DRIVER_PATCHLEVEL0 > > int amdgpu_vram_limit = 0; > -- > 2.17.1 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/amdgpu: update version for timeline syncobj support in amdgpu
Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6870909da926..58cba492ba55 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -70,9 +70,10 @@ * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. + * - 3.28.0 - Add syncobj timeline support to AMDGPU_CS. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 27 +#define KMS_DRIVER_MINOR 28 #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit = 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] drm/amdgpu: add timeline support in amdgpu CS v2
syncobj wait/signal operation is appending in command submission.
v2: separate to two kinds in/out_deps functions
Signed-off-by: Chunming Zhou
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h| 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 111 +
include/uapi/drm/amdgpu_drm.h | 9 ++
3 files changed, 112 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 447c4c7a36d6..6e4a3db56833 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -975,6 +975,11 @@ struct amdgpu_cs_chunk {
void*kdata;
};
+struct amdgpu_cs_syncobj_post_dep {
+ struct drm_syncobj *post_dep_syncobj;
+ u64 point;
+};
+
struct amdgpu_cs_parser {
struct amdgpu_device*adev;
struct drm_file *filp;
@@ -1003,9 +1008,8 @@ struct amdgpu_cs_parser {
/* user fence */
struct amdgpu_bo_list_entry uf_entry;
-
+ struct amdgpu_cs_syncobj_post_dep *post_dep_syncobjs;
unsigned num_post_dep_syncobjs;
- struct drm_syncobj **post_dep_syncobjs;
};
static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 412fac238575..7429e7941f4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -204,6 +204,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
case AMDGPU_CHUNK_ID_DEPENDENCIES:
case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
+ case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT:
+ case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
break;
default:
@@ -783,7 +785,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
*parser, int error,
>validated);
for (i = 0; i < parser->num_post_dep_syncobjs; i++)
- drm_syncobj_put(parser->post_dep_syncobjs[i]);
+ drm_syncobj_put(parser->post_dep_syncobjs[i].post_dep_syncobj);
kfree(parser->post_dep_syncobjs);
dma_fence_put(parser->fence);
@@ -1098,13 +1100,17 @@ static int amdgpu_cs_process_fence_dep(struct
amdgpu_cs_parser *p,
}
static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
-uint32_t handle)
+uint32_t handle, u64 point,
+u64 flags)
{
int r;
struct dma_fence *fence;
- r = drm_syncobj_find_fence(p->filp, handle, 0, 0, );
- if (r)
+
+ r = drm_syncobj_find_fence(p->filp, handle, point, flags, );
+ if (r) {
+ DRM_ERROR("syncobj %u failed to find fence!\n", handle);
return r;
+ }
r = amdgpu_sync_fence(p->adev, >job->sync, fence, true);
dma_fence_put(fence);
@@ -1115,46 +1121,108 @@ static int
amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p,
struct amdgpu_cs_chunk *chunk)
{
+ struct drm_amdgpu_cs_chunk_sem *deps;
unsigned num_deps;
int i, r;
- struct drm_amdgpu_cs_chunk_sem *deps;
deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
num_deps = chunk->length_dw * 4 /
sizeof(struct drm_amdgpu_cs_chunk_sem);
+ for (i = 0; i < num_deps; ++i) {
+ r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle,
+ 0, 0);
+ if (r)
+ return r;
+ }
+
+ return 0;
+}
+
+
+static int amdgpu_cs_process_syncobj_timeline_in_dep(struct amdgpu_cs_parser
*p,
+struct amdgpu_cs_chunk
*chunk)
+{
+ struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps;
+ unsigned num_deps;
+ int i, r;
+ syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata;
+ num_deps = chunk->length_dw * 4 /
+ sizeof(struct drm_amdgpu_cs_chunk_syncobj);
for (i = 0; i < num_deps; ++i) {
- r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle);
+ r = amdgpu_syncobj_lookup_and_add_to_sync(p,
+
syncobj_deps[i].handle,
+ syncobj_deps[i].point,
+
syncobj_deps[i].flags);
if (r)
return r;
}
+
return 0;
}
static int amdgpu_cs_process_syncobj_out_dep(struct
[PATCH 5/7] drm: add timeline support for syncobj export/import
user space can specify timeline point fence to export/import.
Signed-off-by: Chunming Zhou
---
drivers/gpu/drm/drm_internal.h | 4 ++
drivers/gpu/drm/drm_ioctl.c| 4 ++
drivers/gpu/drm/drm_syncobj.c | 76 ++
include/uapi/drm/drm.h | 11 +
4 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 9c4826411a3c..5ad6cbdb68ab 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private);
int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index c3c0617e6372..b80c2c28aee0 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -667,6 +667,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE,
drm_syncobj_fd_to_handle_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2,
drm_syncobj_handle_to_fd_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2,
drm_syncobj_fd_to_handle_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 17124d40532f..aed492570d85 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -683,7 +683,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file
*file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
- int fd, int handle)
+ int fd, int handle, uint64_t
point)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
@@ -697,14 +697,14 @@ static int drm_syncobj_import_sync_file_fence(struct
drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, 0, fence);
+ drm_syncobj_replace_fence(syncobj, point, fence);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return 0;
}
static int drm_syncobj_export_sync_file(struct drm_file *file_private,
- int handle, int *p_fd)
+ int handle, uint64_t point, int *p_fd)
{
int ret;
struct dma_fence *fence;
@@ -714,7 +714,7 @@ static int drm_syncobj_export_sync_file(struct drm_file
*file_private,
if (fd < 0)
return fd;
- ret = drm_syncobj_find_fence(file_private, handle, 0, 0, );
+ ret = drm_syncobj_find_fence(file_private, handle, point, 0, );
if (ret)
goto err_put_fd;
@@ -823,9 +823,14 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
return -EINVAL;
- if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+ if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) {
+ struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
+ args->handle);
+ if (!syncobj || syncobj->type != DRM_SYNCOBJ_TYPE_BINARY)
+ return -EINVAL;
return drm_syncobj_export_sync_file(file_private, args->handle,
- >fd);
+ 0, >fd);
+ }
return drm_syncobj_handle_to_fd(file_private, args->handle,
>fd);
@@ -837,6 +842,61 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev,
void *data,
{
struct drm_syncobj_handle *args = data;
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ return -ENODEV;
+
+ if
[PATCH 4/7] drm: add timeline syncobj payload query ioctl v2
user mode can query timeline payload.
v2: check return value of copy_to_user
Signed-off-by: Chunming Zhou
---
drivers/gpu/drm/drm_internal.h | 2 ++
drivers/gpu/drm/drm_ioctl.c| 2 ++
drivers/gpu/drm/drm_syncobj.c | 52 ++
include/uapi/drm/drm.h | 11 +++
4 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 566d44e3c782..9c4826411a3c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -189,6 +189,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
/* drm_framebuffer.c */
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index c0891614f516..c3c0617e6372 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE,
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl,
DRM_MASTER|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index f3f11ac2ef28..17124d40532f 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1304,3 +1304,55 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void
*data,
return ret;
}
+
+static void drm_syncobj_timeline_query_payload(struct drm_syncobj *syncobj,
+ uint64_t *point)
+{
+ if (syncobj->type != DRM_SYNCOBJ_TYPE_TIMELINE) {
+ DRM_ERROR("Normal syncobj cann't be queried!");
+ *point = 0;
+ return;
+ }
+ *point = syncobj->signal_point;
+}
+
+int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private)
+{
+ struct drm_syncobj_timeline_query *args = data;
+ struct drm_syncobj **syncobjs;
+ uint64_t *points;
+ uint32_t i;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ return -ENODEV;
+
+ if (args->count_handles == 0)
+ return -EINVAL;
+
+ ret = drm_syncobj_array_find(file_private,
+u64_to_user_ptr(args->handles),
+args->count_handles,
+);
+ if (ret < 0)
+ return ret;
+
+
+ points = kmalloc_array(args->count_handles, sizeof(*points),
+ GFP_KERNEL);
+ if (points == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ for (i = 0; i < args->count_handles; i++)
+ drm_syncobj_timeline_query_payload(syncobjs[i], [i]);
+ ret = copy_to_user(u64_to_user_ptr(args->points), points,
+ sizeof(uint64_t) * args->count_handles) ? -EFAULT :
0;
+
+ kfree(points);
+out:
+ drm_syncobj_array_free(syncobjs, args->count_handles);
+
+ return ret;
+}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index c8bc1414753d..23c4979d8a1c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -771,6 +771,15 @@ struct drm_syncobj_array {
__u32 pad;
};
+struct drm_syncobj_timeline_query {
+ __u64 handles;
+ /* points are timeline syncobjs payloads returned by query ioctl */
+ __u64 points;
+ __u32 count_handles;
+ __u32 pad;
+};
+
+
/* Query current scanout sequence number */
struct drm_crtc_get_sequence {
__u32 crtc_id; /* requested crtc_id */
@@ -928,6 +937,8 @@ extern "C" {
#define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct
drm_mode_revoke_lease)
#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAITDRM_IOWR(0xCA, struct
drm_syncobj_timeline_wait)
+#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct
drm_syncobj_timeline_query)
+
/**
* Device specific ioctls should only be in their respective headers
* The device specific ioctl range is from 0x40 to 0x9f.
--
2.17.1
___
dri-devel
[PATCH 3/7] drm: add support of syncobj timeline point wait v2
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.
v3:
userspace can specify two kinds waits::
a. Wait for time point to be completed.
b. and wait for time point to become available
Signed-off-by: Chunming Zhou
---
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/drm_ioctl.c| 2 +
drivers/gpu/drm/drm_syncobj.c | 118 -
include/uapi/drm/drm.h | 18 +
4 files changed, 124 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 67472bd77c83..f3f11ac2ef28 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct
drm_syncobj *syncobj,
}
static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+u64 point,
struct dma_fence **fence,
struct drm_syncobj_cb *cb,
drm_syncobj_func_t func)
{
int ret;
- ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+ ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;
@@ -143,7 +144,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct
drm_syncobj *syncobj,
*/
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
- drm_syncobj_search_fence(syncobj, 0, 0, fence);
+ drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
return 1;
spin_lock(>lock);
@@ -354,13 +355,17 @@ void drm_syncobj_replace_fence(struct drm_syncobj
*syncobj,
drm_syncobj_create_signal_pt(syncobj, fence, pt_value);
if (fence) {
struct drm_syncobj_cb *cur, *tmp;
+ struct list_head cb_list;
+
+ INIT_LIST_HEAD(_list);
spin_lock(>lock);
- list_for_each_entry_safe(cur, tmp, >cb_list, node) {
+ list_splice_init(>cb_list, _list);
+ spin_unlock(>lock);
+ list_for_each_entry_safe(cur, tmp, _list, node) {
list_del_init(>node);
cur->func(syncobj, cur);
}
- spin_unlock(>lock);
}
}
EXPORT_SYMBOL(drm_syncobj_replace_fence);
@@ -856,6 +861,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+ u64point;
};
static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -873,12 +879,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
- drm_syncobj_search_fence(syncobj, 0, 0, >fence);
+ drm_syncobj_search_fence(syncobj, wait->point, 0, >fence);
wake_up_process(wait->task);
}
static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj
**syncobjs,
+ void __user *user_points,
uint32_t count,
[PATCH 2/7] drm: add syncobj timeline support v8
This patch is for VK_KHR_timeline_semaphore extension, semaphore is called
syncobj in kernel side:
This extension introduces a new type of syncobj that has an integer payload
identifying a point in a timeline. Such timeline syncobjs support the
following operations:
* CPU query - A host operation that allows querying the payload of the
timeline syncobj.
* CPU wait - A host operation that allows a blocking wait for a
timeline syncobj to reach a specified value.
* Device wait - A device operation that allows waiting for a
timeline syncobj to reach a specified value.
* Device signal - A device operation that allows advancing the
timeline syncobj to a specified value.
v1:
Since it's a timeline, that means the front time point(PT) always is signaled
before the late PT.
a. signal PT design:
Signal PT fence N depends on PT[N-1] fence and signal opertion fence, when
PT[N] fence is signaled,
the timeline will increase to value of PT[N].
b. wait PT design:
Wait PT fence is signaled by reaching timeline point value, when timeline is
increasing, will compare
wait PTs value with new timeline value, if PT value is lower than timeline
value, then wait PT will be
signaled, otherwise keep in list. syncobj wait operation can wait on any point
of timeline,
so need a RB tree to order them. And wait PT could ahead of signal PT, we need
a sumission fence to
perform that.
v2:
1. remove unused DRM_SYNCOBJ_CREATE_TYPE_NORMAL. (Christian)
2. move unexposed denitions to .c file. (Daniel Vetter)
3. split up the change to drm_syncobj_find_fence() in a separate patch.
(Christian)
4. split up the change to drm_syncobj_replace_fence() in a separate patch.
5. drop the submission_fence implementation and instead use wait_event() for
that. (Christian)
6. WARN_ON(point != 0) for NORMAL type syncobj case. (Daniel Vetter)
v3:
1. replace normal syncobj with timeline implemenation. (Vetter and Christian)
a. normal syncobj signal op will create a signal PT to tail of signal pt
list.
b. normal syncobj wait op will create a wait pt with last signal point, and
this wait PT is only signaled by related signal point PT.
2. many bug fix and clean up
3. stub fence moving is moved to other patch.
v4：
1. fix RB tree loop with while(node=rb_first(...)). (Christian)
2. fix syncobj lifecycle. (Christian)
3. only enable_signaling when there is wait_pt. (Christian)
4. fix timeline path issues.
5. write a timeline test in libdrm
v5: (Christian)
1. semaphore is called syncobj in kernel side.
2. don't need 'timeline' characters in some function name.
3. keep syncobj cb.
v6: (Christian)
1. merge syncobj_timeline to syncobj structure.
2. simplify some check sentences.
3. some misc change.
4. fix CTS failed issue.
v7: (Christian)
1. error handling when creating signal pt.
2. remove timeline naming in func.
3. export flags in find_fence.
4. allow reset timeline.
v8:
1. use wait_event_interruptible without timeout
2. rename _TYPE_INDIVIDUAL to _TYPE_BINARY
individual syncobj is tested by ./deqp-vk -n dEQP-VK*semaphore*
timeline syncobj is tested by ./amdgpu_test -s 9
Signed-off-by: Chunming Zhou
Cc: Christian Konig
Cc: Dave Airlie
Cc: Daniel Rakos
Cc: Daniel Vetter
Reviewed-by: Christian König
---
drivers/gpu/drm/drm_syncobj.c | 287 ++---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
include/drm/drm_syncobj.h | 65 ++---
include/uapi/drm/drm.h | 1 +
4 files changed, 281 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index f796c9fc3858..67472bd77c83 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,6 +56,9 @@
#include "drm_internal.h"
#include
+/* merge normal syncobj to timeline syncobj, the point interval is 1 */
+#define DRM_SYNCOBJ_BINARY_POINT 1
+
struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -82,6 +85,11 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops
= {
.release = drm_syncobj_stub_fence_release,
};
+struct drm_syncobj_signal_pt {
+ struct dma_fence_array *base;
+ u64value;
+ struct list_head list;
+};
/**
* drm_syncobj_find - lookup and reference a sync object.
@@ -124,8 +132,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct
drm_syncobj *syncobj,
{
int ret;
- *fence = drm_syncobj_fence_get(syncobj);
- if (*fence)
+ ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+ if (!ret)
return 1;
spin_lock(>lock);
@@ -133,10 +141,12 @@ static int drm_syncobj_fence_get_or_add_callback(struct
drm_syncobj *syncobj,
* have the lock, try one more time just to be sure we don't add a
* callback when a fence has already been set.
*/
- if (syncobj->fence) {
- *fence =
[PATCH 1/7] drm: add flags to drm_syncobj_find_fence
flags can be used by driver to decide whether need to block wait submission.
Signed-off-by: Chunming Zhou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/drm_syncobj.c | 4 ++--
drivers/gpu/drm/v3d/v3d_gem.c | 4 ++--
drivers/gpu/drm/vc4/vc4_gem.c | 2 +-
include/drm/drm_syncobj.h | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d9d2ede96490..412fac238575 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1102,7 +1102,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct
amdgpu_cs_parser *p,
{
int r;
struct dma_fence *fence;
- r = drm_syncobj_find_fence(p->filp, handle, 0, );
+ r = drm_syncobj_find_fence(p->filp, handle, 0, 0, );
if (r)
return r;
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e9ce623d049e..f796c9fc3858 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -235,7 +235,7 @@ static int drm_syncobj_assign_null_handle(struct
drm_syncobj *syncobj)
* dma_fence_put().
*/
int drm_syncobj_find_fence(struct drm_file *file_private,
- u32 handle, u64 point,
+ u32 handle, u64 point, u64 flags,
struct dma_fence **fence)
{
struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle);
@@ -506,7 +506,7 @@ static int drm_syncobj_export_sync_file(struct drm_file
*file_private,
if (fd < 0)
return fd;
- ret = drm_syncobj_find_fence(file_private, handle, 0, );
+ ret = drm_syncobj_find_fence(file_private, handle, 0, 0, );
if (ret)
goto err_put_fd;
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 70c54774400b..97477879d3d4 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -521,12 +521,12 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
kref_init(>refcount);
ret = drm_syncobj_find_fence(file_priv, args->in_sync_bcl,
-0, >bin.in_fence);
+0, 0, >bin.in_fence);
if (ret == -EINVAL)
goto fail;
ret = drm_syncobj_find_fence(file_priv, args->in_sync_rcl,
-0, >render.in_fence);
+0, 0, >render.in_fence);
if (ret == -EINVAL)
goto fail;
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 5b22e996af6c..251198194c38 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -1173,7 +1173,7 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
if (args->in_sync) {
ret = drm_syncobj_find_fence(file_priv, args->in_sync,
-0, _fence);
+0, 0, _fence);
if (ret)
goto fail;
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 425432b85a87..2eda44def639 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -134,7 +134,7 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file
*file_private,
void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, u64 point,
struct dma_fence *fence);
int drm_syncobj_find_fence(struct drm_file *file_private,
- u32 handle, u64 point,
+ u32 handle, u64 point, u64 flags,
struct dma_fence **fence);
void drm_syncobj_free(struct kref *kref);
int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108260] [Regression?] [powerplay] Failed to retrieve minimum clocks. 4.19-rc6+
https://bugs.freedesktop.org/show_bug.cgi?id=108260 Daniel Exner changed: What|Removed |Added CC||dex+fdobugzilla@dragonslave ||.de --- Comment #3 from Daniel Exner --- Can confirm the same message on: 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 470/480/570/570X/580/580X] (rev e7) (prog-if 00 [VGA controller]) Subsystem: Tul Corporation / PowerColor Ellesmere [Radeon RX 470/480/570/570X/580/580X] Kernel 4.19.0-rc8 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
Hi Thomas, > 1. You need the a lock with the order lock->mmap_sem, cause otherwise > you run into trouble when drm_vma_node_unmap() needs to be called. > Why is this? unmap_mapping_range() never takes the mmap_sem. It takes > a finer-level per-address-space lock. Well, that is interesting. Where do we actually then have the order of reservation->mmap_sem defined? > Do you mean to temporarily pin the bo while page-table-entries are set > up? That is basically what we already do by taking the reservation lock. > Otherwise whatever lock we introduce in the fault handler will also > need to block the bo from being moved.. Correct, let me explain it a bit differently: We need one lock which is held while the BO backing store is replaced to do faults and other stuff. And we need another lock which is held while we allocate backing store for the BO, do command submission and prepare moves. As far as I can see the first one should be a simple mutex while the second is the reservation object. Regards, Christian. Am 15.10.2018 um 10:20 schrieb Thomas Hellstrom: > Hi! > > Interesting disscussion. Some comments below. > > On 10/15/2018 09:41 AM, Koenig, Christian wrote: >>> Now amdgpu switched over >> Well exactly that's the point. amdgpu has *NOT* switched the order over. >> >>> like i915 did yearsearlier >> I actually consider this a problem in i915 which should be fixed sooner >> or later. >> >> The locking in the fault handler as exposed by TTM is actually pointing >> out a deeper issue which nobody seems to have correctly understood so >> far. >> >> Let me explain a bit: >> 1. You need the a lock with the order lock->mmap_sem, cause otherwise >> you run into trouble when drm_vma_node_unmap() needs to be called. > Why is this? unmap_mapping_range() never takes the mmap_sem. It takes > a finer-level per-address-space lock. > >> 2. But for CPU fault processing you also need a lock to determine the >> actual address where a BO is currently located > Do you mean to temporarily pin the bo while page-table-entries are set > up? > >> >> i915 has solved this by separating the locks and using the reservation >> lock as fault processing lock. >> >> Problem is that this clashes with filling reservation objects on demand >> when another driver wants to use it. > > Could you elaborate a bit on this, what locking scenarios are > conflicting etc. > >> >> So what we should probably do is to fix i915 to not use the reservation >> object as fault processing lock any more, then separate the TTM handling >> into two locks as well. > > This sounds a bit strange to me. What we want to do in the fault > handler is to temporarily pin down the object so that we can set up > page-table entries, which is exactly what we accomplish with > reservation objects. Otherwise whatever lock we introduce in the fault > handler will also need to block the bo from being moved.. > > Thanks, Thomas. > >> And last fix the the reservation object logic to allow pinning DMA-bufs >> on demand. >> >> Christian. >> >> Am 15.10.2018 um 08:55 schrieb Daniel Vetter: >>> On Sun, Oct 14, 2018 at 8:20 PM Koenig, Christian >>> wrote: Hi Thomas, > that the access() handler took a shortcut when the new locking order > was established There is no new locking order, the access handler is just for debugging and ignoring the correct locking order between mmap_sem and bo_reserve. That this is throwing a lockdep warning is perfectly possible. We should probably move that to a trylock instead. > bo_reserve() > copy_to_user() / copy_from_user() > bo_unreserve() That one is illegal for a completely different reason. The address accessed by copy_to_user()/copy_from_user() could be a BO itself, so to resolve this we could end up locking a BO twice. Adding a might_lock() to the beginning of ttm_bo_vm_fault as you suggested doesn't work either, because at this point the mmap_sem is still locked. So lockdep would complain about the incorrect bo_reserve and mmap_sem order. >>> I think Thomas' point is the one below: >>> Christian. Am 13.10.2018 um 21:04 schrieb Thomas Hellstrom: > Hi, Christian, > > On 10/13/2018 07:36 PM, Christian König wrote: >> Hi Thomas, >> >>> bo_reserve() >>> copy_to_user() / copy_from_user() >>> bo_unreserve() >> That pattern is illegal for a number of reasons and the mmap_sem is >> only one of it. >> >> So the locking order must always be mmap_sem->bo_reservation. See >> the >> userptr implementation in amdgpu as well. >> >> Christian. > I'm not arguing against that, and since vmwgfx doesn't use that > pattern, the locking order doesn't really matter to me since it's > even > possible to make the TTM fault() handler more well-behaved if we were > to fix the locking order to mmap_sem->bo_reserve.
Re: Planes enumeration with sun4i-drm driver
Hi, On Fri, Oct 12, 2018 at 06:47:13PM +0200, Paul Kocialkowski wrote: > I'm looking at the sun4i DRM driver these days (especially for > mainlining the VPU tiled format support through the frontend). > > The way things are done currently, all the possibly-supported plane > formats are listed in sun4i_backend_layer_formats and exposed as-is to > userspace for every plane. However, some of these formats cannot be > used on all planes at the same time and will be rejected when checking > the atomic commit. > > I find this a bit confusing from a userspace perspective. > > Not all constraints can be provided to userspace (e.g. the number of > planes we can effectively scale), but when it comes to formats, we have > that possibility. So perhaps it would make sense to only enumerate > formats as many times as we can support them. > > For instance, it could look like: > # plane 0: primary, RGB only > # plane 1: overlay, RGB + backend YUV formats > # plane 2: overlay, RGB + frontend YUV formats > # plane 3: overlay, RGB only > > That's not perfect either, because e.g. requesting a scaled RGB plane > will require the frontend and thus make it impossible to use the > frontend YUV formats that would still be described. But it feels like > it would be closer to representing hardware capabilities than our > current situation. That's really arguable. The hardware capabilities are that you can use any of those formats or features, on any plane, *but* on only one plane at the same time. What you describe isn't closer to it, it's just a way to workaround the issue we are facing (ie being able to show those constraints to userspace), and an imperfect workaround. > I am really not sure about the DRM subsystem policy about this, though. > Perhaps it was already decided that it's fine to deal with everything > at commit checking time and report more formats than the hardware can > really handle. It doesn't really matter what the DRM policy is here. Any change in the direction you suggest would be a regression from the userspace point of view and therefore would have to be reverted. IIRC Weston tries to discover those constraints by brute-forcing atomic_check and figuring out a combination that works, and we would surely benefit some kind of API to expose composition constraints. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
Hi! Interesting disscussion. Some comments below. On 10/15/2018 09:41 AM, Koenig, Christian wrote: Now amdgpu switched over Well exactly that's the point. amdgpu has *NOT* switched the order over. like i915 did yearsearlier I actually consider this a problem in i915 which should be fixed sooner or later. The locking in the fault handler as exposed by TTM is actually pointing out a deeper issue which nobody seems to have correctly understood so far. Let me explain a bit: 1. You need the a lock with the order lock->mmap_sem, cause otherwise you run into trouble when drm_vma_node_unmap() needs to be called. Why is this? unmap_mapping_range() never takes the mmap_sem. It takes a finer-level per-address-space lock. 2. But for CPU fault processing you also need a lock to determine the actual address where a BO is currently located Do you mean to temporarily pin the bo while page-table-entries are set up? i915 has solved this by separating the locks and using the reservation lock as fault processing lock. Problem is that this clashes with filling reservation objects on demand when another driver wants to use it. Could you elaborate a bit on this, what locking scenarios are conflicting etc. So what we should probably do is to fix i915 to not use the reservation object as fault processing lock any more, then separate the TTM handling into two locks as well. This sounds a bit strange to me. What we want to do in the fault handler is to temporarily pin down the object so that we can set up page-table entries, which is exactly what we accomplish with reservation objects. Otherwise whatever lock we introduce in the fault handler will also need to block the bo from being moved.. Thanks, Thomas. And last fix the the reservation object logic to allow pinning DMA-bufs on demand. Christian. Am 15.10.2018 um 08:55 schrieb Daniel Vetter: On Sun, Oct 14, 2018 at 8:20 PM Koenig, Christian wrote: Hi Thomas, that the access() handler took a shortcut when the new locking order was established There is no new locking order, the access handler is just for debugging and ignoring the correct locking order between mmap_sem and bo_reserve. That this is throwing a lockdep warning is perfectly possible. We should probably move that to a trylock instead. bo_reserve() copy_to_user() / copy_from_user() bo_unreserve() That one is illegal for a completely different reason. The address accessed by copy_to_user()/copy_from_user() could be a BO itself, so to resolve this we could end up locking a BO twice. Adding a might_lock() to the beginning of ttm_bo_vm_fault as you suggested doesn't work either, because at this point the mmap_sem is still locked. So lockdep would complain about the incorrect bo_reserve and mmap_sem order. I think Thomas' point is the one below: Christian. Am 13.10.2018 um 21:04 schrieb Thomas Hellstrom: Hi, Christian, On 10/13/2018 07:36 PM, Christian König wrote: Hi Thomas, bo_reserve() copy_to_user() / copy_from_user() bo_unreserve() That pattern is illegal for a number of reasons and the mmap_sem is only one of it. So the locking order must always be mmap_sem->bo_reservation. See the userptr implementation in amdgpu as well. Christian. I'm not arguing against that, and since vmwgfx doesn't use that pattern, the locking order doesn't really matter to me since it's even possible to make the TTM fault() handler more well-behaved if we were to fix the locking order to mmap_sem->bo_reserve. My concern is, since the _opposite_ locking order is (admittedly vaguely) documented in the fault handler, that the access() handler took a shortcut when the new locking order was established possibly without auditing of the other TTM drivers for locking inversion: For example it looks from a quick glance like nouveau_gem_pushbuf_reloc_apply() calls copy_from_user() with bo's reserved (which IIRC was the typical use-case at the time this was last lifted). And lockdep won't trip unless the access() callback is actually called. My point is if AMD wants to enforce this locking order, then IMHO the other drivers need to be audited and corrected if they are assuming the locking order documented in fault(). A good way to catch such drivers would be to add that might_lock(). ^^ This one here. There's a bunch of drivers which try-lock in the fault handler, so that the _can_ do the bo_reserve() copy*user() bo_unreserve() pattern. Yes the trylock will just loop forever if you copy*user() hits a bo itself that's already in the CS. Iirc I've complained about this years back. Now amdgpu switched over (like i915 did years earlier), because it's the only thing that reliably works even when facing evil userspace, but there's still radeon I think Thomas argues that we should fix those, and I agree. Once those are fixed I also think that a might_lock in the fault handler should not blow up anymore. If it does, you have an inversion still somewhere. Aside: I think it'd be good to
Re: [PATCH] drm/sti: clean up after drm_atomic_helper_shutdown rework
On Fri, Oct 12, 2018 at 11:46:39AM +0200, Benjamin Gaignard wrote:
> Since drm_atomic_helper_shutdown() rework it is possible to do additional
> clean up in sti driver: custom plane destroy functions become useless and
> clean up encoder is no more needed.
>
> Signed-off-by: Benjamin Gaignard
> ---
> drivers/gpu/drm/sti/sti_cursor.c | 9 +
> drivers/gpu/drm/sti/sti_gdp.c| 9 +
> drivers/gpu/drm/sti/sti_hqvdp.c | 9 +
> drivers/gpu/drm/sti/sti_tvout.c | 24
> 4 files changed, 3 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_cursor.c
> b/drivers/gpu/drm/sti/sti_cursor.c
> index bc908453ffb3..e1ba253055c7 100644
> --- a/drivers/gpu/drm/sti/sti_cursor.c
> +++ b/drivers/gpu/drm/sti/sti_cursor.c
> @@ -328,13 +328,6 @@ static const struct drm_plane_helper_funcs
> sti_cursor_helpers_funcs = {
> .atomic_disable = sti_cursor_atomic_disable,
> };
>
> -static void sti_cursor_destroy(struct drm_plane *drm_plane)
> -{
> - DRM_DEBUG_DRIVER("\n");
> -
> - drm_plane_cleanup(drm_plane);
> -}
> -
> static int sti_cursor_late_register(struct drm_plane *drm_plane)
> {
> struct sti_plane *plane = to_sti_plane(drm_plane);
> @@ -346,7 +339,7 @@ static int sti_cursor_late_register(struct drm_plane
> *drm_plane)
> static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .destroy = sti_cursor_destroy,
> + .destroy = drm_plane_cleanup,
> .reset = sti_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> index 3c19614d3f75..87b50451afd7 100644
> --- a/drivers/gpu/drm/sti/sti_gdp.c
> +++ b/drivers/gpu/drm/sti/sti_gdp.c
> @@ -879,13 +879,6 @@ static const struct drm_plane_helper_funcs
> sti_gdp_helpers_funcs = {
> .atomic_disable = sti_gdp_atomic_disable,
> };
>
> -static void sti_gdp_destroy(struct drm_plane *drm_plane)
> -{
> - DRM_DEBUG_DRIVER("\n");
> -
> - drm_plane_cleanup(drm_plane);
> -}
> -
> static int sti_gdp_late_register(struct drm_plane *drm_plane)
> {
> struct sti_plane *plane = to_sti_plane(drm_plane);
> @@ -897,7 +890,7 @@ static int sti_gdp_late_register(struct drm_plane
> *drm_plane)
> static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .destroy = sti_gdp_destroy,
> + .destroy = drm_plane_cleanup,
> .reset = sti_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
> index 23565f52dd71..065a5b08a702 100644
> --- a/drivers/gpu/drm/sti/sti_hqvdp.c
> +++ b/drivers/gpu/drm/sti/sti_hqvdp.c
> @@ -1256,13 +1256,6 @@ static const struct drm_plane_helper_funcs
> sti_hqvdp_helpers_funcs = {
> .atomic_disable = sti_hqvdp_atomic_disable,
> };
>
> -static void sti_hqvdp_destroy(struct drm_plane *drm_plane)
> -{
> - DRM_DEBUG_DRIVER("\n");
> -
> - drm_plane_cleanup(drm_plane);
> -}
> -
> static int sti_hqvdp_late_register(struct drm_plane *drm_plane)
> {
> struct sti_plane *plane = to_sti_plane(drm_plane);
> @@ -1274,7 +1267,7 @@ static int sti_hqvdp_late_register(struct drm_plane
> *drm_plane)
> static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .destroy = sti_hqvdp_destroy,
> + .destroy = drm_plane_cleanup,
> .reset = sti_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c
> index ea4a3b87fa55..4dc3b2ec40eb 100644
> --- a/drivers/gpu/drm/sti/sti_tvout.c
> +++ b/drivers/gpu/drm/sti/sti_tvout.c
> @@ -788,21 +788,6 @@ static void sti_tvout_create_encoders(struct drm_device
> *dev,
> tvout->dvo = sti_tvout_create_dvo_encoder(dev, tvout);
> }
>
> -static void sti_tvout_destroy_encoders(struct sti_tvout *tvout)
> -{
> - if (tvout->hdmi)
> - drm_encoder_cleanup(tvout->hdmi);
> - tvout->hdmi = NULL;
> -
> - if (tvout->hda)
> - drm_encoder_cleanup(tvout->hda);
> - tvout->hda = NULL;
> -
> - if (tvout->dvo)
> - drm_encoder_cleanup(tvout->dvo);
> - tvout->dvo = NULL;
> -}
> -
> static int sti_tvout_bind(struct device *dev, struct device *master, void
> *data)
> {
> struct sti_tvout *tvout =
Re: [PATCH v14 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp
On Fri, 2018-10-12 at 11:42 -0700, Radhakrishna Sripada wrote:
> Use the newly added "max bpc" connector property to limit pipe bpp.
>
> V3: Use drm_connector_state to access the "max bpc" property
> V4: Initialize the drm property, add suuport to DP(Ville)
> V5: Use the property in the connector and fix CI failure(Ville)
> V6: Use the core function to attach max_bpc property, remove the
> redundant
> clamping of pipe bpp based on connector info
> V7: Fix Checkpatch warnings
> V9: Cleanup connected_sink_max_bpp and fix initial value in DP(Ville)
> V12: Fix debug message(Ville)
> V13: Remove the redundant check and simplify the check logic(Stan)
> V14: Fix the check in connected_sink_max_bpp(Stan)
>
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> Cc: Rodrigo Vivi
> Cc: Kishore Kadiyala
> Cc: Manasi Navare
> Cc: Stanislav Lisovskiy
> Signed-off-by: Radhakrishna Sripada
> ---
> drivers/gpu/drm/i915/intel_display.c | 49
>
> drivers/gpu/drm/i915/intel_dp.c | 4 +++
> drivers/gpu/drm/i915/intel_hdmi.c| 5
> 3 files changed, 37 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 980f4ea68e48..ddcb7da3a4e1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10791,30 +10791,37 @@ static void
> intel_modeset_update_connector_atomic_state(struct drm_device *dev)
> drm_connector_list_iter_end(_iter);
> }
>
> -static void
> -connected_sink_compute_bpp(struct intel_connector *connector,
> -struct intel_crtc_state *pipe_config)
> +static int
> +connected_sink_max_bpp(const struct drm_connector_state *conn_state,
> +struct intel_crtc_state *pipe_config)
> {
> - const struct drm_display_info *info =
> >base.display_info;
> - int bpp = pipe_config->pipe_bpp;
> + int bpp;
> + struct drm_display_info *info = _state->connector-
> >display_info;
>
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp
> constrains\n",
> - connector->base.base.id,
> - connector->base.name);
> -
> - /* Don't use an invalid EDID bpc value */
> - if (info->bpc != 0 && info->bpc * 3 < bpp) {
> - DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID
> reported max of %d\n",
> - bpp, info->bpc * 3);
> - pipe_config->pipe_bpp = info->bpc * 3;
> + switch (conn_state->max_bpc) {
> + case 6 ... 7:
> + bpp = 6 * 3;
> + case 8 ... 9:
> + bpp = 8 * 3;
> + break;
> + case 10 ... 11:
> + bpp = 10 * 3;
> + break;
> + case 12:
> + bpp = 12 * 3;
> + break;
> + default:
> + return -EINVAL;
> }
Sorry, but one more comment(just wanted to give "reviewed-by" and
noticed this): there is missing break here, after case 6 .. 7.
>
> - /* Clamp bpp to 8 on screens without EDID 1.4 */
> - if (info->bpc == 0 && bpp > 24) {
> - DRM_DEBUG_KMS("clamping display bpp (was %d) to
> default limit of 24\n",
> - bpp);
> - pipe_config->pipe_bpp = 24;
> + if (bpp < pipe_config->pipe_bpp) {
> + DRM_DEBUG_KMS("Limiting display bpp to %d instead of
> Edid bpp "
> + "%d, requested bpp %d, max platform
> bpp %d\n", bpp,
> + 3 * info->bpc, 3 * conn_state-
> >max_requested_bpc,
> + pipe_config->pipe_bpp);
> + pipe_config->pipe_bpp = bpp;
> }
> + return 0;
> }
>
> static int
> @@ -10845,8 +10852,8 @@ compute_baseline_pipe_bpp(struct intel_crtc
> *crtc,
> if (connector_state->crtc != >base)
> continue;
>
> - connected_sink_compute_bpp(to_intel_connector(connec
> tor),
> -pipe_config);
> + if (connected_sink_max_bpp(connector_state,
> pipe_config) < 0)
> + return -EINVAL;
> }
>
> return bpp;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 13ff89be6ad6..5f594430b2a4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5735,6 +5735,10 @@ intel_dp_add_properties(struct intel_dp
> *intel_dp, struct drm_connector *connect
> intel_attach_force_audio_property(connector);
>
> intel_attach_broadcast_rgb_property(connector);
> + if (HAS_GMCH_DISPLAY(dev_priv))
> + drm_connector_attach_max_bpc_property(connector, 6,
> 10);
> + else if (INTEL_GEN(dev_priv) >= 5)
> + drm_connector_attach_max_bpc_property(connector, 6,
> 12);
>
> if (intel_dp_is_edp(intel_dp)) {
> u32 allowed_scalers;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>
Re: [PATCH] drm/exynos: checking for NULL instead of IS_ERR()
On 13.10.2018 12:21, Dan Carpenter wrote:
> The of_drm_find_panel() function returns error pointers and never NULL
> but we the driver assumes that ->panel is NULL when it's not present.
>
> Fixes: 6afb7721e2a0 ("drm/exynos: move connector creation to attach callback")
> Signed-off-by: Dan Carpenter
Thanks for spotting it (and sorry for previous reply mistake).
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 07af7758066d..32f256749789 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1527,7 +1527,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host
> *host,
> }
>
> dsi->panel = of_drm_find_panel(device->dev.of_node);
> - if (dsi->panel) {
> + if (IS_ERR(dsi->panel)) {
> + dsi->panel = NULL;
> + } else {
> drm_panel_attach(dsi->panel, >connector);
> dsi->connector.status = connector_status_connected;
> }
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/8] cpufreq: tegra186: don't pass GFP_DMA32 to dma_alloc_coherent
On Sat, Oct 13, 2018 at 5:17 PM Christoph Hellwig wrote: > > The DMA API does its own zone decisions based on the coherent_dma_mask. > > Signed-off-by: Christoph Hellwig Acked-by: Rafael J. Wysocki > --- > drivers/cpufreq/tegra186-cpufreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/tegra186-cpufreq.c > b/drivers/cpufreq/tegra186-cpufreq.c > index 1f59966573aa..f1e09022b819 100644 > --- a/drivers/cpufreq/tegra186-cpufreq.c > +++ b/drivers/cpufreq/tegra186-cpufreq.c > @@ -121,7 +121,7 @@ static struct cpufreq_frequency_table *init_vhint_table( > void *virt; > > virt = dma_alloc_coherent(bpmp->dev, sizeof(*data), , > - GFP_KERNEL | GFP_DMA32); > + GFP_KERNEL); > if (!virt) > return ERR_PTR(-ENOMEM); > > -- > 2.19.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Possible lock inversion in ttm_bo_vm_access
> Now amdgpu switched over Well exactly that's the point. amdgpu has *NOT* switched the order over. > like i915 did yearsearlier I actually consider this a problem in i915 which should be fixed sooner or later. The locking in the fault handler as exposed by TTM is actually pointing out a deeper issue which nobody seems to have correctly understood so far. Let me explain a bit: 1. You need the a lock with the order lock->mmap_sem, cause otherwise you run into trouble when drm_vma_node_unmap() needs to be called. 2. But for CPU fault processing you also need a lock to determine the actual address where a BO is currently located. i915 has solved this by separating the locks and using the reservation lock as fault processing lock. Problem is that this clashes with filling reservation objects on demand when another driver wants to use it. So what we should probably do is to fix i915 to not use the reservation object as fault processing lock any more, then separate the TTM handling into two locks as well. And last fix the the reservation object logic to allow pinning DMA-bufs on demand. Christian. Am 15.10.2018 um 08:55 schrieb Daniel Vetter: > On Sun, Oct 14, 2018 at 8:20 PM Koenig, Christian > wrote: >> Hi Thomas, >> >>> that the access() handler took a shortcut when the new locking order >>> was established >> There is no new locking order, the access handler is just for debugging >> and ignoring the correct locking order between mmap_sem and bo_reserve. >> >> That this is throwing a lockdep warning is perfectly possible. We should >> probably move that to a trylock instead. >> >>> bo_reserve() >>> copy_to_user() / copy_from_user() >>> bo_unreserve() >> That one is illegal for a completely different reason. >> >> The address accessed by copy_to_user()/copy_from_user() could be a BO >> itself, so to resolve this we could end up locking a BO twice. >> >> Adding a might_lock() to the beginning of ttm_bo_vm_fault as you >> suggested doesn't work either, because at this point the mmap_sem is >> still locked. >> >> So lockdep would complain about the incorrect bo_reserve and mmap_sem order. > I think Thomas' point is the one below: > >> Christian. >> >> Am 13.10.2018 um 21:04 schrieb Thomas Hellstrom: >>> Hi, Christian, >>> >>> On 10/13/2018 07:36 PM, Christian König wrote: Hi Thomas, > bo_reserve() > copy_to_user() / copy_from_user() > bo_unreserve() That pattern is illegal for a number of reasons and the mmap_sem is only one of it. So the locking order must always be mmap_sem->bo_reservation. See the userptr implementation in amdgpu as well. Christian. >>> I'm not arguing against that, and since vmwgfx doesn't use that >>> pattern, the locking order doesn't really matter to me since it's even >>> possible to make the TTM fault() handler more well-behaved if we were >>> to fix the locking order to mmap_sem->bo_reserve. >>> >>> My concern is, since the _opposite_ locking order is (admittedly >>> vaguely) documented in the fault handler, that the access() handler >>> took a shortcut when the new locking order was established possibly >>> without auditing of the other TTM drivers for locking inversion: For >>> example it looks from a quick glance like >>> nouveau_gem_pushbuf_reloc_apply() calls copy_from_user() with bo's >>> reserved (which IIRC was the typical use-case at the time this was >>> last lifted). And lockdep won't trip unless the access() callback is >>> actually called. >>> >>> My point is if AMD wants to enforce this locking order, then IMHO the >>> other drivers need to be audited and corrected if they are assuming >>> the locking order documented in fault(). A good way to catch such >>> drivers would be to add that might_lock(). > ^^ This one here. There's a bunch of drivers which try-lock in the > fault handler, so that the _can_ do the > > bo_reserve() > copy*user() > bo_unreserve() > > pattern. Yes the trylock will just loop forever if you copy*user() > hits a bo itself that's already in the CS. Iirc I've complained about > this years back. Now amdgpu switched over (like i915 did years > earlier), because it's the only thing that reliably works even when > facing evil userspace, but there's still radeon I think Thomas > argues that we should fix those, and I agree. > > Once those are fixed I also think that a might_lock in the fault > handler should not blow up anymore. If it does, you have an inversion > still somewhere. > > Aside: I think it'd be good to document this as part of struct > reservation_object, preferrably with lockdep annotations, to make sure > no one gets this wrong. Since we need _every_ driver to obey this, or > you just need the right buffer sharing combination to deadlock. > > Cheers, Daniel > >>> Thanks, >>> Thomas >>> >>> Am 12.10.2018 um 16:52 schrieb Thomas Hellstrom: > Hi, Felix, > > It looks like there is a locking inversion in ttm_bo_vm_access() > where we take a sleeping
Re: [PATCH v3 06/24] powerpc/mm: properly set PAGE_KERNEL flags in ioremap()
Christophe Leroy writes: > Set PAGE_KERNEL directly in the caller and do not rely on a > hack adding PAGE_KERNEL flags when _PAGE_PRESENT is not set. > > As already done for PPC64, use pgprot_cache() helpers instead of > _PAGE_XXX flags in PPC32 ioremap() derived functions. > > Signed-off-by: Christophe Leroy Something in here is breaking my p5020ds (both 32-bit and 64-bit): # first bad commit: [cb9220cc18345a2a1ec287752e576a2c88fa4a2b] powerpc/mm: properly set PAGE_KERNEL flags in ioremap() 64-bit: io scheduler mq-deadline registered io scheduler kyber registered pcieport :00:00.0: AER enabled with IRQ 482 pcieport 2000:00:00.0: AER enabled with IRQ 480 Unable to handle kernel paging request for data at address 0x88008008 Faulting instruction address: 0xc00152e4 Oops: Kernel access of bad area, sig: 11 [#1] BE SMP NR_CPUS=32 CoreNet Generic Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc-7.3.1-00135-gcb9220cc1834 #16 NIP: c00152e4 LR: c05173b8 CTR: 0010 REGS: c000f30eb420 TRAP: 0300 Not tainted (4.19.0-rc3-gcc-7.3.1-00135-gcb9220cc1834) MSR: 80029000 CR: 24000224 XER: DEAR: 88008008 ESR: 0080 IRQMASK: 0 GPR00: c05173a0 c000f30eb6a0 c0f86e00 88008008 GPR04: 0040 0ffbff04 0008 GPR08: 0010 0080 GPR12: 84000422 c00037c0 c000263c GPR16: GPR20: GPR24: c0dbe0a0 c0d41bf8 88008008 c00089a8 GPR28: c000f3492400 c000f3492410 0040 c0ff0a18 NIP [c00152e4] ._memset_io+0x6c/0x9c LR [c05173b8] .fsl_qman_probe+0x188/0x918 Call Trace: [c000f30eb6a0] [c05173a0] .fsl_qman_probe+0x170/0x918 (unreliable) [c000f30eb740] [c05797dc] .platform_drv_probe+0x58/0xac [c000f30eb7c0] [c05772b0] .really_probe+0x2a8/0x380 [c000f30eb860] [c05776f0] .__driver_attach+0x138/0x13c [c000f30eb8f0] [c0574550] .bus_for_each_dev+0x9c/0x110 [c000f30eb9a0] [c0576a18] .driver_attach+0x24/0x38 [c000f30eba10] [c05761ec] .bus_add_driver+0x16c/0x2ac [c000f30ebab0] [c0578194] .driver_register+0x88/0x178 [c000f30ebb30] [c0579770] .__platform_driver_register+0x48/0x5c [c000f30ebba0] [c0d85744] .fsl_qman_driver_init+0x1c/0x30 [c000f30ebc10] [c0002374] .do_one_initcall+0x70/0x258 [c000f30ebcf0] [c0d4a244] .kernel_init_freeable+0x340/0x43c [c000f30ebdb0] [c0002658] .kernel_init+0x1c/0x130 [c000f30ebe30] [c9e4] .ret_from_kernel_thread+0x58/0x74 Instruction dump: 4e800020 2ba50003 40dd003c 3925fffc 5488402e 7929f082 7d082378 39290001 550a801e 7d2903a6 7d4a4378 794a0020 <9143> 38630004 4200fff8 70a50003 ---[ end trace 372a57fd67efb6fe ]--- 32 bit: [1.076133] pcieport 2000:02:00.0: AER enabled with IRQ 480 [1.082106] Unable to handle kernel paging request for data at address 0xf110 [1.089488] Faulting instruction address: 0xc0011c80 [1.094437] Oops: Kernel access of bad area, sig: 11 [#1] [1.099816] BE SMP NR_CPUS=24 CoreNet Generic [1.104157] Modules linked in: [1.107197] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc7x-g8f0c636b0542 #1 [1.115181] NIP: c0011c80 LR: c058f970 CTR: 0010 [1.120216] REGS: e8107c80 TRAP: 0300 Not tainted (4.19.0-rc3-gcc7x-g8f0c636b0542) [1.128026] MSR: 00029002 CR: 24000284 XER: [1.134190] DEAR: f110 ESR: 0080 [1.134190] GPR00: c058f958 e8107d30 e8108000 f110 0040 e8107cd8 e8107d0c [1.134190] GPR08: 0010 ff00 24000282 c0003340 [1.134190] GPR16: c0d64410 c0edb34c c0ec6700 0007 [1.134190] GPR24: c0ef f110 efffcab8 c0ef e8231c00 e8231c10 0040003f c101d290 [1.171519] NIP [c0011c80] _memset_io+0x90/0xd0 [1.176030] LR [c058f970] fsl_qman_probe+0x190/0x8c0 [1.180975] Call Trace: [1.183410] [e8107d30] [c001f6c0] ioremap_prot+0x40/0x50 (unreliable) [1.189830] [e8107d40] [c058f958] fsl_qman_probe+0x178/0x8c0 [1.195475] [e8107d70] [c0600894] platform_drv_probe+0x54/0xb0 [1.201287] [e8107d90] [c05fe15c] really_probe+0x28c/0x350 [1.206756] [e8107dc0] [c05fe73c] __driver_attach+0x12c/0x130 [1.212485] [e8107de0] [c05fb5a8] bus_for_each_dev+0x98/0x110 [1.218213] [e8107e10] [c05fd048] bus_add_driver+0x158/0x2b0 [1.223855] [e8107e40] [c05ff1c4]
Re: [PATCH 2/2] vgaarb: Keep adding VGA device in queue
Hi David: Could you review and apply these 2 patches? Regards, Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v3,01/24] powerpc/32: Add ioremap_wt() and ioremap_coherent()
On Tue, 2018-10-09 at 13:51:33 UTC, Christophe Leroy wrote: > Other arches have ioremap_wt() to map IO areas write-through. > Implement it on PPC as well in order to avoid drivers using > __ioremap(_PAGE_WRITETHRU) > > Also implement ioremap_coherent() to avoid drivers using > __ioremap(_PAGE_COHERENT) > > Signed-off-by: Christophe Leroy Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/86c391bd5f47101acf1f3e0abd9fe0 cheers ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 06/24] powerpc/mm: properly set PAGE_KERNEL flags in ioremap()
Michael Ellerman a écrit : Christophe Leroy writes: Set PAGE_KERNEL directly in the caller and do not rely on a hack adding PAGE_KERNEL flags when _PAGE_PRESENT is not set. As already done for PPC64, use pgprot_cache() helpers instead of _PAGE_XXX flags in PPC32 ioremap() derived functions. Signed-off-by: Christophe Leroy Something in here is breaking my p5020ds (both 32-bit and 64-bit): # first bad commit: [cb9220cc18345a2a1ec287752e576a2c88fa4a2b] powerpc/mm: properly set PAGE_KERNEL flags in ioremap() Strange, it is calling ioremap_prot(), it should have been calling ioremap_cache() hence ioremap() since patch 4 of the serie. Did patch 4 apply correctly ? Christophe 64-bit: io scheduler mq-deadline registered io scheduler kyber registered pcieport :00:00.0: AER enabled with IRQ 482 pcieport 2000:00:00.0: AER enabled with IRQ 480 Unable to handle kernel paging request for data at address 0x88008008 Faulting instruction address: 0xc00152e4 Oops: Kernel access of bad area, sig: 11 [#1] BE SMP NR_CPUS=32 CoreNet Generic Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc-7.3.1-00135-gcb9220cc1834 #16 NIP: c00152e4 LR: c05173b8 CTR: 0010 REGS: c000f30eb420 TRAP: 0300 Not tainted (4.19.0-rc3-gcc-7.3.1-00135-gcb9220cc1834) MSR: 80029000 CR: 24000224 XER: DEAR: 88008008 ESR: 0080 IRQMASK: 0 GPR00: c05173a0 c000f30eb6a0 c0f86e00 88008008 GPR04: 0040 0ffbff04 0008 GPR08: 0010 0080 GPR12: 84000422 c00037c0 c000263c GPR16: GPR20: GPR24: c0dbe0a0 c0d41bf8 88008008 c00089a8 GPR28: c000f3492400 c000f3492410 0040 c0ff0a18 NIP [c00152e4] ._memset_io+0x6c/0x9c LR [c05173b8] .fsl_qman_probe+0x188/0x918 Call Trace: [c000f30eb6a0] [c05173a0] .fsl_qman_probe+0x170/0x918 (unreliable) [c000f30eb740] [c05797dc] .platform_drv_probe+0x58/0xac [c000f30eb7c0] [c05772b0] .really_probe+0x2a8/0x380 [c000f30eb860] [c05776f0] .__driver_attach+0x138/0x13c [c000f30eb8f0] [c0574550] .bus_for_each_dev+0x9c/0x110 [c000f30eb9a0] [c0576a18] .driver_attach+0x24/0x38 [c000f30eba10] [c05761ec] .bus_add_driver+0x16c/0x2ac [c000f30ebab0] [c0578194] .driver_register+0x88/0x178 [c000f30ebb30] [c0579770] .__platform_driver_register+0x48/0x5c [c000f30ebba0] [c0d85744] .fsl_qman_driver_init+0x1c/0x30 [c000f30ebc10] [c0002374] .do_one_initcall+0x70/0x258 [c000f30ebcf0] [c0d4a244] .kernel_init_freeable+0x340/0x43c [c000f30ebdb0] [c0002658] .kernel_init+0x1c/0x130 [c000f30ebe30] [c9e4] .ret_from_kernel_thread+0x58/0x74 Instruction dump: 4e800020 2ba50003 40dd003c 3925fffc 5488402e 7929f082 7d082378 39290001 550a801e 7d2903a6 7d4a4378 794a0020 <9143> 38630004 4200fff8 70a50003 ---[ end trace 372a57fd67efb6fe ]--- 32 bit: [1.076133] pcieport 2000:02:00.0: AER enabled with IRQ 480 [1.082106] Unable to handle kernel paging request for data at address 0xf110 [1.089488] Faulting instruction address: 0xc0011c80 [1.094437] Oops: Kernel access of bad area, sig: 11 [#1] [1.099816] BE SMP NR_CPUS=24 CoreNet Generic [1.104157] Modules linked in: [1.107197] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc7x-g8f0c636b0542 #1 [1.115181] NIP: c0011c80 LR: c058f970 CTR: 0010 [1.120216] REGS: e8107c80 TRAP: 0300 Not tainted (4.19.0-rc3-gcc7x-g8f0c636b0542) [1.128026] MSR: 00029002 CR: 24000284 XER: [1.134190] DEAR: f110 ESR: 0080 [1.134190] GPR00: c058f958 e8107d30 e8108000 f110 0040 e8107cd8 e8107d0c [1.134190] GPR08: 0010 ff00 24000282 c0003340 [1.134190] GPR16: c0d64410 c0edb34c c0ec6700 0007 [1.134190] GPR24: c0ef f110 efffcab8 c0ef e8231c00 e8231c10 0040003f c101d290 [1.171519] NIP [c0011c80] _memset_io+0x90/0xd0 [1.176030] LR [c058f970] fsl_qman_probe+0x190/0x8c0 [1.180975] Call Trace: [1.183410] [e8107d30] [c001f6c0] ioremap_prot+0x40/0x50 (unreliable) [1.189830] [e8107d40] [c058f958] fsl_qman_probe+0x178/0x8c0 [1.195475] [e8107d70] [c0600894] platform_drv_probe+0x54/0xb0 [1.201287] [e8107d90] [c05fe15c] really_probe+0x28c/0x350 [1.206756] [e8107dc0] [c05fe73c]
