[PATCH 2/3] drm/i915/display/debug: Expose crtc current bpc via debugfs
This new debugfs will expose the currently using bpc by crtc.
It is very useful for verifying whether we enter the correct
output color depth from IGT.
This patch will also add the connector's max supported bpc to
"i915_display_info" debugfs.
Example:
cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
Current: 8
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: Uma Shankar
Signed-off-by: Bhanuprakash Modem
---
.../drm/i915/display/intel_display_debugfs.c | 28 +++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 452d773fd4e3..6c3954479047 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -590,6 +590,8 @@ static void intel_connector_info(struct seq_file *m,
seq_puts(m, "\tHDCP version: ");
intel_hdcp_info(m, intel_connector);
+ seq_printf(m, "\tmax bpc: %u\n", connector->display_info.bpc);
+
intel_panel_info(m, intel_connector);
seq_printf(m, "\tmodes:\n");
@@ -2202,6 +2204,29 @@ static const struct file_operations i915_dsc_bpp_fops = {
.write = i915_dsc_bpp_write
};
+/*
+ * Returns the Current CRTC's bpc.
+ * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
+ */
+static int i915_current_bpc_show(struct seq_file *m, void *data)
+{
+ struct intel_crtc *crtc = to_intel_crtc(m->private);
+ struct intel_crtc_state *crtc_state;
+ int ret;
+
+ ret = drm_modeset_lock_single_interruptible(&crtc->base.mutex);
+ if (ret)
+ return ret;
+
+ crtc_state = to_intel_crtc_state(crtc->base.state);
+ seq_printf(m, "Current: %u\n", crtc_state->pipe_bpp / 3);
+
+ drm_modeset_unlock(&crtc->base.mutex);
+
+ return ret;
+}
+DEFINE_SHOW_ATTRIBUTE(i915_current_bpc);
+
/**
* intel_connector_debugfs_add - add i915 specific connector debugfs files
* @connector: pointer to a registered drm_connector
@@ -2272,4 +2297,7 @@ void intel_crtc_debugfs_add(struct drm_crtc *crtc)
crtc_updates_add(crtc);
intel_fbc_crtc_debugfs_add(to_intel_crtc(crtc));
+
+ debugfs_create_file("i915_current_bpc", 0444, crtc->debugfs_entry, crtc,
+ &i915_current_bpc_fops);
}
--
2.35.1
[PATCH 0/3] Expose max and current bpc via debugfs
This series will expose the Connector's max supported bpc via connector debugfs and Crtc's current bpc via crtc debugfs. Also move the existing vendor specific "output_bpc" logic to drm. Test-with: 20220408065143.1485069-2-bhanuprakash.mo...@intel.com Bhanuprakash Modem (3): drm/debug: Expose connector's max supported bpc via debugfs drm/i915/display/debug: Expose crtc current bpc via debugfs drm/amd/display: Move connector debugfs to drm .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 -- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 38 +++ .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.h | 2 - drivers/gpu/drm/drm_debugfs.c | 21 ++ .../drm/i915/display/intel_display_debugfs.c | 28 ++ 5 files changed, 62 insertions(+), 31 deletions(-) -- 2.35.1
[PATCH 3/3] drm/amd/display: Move connector debugfs to drm
As drm_connector already have the display_info, instead of creating
"output_bpc" debugfs in vendor specific driver, move the logic to
the drm layer.
This patch will also move "Current" bpc to the crtc debugfs from
connector debugfs, since we are getting this info from crtc_state.
Cc: Harry Wentland
Cc: Rodrigo Siqueira
Signed-off-by: Bhanuprakash Modem
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 --
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 38 +++
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.h | 2 -
3 files changed, 13 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 73423b805b54..f89651c71ec7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6615,14 +6615,12 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
return &state->base;
}
-#ifdef CONFIG_DRM_AMD_SECURE_DISPLAY
static int amdgpu_dm_crtc_late_register(struct drm_crtc *crtc)
{
crtc_debugfs_init(crtc);
return 0;
}
-#endif
static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable)
{
@@ -6720,9 +6718,7 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs =
{
.enable_vblank = dm_enable_vblank,
.disable_vblank = dm_disable_vblank,
.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
-#if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
.late_register = amdgpu_dm_crtc_late_register,
-#endif
};
static enum drm_connector_status
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index da17ece1a2c5..3ee26083920b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -873,28 +873,18 @@ static int psr_capability_show(struct seq_file *m, void
*data)
}
/*
- * Returns the current and maximum output bpc for the connector.
- * Example usage: cat /sys/kernel/debug/dri/0/DP-1/output_bpc
+ * Returns the current bpc for the crtc.
+ * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_bpc
*/
-static int output_bpc_show(struct seq_file *m, void *data)
+static int amdgpu_current_bpc_show(struct seq_file *m, void *data)
{
- struct drm_connector *connector = m->private;
- struct drm_device *dev = connector->dev;
- struct drm_crtc *crtc = NULL;
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
struct dm_crtc_state *dm_crtc_state = NULL;
int res = -ENODEV;
unsigned int bpc;
mutex_lock(&dev->mode_config.mutex);
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-
- if (connector->state == NULL)
- goto unlock;
-
- crtc = connector->state->crtc;
- if (crtc == NULL)
- goto unlock;
-
drm_modeset_lock(&crtc->mutex, NULL);
if (crtc->state == NULL)
goto unlock;
@@ -924,18 +914,15 @@ static int output_bpc_show(struct seq_file *m, void *data)
}
seq_printf(m, "Current: %u\n", bpc);
- seq_printf(m, "Maximum: %u\n", connector->display_info.bpc);
res = 0;
unlock:
- if (crtc)
- drm_modeset_unlock(&crtc->mutex);
-
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
+ drm_modeset_unlock(&crtc->mutex);
mutex_unlock(&dev->mode_config.mutex);
return res;
}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_bpc);
/*
* Example usage:
@@ -2541,7 +2528,6 @@ static int target_backlight_show(struct seq_file *m, void
*unused)
DEFINE_SHOW_ATTRIBUTE(dp_dsc_fec_support);
DEFINE_SHOW_ATTRIBUTE(dmub_fw_state);
DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer);
-DEFINE_SHOW_ATTRIBUTE(output_bpc);
DEFINE_SHOW_ATTRIBUTE(dp_lttpr_status);
#ifdef CONFIG_DRM_AMD_DC_HDCP
DEFINE_SHOW_ATTRIBUTE(hdcp_sink_capability);
@@ -2788,7 +2774,6 @@ static const struct {
const struct file_operations *fops;
} connector_debugfs_entries[] = {
{"force_yuv420_output", &force_yuv420_output_fops},
- {"output_bpc", &output_bpc_fops},
{"trigger_hotplug", &trigger_hotplug_debugfs_fops},
{"internal_display", &internal_display_fops}
};
@@ -3172,9 +3157,10 @@ static int crc_win_update_get(void *data, u64 *val)
DEFINE_DEBUGFS_ATTRIBUTE(crc_win_update_fops, crc_win_update_get,
crc_win_update_set, "%llu\n");
-
+#endif
void crtc_debugfs_init(struct drm_crtc *crtc)
{
+#ifdef CONFIG_DRM_AMD_SECURE_DISPLAY
struct dentry *dir = debugfs_lookup("crc", crtc->debugfs_entry);
if (!dir)
@@ -3190,9 +3176,11 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
&crc_win_y_end_fops);
debugfs_create_file_unsafe("crc_win_update", 0644, dir, crtc,
[PATCH 1/3] drm/debug: Expose connector's max supported bpc via debugfs
It's useful to know the connector's max supported bpc for IGT
testing. Expose it via a debugfs file on the connector "output_bpc".
Example: cat /sys/kernel/debug/dri/0/DP-1/output_bpc
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: Harry Wentland
Signed-off-by: Bhanuprakash Modem
---
drivers/gpu/drm/drm_debugfs.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 7f1b82dbaebb..33e5345c6f3e 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -395,6 +395,23 @@ static int vrr_range_show(struct seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(vrr_range);
+/*
+ * Returns Connector's max supported bpc through debugfs file.
+ * Example usage: cat /sys/kernel/debug/dri/0/DP-1/max_bpc
+ */
+static int output_bpc_show(struct seq_file *m, void *data)
+{
+ struct drm_connector *connector = m->private;
+
+ if (connector->status != connector_status_connected)
+ return -ENODEV;
+
+ seq_printf(m, "Maximum: %u\n", connector->display_info.bpc);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(output_bpc);
+
static const struct file_operations drm_edid_fops = {
.owner = THIS_MODULE,
.open = edid_open,
@@ -437,6 +454,10 @@ void drm_debugfs_connector_add(struct drm_connector
*connector)
debugfs_create_file("vrr_range", S_IRUGO, root, connector,
&vrr_range_fops);
+ /* max bpc */
+ debugfs_create_file("output_bpc", 0444, root, connector,
+ &output_bpc_fops);
+
if (connector->funcs->debugfs_init)
connector->funcs->debugfs_init(connector, root);
}
--
2.35.1
[PATCH] drm/dp: Don't rewrite link config when setting phy CTS test pattern with LTTPR
The sequence for Source DP PHY CTS automation[1]:
1- Emulate successful Link Training
2- Short HPD and Change link rates and number of lanes
3- Short HPD and Change PHY test pattern and swing/pre-emp levels
With DP PHY CTS setup as follow:
[DPTX + on board LTTPR]--Main Link--->[Scope]
^ |
| |
| |
--Aux Ch-->[Aux Emulator]
Writing to LINK_BW_SET on a port that has LTTPR is an indication of the
LT start for LTTPR [Check DP 2.0 E11 - Sec 3.6.8.2 & 3.6.8.6.3]. As
LTTPR snoops LINK_BW_SET/LANE_COUNT_SET, it will stop sending DP signals
to DP Scope causing the measurements to fail.
This can be tested with a monitor connected to LTTPR port by writing to
LINK_BW_SET as follow:
igt/tools/dpcd_reg write --offset=0x100 --value 0x14 --device=2
OR
printf '\x14' | sudo dd of=/dev/drm_dp_aux2 bs=1 count=1 conv=notrunc
seek=$((0x100))
This single aux write causes the screen to blank, sending short HPD to
DPTX, setting LINK_STATUS_UPDATE = 1 in DPCD 0x204, and triggering LT.
However, sending the same aux write on a port without LTTPR (direct port
to the monitor) has no effect.
In the case of DP PHY CTS setup described above, the AUX emulator executes
a script file of aux transactoins it sends to DPTX. For setting PHY pattern
the relevant segment of the script looks like the following:
# Set TEST_REQUEST (0x0218): PHY_TEST_PATTERN (0x0218.3) to 1
SetByte 0x218 0
SetBit 0x201 1
SetBit 0x218 3
# Set Test Pattern
SetPattern
# Trigger 1ms HPD Pulse
HPDPulse 0x06
# Wait For 2 seconds
Wait 200
MODULEEND
After the aux emulator finish executing the above segment, the scope waits
for the required pattern from DPTX to verify it is the right one and
perform the measurements. No more aux transactions the AUX emulator
will send. So, when DPTX update LINK_BW_SET/LANE_COUNT_SET, the LTTPR will
stop the signals on the main link to DPRX/Scope in order to adjust rate and
lane count it snooped and will wait for link training to start which will
never happen given the script file for aux transactions already finished.
The fix for this issue, is to not rewrite link config that is already done
in step 2 by modeset, and just change PHY test patterns and swing/pre-emph
levels.
[1]: LTTPR Re-timer PHY test procedure proposal
https://groups.vesa.org/wg/Link/document/16521
Cc: Imre Deak
Cc: Jani Nikula
Signed-off-by: Khaled Almahallawy
---
drivers/gpu/drm/dp/drm_dp.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/dp/drm_dp.c b/drivers/gpu/drm/dp/drm_dp.c
index 580016a1b9eb..f72d48e59b89 100644
--- a/drivers/gpu/drm/dp/drm_dp.c
+++ b/drivers/gpu/drm/dp/drm_dp.c
@@ -2613,17 +2613,8 @@ int drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
struct drm_dp_phy_test_params *data, u8 dp_rev)
{
int err, i;
- u8 link_config[2];
u8 test_pattern;
- link_config[0] = drm_dp_link_rate_to_bw_code(data->link_rate);
- link_config[1] = data->num_lanes;
- if (data->enhanced_frame_cap)
- link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
- err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, link_config, 2);
- if (err < 0)
- return err;
-
test_pattern = data->phy_pattern;
if (dp_rev < 0x12) {
test_pattern = (test_pattern << 2) &
--
2.25.1
Re: [PATCH] drm/rockchip: Refactor IOMMU initialisation
On Tue, Apr 05, 2022 at 03:32:50PM +0100, Robin Murphy wrote: > Defer the IOMMU domain setup until after successfully binding > components, so we can figure out IOMMU support directly from the VOP > devices themselves, rather than manually inferring it from the DT (which > also fails to account for whether the IOMMU driver is actually loaded). > Although this is somewhat of a logical cleanup, the main motivation is > to prepare for a change in the iommu_domain_alloc() interface. > > Signed-off-by: Robin Murphy > --- > > Lightly tested on RK3288. This does stand to collide with the in-flight > VOP2 driver a little, if only that that will want an equivalent > rockchip_drm_dma_init_device() call too be able to use its IOMMU. I'm > happy to help sort that out either way, it just depends on what we want > to merge first. I tested it with the VOP2 driver. It needed a little refactoring, I had to move the call to rockchip_drm_dma_attach_device() from vop2_bind() to vop2_enable(), but then it works as expected. Assuming that this patch goes through Heikos tree just like the VOP2 driver we can merge it first. I'll base the next VOP2 round on it. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [git pull] drm fixes for 5.18-rc2
On Thu, Apr 7, 2022 at 2:20 PM Dave Airlie wrote: > > I think this should fix the amdgpu splat you have been seeing since rc1. Not the machine I'm currently traveling with, but I'll double-check when I'm back home. Thanks, Linus
Re: [git pull] drm fixes for 5.18-rc2
The pull request you sent on Fri, 8 Apr 2022 10:19:47 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-04-08 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/1831fed559732b132aef0ea8261ac77e73f7eadf Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [Intel-gfx] [PATCH] drm/i915: Sunset igpu legacy mmap support based on GRAPHICS_VER_FULL
On Thu, Apr 07, 2022 at 09:18:39AM -0700, Matt Roper wrote:
The intent of the version check in the mmap ioctl was to maintain
support for existing platforms (i.e., ADL/RPL and earlier), but drop
support on all future igpu platforms. As we've seen on the dgpu side,
the hardware teams are using a more fine-grained numbering system for IP
version numbers these days, so it's possible the version number
associated with our next igpu could be some form of "12.xx" rather than
13 or higher. Comparing against the full ver.release number will ensure
the intent of the check is maintained no matter what numbering the
hardware teams settle on.
Fixes: d3f3baa3562a ("drm/i915: Reinstate the mmap ioctl for some platforms")
Cc: Thomas Hellström
Cc: Lucas De Marchi
Signed-off-by: Matt Roper
Reviewed-by: Lucas De Marchi
thanks
Lucas De Marchi
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c3ea243d414d..0c5c43852e24 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -70,7 +70,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
* mmap ioctl is disallowed for all discrete platforms,
* and for all platforms with GRAPHICS_VER > 12.
*/
- if (IS_DGFX(i915) || GRAPHICS_VER(i915) > 12)
+ if (IS_DGFX(i915) || GRAPHICS_VER_FULL(i915) > IP_VER(12, 0))
return -EOPNOTSUPP;
if (args->flags & ~(I915_MMAP_WC))
--
2.34.1
Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence
On Thu, Apr 07, 2022 at 05:45:32PM +0100, Matthew Auld wrote:
All of CI is just failing with the following, which prevents loading of
the module:
i915 :03:00.0: [drm] *ERROR* Scratch setup failed
Best guess is that this comes from the pin_map() for the scratch page,
which does an i915_gem_object_wait_moving_fence() somewhere. It looks
like this now calls into dma_resv_wait_timeout() which can return the
remaining timeout, leading to the caller thinking this is an error.
Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
Signed-off-by: Matthew Auld
Cc: Christian König
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 2998d895a6b3..1c88d4121658 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct
drm_i915_gem_object *obj,
int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
bool intr)
{
+ long ret;
+
assert_object_held(obj);
- return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
-intr, MAX_SCHEDULE_TIMEOUT);
+
+ ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
+ intr, MAX_SCHEDULE_TIMEOUT);
+
+ return ret < 0 ? ret : 0;
shouldn't == 0 also be an error since it would be a timeout?
Lucas De Marchi
Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller
On 2022/3/23 21:11, Rob Herring wrote:
On Wed, Mar 23, 2022 at 12:12:43PM +0800, Sui Jingfeng wrote:
On 2022/3/23 04:49, Rob Herring wrote:
+/*
+ * mainly for dc in ls7a1000 which have builtin gpio emulated i2c
+ *
+ * @index : output channel index, 0 for DVO0, 1 for DVO1
+ */
+struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned
int index)
+{
+ char compat[32] = {0};
+ unsigned int udelay = 5;
+ unsigned int timeout = 2200;
+ int nr = -1;
+ struct i2c_adapter *adapter;
+ struct lsdc_i2c *li2c;
+ struct device_node *i2c_np;
+ int ret;
+
+ li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL);
+ if (!li2c)
+ return ERR_PTR(-ENOMEM);
+
+ li2c->index = index;
+ li2c->dev = dev;
+
+ if (index == 0) {
+ li2c->sda = 0x01;
+ li2c->scl = 0x02;
+ } else if (index == 1) {
+ li2c->sda = 0x04;
+ li2c->scl = 0x08;
Just require this to be in DT rather than having some default.
By design, I am try very hard to let the code NOT fully DT dependent. DT is
nice , easy to learn and use.
But kernel side developer plan to follow UEFI + ACPI Specification on LS3A5000
+ LS7A1000 platform. See [1]
There will no DT support then, provide a convention support make the driver
more flexible. I want the
driver works with minimal requirement. The driver just works on simple boards
by put the following dc device
node in arch/mips/dts/loongson/loongson64g_4core_ls7a.dts,
Pick DT or ACPI for the platform, not both. We don't need to have both
in the kernel to support.
Rob
Hi, everybody
I have send new version of my patch, there may still have flaws though.
Would you like to help to review it again?
https://patchwork.freedesktop.org/series/102104/
@Rob @Maxime @Krzysztof
I have correct many issues as you guys mentioned before,
if something get ignored and I may miss the point, would like to
mention it again
on my new patches? because mails received previously got lost(flushed
by new mails).
I can only reply to new reviews.
Thanks for your time.
Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
On Fri, Apr 8, 2022 at 3:50 AM Helge Deller wrote: > > On 4/4/22 10:47, Zheyu Ma wrote: > > The userspace program could pass any values to the driver through > > ioctl() interface. If the driver doesn't check the value of 'pixclock', > > it may cause divide error. > > > > Fix this by checking whether 'pixclock' is zero in the function > > i740fb_check_var(). > > > > The following log reveals it: > > > > divide error: [#1] PREEMPT SMP KASAN PTI > > RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline] > > RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739 > > Call Trace: > > fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036 > > do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112 > > fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191 > > vfs_ioctl fs/ioctl.c:51 [inline] > > __do_sys_ioctl fs/ioctl.c:874 [inline] > > > > Signed-off-by: Zheyu Ma > > Hello Zheyu, > > I've applied the patches #2-#7 of this series, but left > out this specific patch (for now). > As discussed on the mailing list we can try to come up with a > better fix (to round up the pixclock when it's invalid). > If not, I will apply this one later. I'm also looking forward to a more appropriate patch for this driver! Thanks, Zheyu Ma
Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence
On Thu, Apr 07, 2022 at 05:45:32PM +0100, Matthew Auld wrote:
All of CI is just failing with the following, which prevents loading of
the module:
i915 :03:00.0: [drm] *ERROR* Scratch setup failed
Best guess is that this comes from the pin_map() for the scratch page,
which does an i915_gem_object_wait_moving_fence() somewhere. It looks
like this now calls into dma_resv_wait_timeout() which can return the
remaining timeout, leading to the caller thinking this is an error.
Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
Signed-off-by: Matthew Auld
Cc: Christian König
Cc: Daniel Vetter
This indeed brings CI back to life.
Acked-by: Lucas De Marchi
thanks
Lucas De Marchi
Re: [PATCH] drm/mediatek: dpi: Use mt8183 output formats for mt8192
On Fri, Apr 08, 2022 at 09:36:17AM +0800, CK Hu wrote:
> Hi, Jitao & Rex:
>
> Please help to comment on this patch.
Hi Chuang,
I already sent a v2 of this patch [1] because I forgot to add the Fixes tag.
Sorry for the noise.
Thanks,
Nícolas
[1] https://lore.kernel.org/all/20220408013950.674477-1-nfrapr...@collabora.com/
>
> On Thu, 2022-04-07 at 21:19 -0400, Nícolas F. R. A. Prado wrote:
> > The configuration for mt8192 was incorrectly using the output formats
> > from mt8173. Since the output formats for mt8192 are instead the same
> > ones as for mt8183, which require two bus samples per pixel, the
> > pixelclock and DDR edge setting were misconfigured. This made
> > external
> > displays unable to show the image.
> >
> > Fix the issue by correcting the output format for mt8192 to be the
> > same
> > as for mt8183, fixing the usage of external displays for mt8192.
> >
> > Signed-off-by: Nícolas F. R. A. Prado
> >
> > ---
> >
> > drivers/gpu/drm/mediatek/mtk_dpi.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > index 4554e2de1430..e61cd67b978f 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > @@ -819,8 +819,8 @@ static const struct mtk_dpi_conf mt8192_conf = {
> > .cal_factor = mt8183_calculate_factor,
> > .reg_h_fre_con = 0xe0,
> > .max_clock_khz = 15,
> > - .output_fmts = mt8173_output_fmts,
> > - .num_output_fmts = ARRAY_SIZE(mt8173_output_fmts),
> > + .output_fmts = mt8183_output_fmts,
> > + .num_output_fmts = ARRAY_SIZE(mt8183_output_fmts),
> > };
> >
> > static int mtk_dpi_probe(struct platform_device *pdev)
>
[PATCH v2] drm/mediatek: dpi: Use mt8183 output formats for mt8192
The configuration for mt8192 was incorrectly using the output formats
from mt8173. Since the output formats for mt8192 are instead the same
ones as for mt8183, which require two bus samples per pixel, the
pixelclock and DDR edge setting were misconfigured. This made external
displays unable to show the image.
Fix the issue by correcting the output format for mt8192 to be the same
as for mt8183, fixing the usage of external displays for mt8192.
Fixes: be63f6e8601f ("drm/mediatek: dpi: Add output bus formats to driver data")
Signed-off-by: Nícolas F. R. A. Prado
---
Changes in v2:
- Added Fixes tag
drivers/gpu/drm/mediatek/mtk_dpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 4554e2de1430..e61cd67b978f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -819,8 +819,8 @@ static const struct mtk_dpi_conf mt8192_conf = {
.cal_factor = mt8183_calculate_factor,
.reg_h_fre_con = 0xe0,
.max_clock_khz = 15,
- .output_fmts = mt8173_output_fmts,
- .num_output_fmts = ARRAY_SIZE(mt8173_output_fmts),
+ .output_fmts = mt8183_output_fmts,
+ .num_output_fmts = ARRAY_SIZE(mt8183_output_fmts),
};
static int mtk_dpi_probe(struct platform_device *pdev)
--
2.35.1
[PATCH] drm/bridge: anx7625: Use irq flags from devicetree
Read the irq flags, like which edge to trigger on, from the devicetree
and use those when registering the irq instead of hardcoding them.
In case none was specified, fallback to falling edge trigger.
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 6516f9570b86..97d954b8cc12 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2588,6 +2588,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
struct anx7625_platform_data *pdata;
int ret = 0;
struct device *dev = &client->dev;
+ unsigned long irqflags;
if (!i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -2639,10 +2640,13 @@ static int anx7625_i2c_probe(struct i2c_client *client,
goto free_hdcp_wq;
}
+ irqflags = irq_get_trigger_type(client->irq);
+ if (!irqflags)
+ irqflags = IRQF_TRIGGER_FALLING;
+
ret = devm_request_threaded_irq(dev, platform->pdata.intp_irq,
NULL, anx7625_intr_hpd_isr,
- IRQF_TRIGGER_FALLING |
- IRQF_ONESHOT,
+ irqflags | IRQF_ONESHOT,
"anx7625-intp", platform);
if (ret) {
DRM_DEV_ERROR(dev, "fail to request irq\n");
--
2.35.1
[PATCH] drm/bridge: anx7625: Use uint8 for lane-swing arrays
As defined in the anx7625 dt-binding, the analogix,lane0-swing and
analogix,lane1-swing properties are uint8 arrays. Yet, the driver was
reading the array as if it were of uint32 and masking to 8-bit before
writing to the registers. This means that a devicetree written in
accordance to the dt-binding would have its values incorrectly parsed.
Fix the issue by reading the array as uint8 and storing them as uint8
internally, so that we can also drop the masking when writing the
registers.
Fixes: fd0310b6fe7d ("drm/bridge: anx7625: add MIPI DPI input feature")
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 8
drivers/gpu/drm/bridge/analogix/anx7625.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 6516f9570b86..19a1a90ccff3 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1486,12 +1486,12 @@ static void anx7625_dp_adjust_swing(struct anx7625_data
*ctx)
for (i = 0; i < ctx->pdata.dp_lane0_swing_reg_cnt; i++)
anx7625_reg_write(ctx, ctx->i2c.tx_p1_client,
DP_TX_LANE0_SWING_REG0 + i,
- ctx->pdata.lane0_reg_data[i] & 0xFF);
+ ctx->pdata.lane0_reg_data[i]);
for (i = 0; i < ctx->pdata.dp_lane1_swing_reg_cnt; i++)
anx7625_reg_write(ctx, ctx->i2c.tx_p1_client,
DP_TX_LANE1_SWING_REG0 + i,
- ctx->pdata.lane1_reg_data[i] & 0xFF);
+ ctx->pdata.lane1_reg_data[i]);
}
static void dp_hpd_change_handler(struct anx7625_data *ctx, bool on)
@@ -1598,7 +1598,7 @@ static int anx7625_get_swing_setting(struct device *dev,
num_regs = DP_TX_SWING_REG_CNT;
pdata->dp_lane0_swing_reg_cnt = num_regs;
- of_property_read_u32_array(dev->of_node, "analogix,lane0-swing",
+ of_property_read_u8_array(dev->of_node, "analogix,lane0-swing",
pdata->lane0_reg_data, num_regs);
}
@@ -1608,7 +1608,7 @@ static int anx7625_get_swing_setting(struct device *dev,
num_regs = DP_TX_SWING_REG_CNT;
pdata->dp_lane1_swing_reg_cnt = num_regs;
- of_property_read_u32_array(dev->of_node, "analogix,lane1-swing",
+ of_property_read_u8_array(dev->of_node, "analogix,lane1-swing",
pdata->lane1_reg_data, num_regs);
}
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
b/drivers/gpu/drm/bridge/analogix/anx7625.h
index edbbfe410a56..e257a84db962 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -426,9 +426,9 @@ struct anx7625_platform_data {
int mipi_lanes;
int audio_en;
int dp_lane0_swing_reg_cnt;
- int lane0_reg_data[DP_TX_SWING_REG_CNT];
+ u8 lane0_reg_data[DP_TX_SWING_REG_CNT];
int dp_lane1_swing_reg_cnt;
- int lane1_reg_data[DP_TX_SWING_REG_CNT];
+ u8 lane1_reg_data[DP_TX_SWING_REG_CNT];
u32 low_power_mode;
struct device_node *mipi_host_node;
};
--
2.35.1
[PATCH] drm/mediatek: dpi: Use mt8183 output formats for mt8192
The configuration for mt8192 was incorrectly using the output formats
from mt8173. Since the output formats for mt8192 are instead the same
ones as for mt8183, which require two bus samples per pixel, the
pixelclock and DDR edge setting were misconfigured. This made external
displays unable to show the image.
Fix the issue by correcting the output format for mt8192 to be the same
as for mt8183, fixing the usage of external displays for mt8192.
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 4554e2de1430..e61cd67b978f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -819,8 +819,8 @@ static const struct mtk_dpi_conf mt8192_conf = {
.cal_factor = mt8183_calculate_factor,
.reg_h_fre_con = 0xe0,
.max_clock_khz = 15,
- .output_fmts = mt8173_output_fmts,
- .num_output_fmts = ARRAY_SIZE(mt8173_output_fmts),
+ .output_fmts = mt8183_output_fmts,
+ .num_output_fmts = ARRAY_SIZE(mt8183_output_fmts),
};
static int mtk_dpi_probe(struct platform_device *pdev)
--
2.35.1
linux-next: build failure after merge of the drm-misc tree
Hi all,
After merging the drm-misc tree, today's linux-next build (x86_64
allmodconfig) failed like this:
In file included from include/drm/drm_gem.h:38,
from include/drm/ttm/ttm_bo_api.h:34,
from drivers/gpu/drm/i915/i915_deps.c:9:
drivers/gpu/drm/i915/i915_deps.c: In function 'i915_deps_add_resv':
drivers/gpu/drm/i915/i915_deps.c:229:46: error: implicit conversion from 'enum
' to 'enum dma_resv_usage' [-Werror=enum-conversion]
229 | dma_resv_for_each_fence(&iter, resv, true, fence) {
| ^~~~
include/linux/dma-resv.h:297:47: note: in definition of macro
'dma_resv_for_each_fence'
297 | for (dma_resv_iter_begin(cursor, obj, usage), \
| ^
cc1: all warnings being treated as errors
Caused by commit
7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
I have used the drm-misc tree from next-20220407 for today.
--
Cheers,
Stephen Rothwell
pgpGebA24DTlK.pgp
Description: OpenPGP digital signature
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi, On Thu, Apr 7, 2022 at 4:46 PM Dmitry Baryshkov wrote: > > > The way I'm arguing it should work is that: > > > > 1. A whole bunch of the DP init code should move to the DP driver's > > probe function. This includes parsing the DT, acquiring clocks, > > getting a handle to our PHY, and IO mapping registers. As far as I > > know, there's no reason to wait on all the components being probed in > > order to do this stuff. > > Yes. And that's one of the reasons I tried to stay away from the DP > driver. Each time I open the source code, my hands itch to start > refactoring the code. > > > > > 2. Once we have done the above things, it should be possible to do AUX > > transfers, correct? ...and then we can populate the AUX bus from the > > probe function too. > > No. In the DP case the AUX bus is inaccessible until the dongle is > plugged (see all the HPD handling, phy_init()/phy_power_on() is hidden > somewhere in that path) I guess my thought was that in DP you could still create the AUX bus at probe time. Then for DP you just return an instant "transfer failed" from the AUX bus if HPD isn't asserted. For eDP (as discussed elsewhere) when we try to do an AUX transfer then we delay until HPD is there. So we can still acquire resources (clocks, PHY, io maps, etc) at probe time for DP and create the AUX bus, right? It will just return "-ENODEV" if HPD isn't asserted and you're DP? -Doug
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi, On Thu, Apr 7, 2022 at 4:36 PM Dmitry Baryshkov wrote: > > The ps8640 driver looks 'working by coincidence'. It calls > dp_aux_populate, then immediately after the function returns it checks > for the panel. If panel-edp is built as a module, the probe might fail > easily. > The anx7625 driver has the same kind of issue. The DP AUX bus is > populated from the probe() and after some additional work the panel is > being checked. > This design is fragile and from my quick glance it can break (or be > broken) too easy. It reminds me of our drm msm 'probe' loops > preventing the device to boot completely if the dsi bridge/panel could > not be probed in time. I did spend some time thinking about this, at least for ps8640. I believe that as long as the panel's probe isn't asynchronous. Basically if the panel isn't ready then ps8640 should return and we'll retry later. I do remember the probe loops that we used to have with msm and I don't _think_ this would trigger it. That being said, if we need to separate out the AUX bus into a sub-device like we did in sn65dsi86 we certainly could. -Doug
[git pull] drm fixes for 5.18-rc2
Hi Linus, Main set of fixes for rc2, mostly amdgpu, but some dma-fence fixups as well, along with some other misc ones. I think this should fix the amdgpu splat you have been seeing since rc1. Regards, Dave. drm-fixes-2022-04-08: drm fixes for 5.18-rc2 dma-fence: - fix warning about fence containers - fix logic error in new fence merge code - handle empty dma_fence_arrays gracefully bridge: - Try all possible cases for bridge/panel detection. bindings: - Don't require input port for MIPI-DSI, and make width/height mandatory. fbdev: - Fix unregistering of framebuffers without device. nouveau: - Fix a crash when booting with nouveau on tegra. amdgpu: - GFX 10.3.7 fixes - noretry updates - VCN fixes - TMDS fix - zstate fix for freesync video - DCN 3.1.5 fix - Display stack size fix - Audio fix - DCN 3.1 pstate fix - TMZ VCN fix - APU passthrough fix - Misc other fixes - VCN 3.0 fixes - Misc display fixes - GC 10.3 golden register fix - Suspend fix - SMU 10 fix amdkfd: - Error handling fix - xgmi p2p fix - HWS VMIDs fix - Event fix panel: - ili9341: Fix optional regulator handling imx: - Catch an EDID allocation failure in imx-ldb - fix a leaked drm display mode on DT parsing error in parallel-display - properly remove the dw_hdmi bridge in case the component_add fails in dw_hdmi-imx - fix the IPU clock frequency debug printout in ipu-di. The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17: Linux 5.18-rc1 (2022-04-03 14:08:21 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-04-08 for you to fetch changes up to 88711fa9a14f6f473f4a7645155ca51386e36c21: Merge tag 'drm-misc-fixes-2022-04-07' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2022-04-08 09:22:16 +1000) drm fixes for 5.18-rc2 dma-fence: - fix warning about fence containers - fix logic error in new fence merge code - handle empty dma_fence_arrays gracefully bridge: - Try all possible cases for bridge/panel detection. bindings: - Don't require input port for MIPI-DSI, and make width/height mandatory. fbdev: - Fix unregistering of framebuffers without device. nouveau: - Fix a crash when booting with nouveau on tegra. amdgpu: - GFX 10.3.7 fixes - noretry updates - VCN fixes - TMDS fix - zstate fix for freesync video - DCN 3.1.5 fix - Display stack size fix - Audio fix - DCN 3.1 pstate fix - TMZ VCN fix - APU passthrough fix - Misc other fixes - VCN 3.0 fixes - Misc display fixes - GC 10.3 golden register fix - Suspend fix - SMU 10 fix amdkfd: - Error handling fix - xgmi p2p fix - HWS VMIDs fix - Event fix panel: - ili9341: Fix optional regulator handling imx: - Catch an EDID allocation failure in imx-ldb - fix a leaked drm display mode on DT parsing error in parallel-display - properly remove the dw_hdmi bridge in case the component_add fails in dw_hdmi-imx - fix the IPU clock frequency debug printout in ipu-di. Alex Deucher (4): drm/amdgpu/gmc: use PCI BARs for APUs in passthrough drm/amdgpu: add more cases to noretry=1 drm/amdgpu: don't use BACO for reset in S3 drm/amdgpu/smu10: fix SoC/fclk units in auto mode Aurabindo Pillai (1): drm/amd: Add USBC connector ID Benjamin Marty (1): drm/amdgpu/display: change pipe policy for DCN 2.1 Boyuan Zhang (1): drm/amdgpu/vcn3: send smu interface type CHANDAN VURDIGERE NATARAJ (1): drm/amd/display: Fix by adding FPU protection for dcn30_internal_validate_bw Charlene Liu (3): drm/amd/display: fix audio format not updated after edid updated drm/amd/display: remove destructive verify link for TMDS drm/amd/display: Clear optc false state when disable otg Chiawen Huang (1): drm/amd/display: FEC check in timing validation Chris Park (1): drm/amd/display: Correct Slice reset calculation Christian König (5): dma-buf: Add dma_fence_array_for_each (v2) dma-buf: add dma_fence_unwrap v2 dma-buf/sync-file: fix warning about fence containers dma-buf/sync-file: fix logic error in new fence merge code dma-buf: handle empty dma_fence_arrays gracefully Dan Carpenter (1): drm/amdgpu: fix off by one in amdgpu_gfx_kiq_acquire() Daniel Mack (1): drm/panel: ili9341: fix optional regulator handling Dave Airlie (6): Merge tag 'amd-drm-next-5.18-2022-03-25' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-misc-fixes-2022-03-24' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'imx-drm-fixes-2022-04-06' of git://git.pengutronix.de/pza/linux into drm-fixes Merge tag 'amd-drm-fixes-5.18-2022-04-06' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-misc-next-fixes-2022-04-07' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-mis
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
On Fri, 8 Apr 2022 at 02:35, Doug Anderson wrote:
>
> Hi,
>
> On Thu, Apr 7, 2022 at 3:03 PM Abhinav Kumar
> wrote:
> >
> > Hi Doug
> >
> > Thanks for the response, some comments below.
> >
> > Abhinav
> > On 4/7/2022 1:47 PM, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Thu, Apr 7, 2022 at 1:11 PM Abhinav Kumar
> > > wrote:
> > >>
> > >> Hi Doug and Dmitry
> > >>
> > >> Sorry, but I caught up on this email just now.
> > >>
> > >> Some comments below.
> > >>
> > >> Thanks
> > >>
> > >> Abhinav
> > >> On 4/7/2022 10:07 AM, Doug Anderson wrote:
> > >>> Hi,
> > >>>
> > >>> On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC)
> > >>> wrote:
> >
> > Hi Dmitry and Doug,
> >
> > > Hi,
> > >
> > > On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov
> > > wrote:
> > >>
> > >> On 05/04/2022 20:02, Doug Anderson wrote:
> > >>> Hi,
> > >>>
> > >>> On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
> > >>> wrote:
> > > 3. For DP and eDP HPD means something a little different.
> > > Essentially there are two concepts: a) is a display physically
> > > connected and b) is the display powered up and ready. For DP, the
> > > two are really tied together. From the kernel's point of view you
> > > never "power down" a DP display and you can't detect that it's
> > > physically connected until it's ready. Said another way, on you
> > > tie "is a display there" to the HPD line and the moment a display
> > > is there it's ready for you to do AUX transfers. For eDP, in the
> > > lowest power state of a display it _won't_ assert its "HPD"
> > > signal. However, it's still physically present. For eDP you simply
> > > have to _assume_ it's present without any actual proof since you
> > > can't get proof until you power it up. Thus for eDP, you report
> > > that the display is there as soon as we're asked. We can't _talk_
> > > to the display yet, though. So in get_modes() we need to be able
> > > to power the display on enough to talk over the AUX channel to it.
> > > As part of this, we wait for the signal named "HPD" which really
> > > means
> > > "panel finished powering on" in this context.
> > >
> > > NOTE: for aux transfer, we don't have the _display_ pipe and
> > > clocks running. We only have enough stuff running to do the AUX
> > > transfer.
> > > We're not clocking out pixels. We haven't fully powered on the
> > > display. The AUX transfer is designed to be something that can be
> > > done early _before_ you turn on the display.
> > >
> > >
> > > OK, so basically that was a longwinded way of saying: yes, we
> > > could avoid the AUX transfer in probe, but we can't wait all the
> > > way to enable. We have to be able to transfer in get_modes(). If
> > > you think that's helpful I think it'd be a pretty easy patch to
> > > write even if it would look a tad bit awkward IMO. Let me know if
> > > you want me to post it up.
> >
> > I think it would be a good idea. At least it will allow us to
> > judge, which is the more correct way.
> > >>>
> > >>> I'm still happy to prototype this, but the more I think about it the
> > >>> more it feels like a workaround for the Qualcomm driver. The eDP
> > >>> panel driver is actually given a pointer to the AUX bus at probe
> > >>> time. It's really weird to say that we can't do a transfer on it
> > >>> yet... As you said, this is a little sideband bus. It should be able
> > >>> to be used without all the full blown infra of the rest of the
> > >>> driver.
> > >>
> > >> Yes, I have that feeling too. However I also have a feeling that just
> > >> powering up the PHY before the bus probe is ... a hack. There are no
> > >> obvious stopgaps for the driver not to power it down later.
> > >
> > >>
> > >> Lets go back to why we need to power up the PHY before the bus probe.
> > >>
> > >> We need to power up PHY before bus probe because panel-eDP tries to read
> > >> the EDID in probe() for the panel_id. Not get_modes().
> > >>
> > >> So doug, I didnt follow your comment that panel-eDP only does EDID read
> > >> in get_modes()
> > >>
> > >> panel_id = drm_edid_get_panel_id(panel->ddc);
> > >> if (!panel_id) {
> > >> dev_err(dev, "Couldn't identify panel via EDID\n");
> > >> ret = -EIO;
> > >> goto exit;
> > >> }
> > >>
> > >> If we do not need this part, we really dont need to power up the PHY
> > >> before the probe(). The hack which dmitry was referring to.
> > >
> > > Right. ...so we _could_ remove the above from the panel-edp probe and
> > > defer it to get_modes() and it wouldn't be that hard. ...but:
> > >
> > > 1. It feels l
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
On Thu, 7 Apr 2022 at 23:11, Abhinav Kumar wrote:
>
> Hi Doug and Dmitry
>
> Sorry, but I caught up on this email just now.
>
> Some comments below.
>
> Thanks
>
> Abhinav
> On 4/7/2022 10:07 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC)
> > wrote:
> >>
> >> Hi Dmitry and Doug,
> >>
> >>> Hi,
> >>>
> >>> On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov
> >>> wrote:
>
> On 05/04/2022 20:02, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
> > wrote:
> >>> 3. For DP and eDP HPD means something a little different.
> >>> Essentially there are two concepts: a) is a display physically
> >>> connected and b) is the display powered up and ready. For DP, the
> >>> two are really tied together. From the kernel's point of view you
> >>> never "power down" a DP display and you can't detect that it's
> >>> physically connected until it's ready. Said another way, on you
> >>> tie "is a display there" to the HPD line and the moment a display
> >>> is there it's ready for you to do AUX transfers. For eDP, in the
> >>> lowest power state of a display it _won't_ assert its "HPD"
> >>> signal. However, it's still physically present. For eDP you simply
> >>> have to _assume_ it's present without any actual proof since you
> >>> can't get proof until you power it up. Thus for eDP, you report
> >>> that the display is there as soon as we're asked. We can't _talk_
> >>> to the display yet, though. So in get_modes() we need to be able
> >>> to power the display on enough to talk over the AUX channel to it.
> >>> As part of this, we wait for the signal named "HPD" which really means
> >>> "panel finished powering on" in this context.
> >>>
> >>> NOTE: for aux transfer, we don't have the _display_ pipe and
> >>> clocks running. We only have enough stuff running to do the AUX
> >>> transfer.
> >>> We're not clocking out pixels. We haven't fully powered on the
> >>> display. The AUX transfer is designed to be something that can be
> >>> done early _before_ you turn on the display.
> >>>
> >>>
> >>> OK, so basically that was a longwinded way of saying: yes, we
> >>> could avoid the AUX transfer in probe, but we can't wait all the
> >>> way to enable. We have to be able to transfer in get_modes(). If
> >>> you think that's helpful I think it'd be a pretty easy patch to
> >>> write even if it would look a tad bit awkward IMO. Let me know if
> >>> you want me to post it up.
> >>
> >> I think it would be a good idea. At least it will allow us to
> >> judge, which is the more correct way.
> >
> > I'm still happy to prototype this, but the more I think about it the
> > more it feels like a workaround for the Qualcomm driver. The eDP
> > panel driver is actually given a pointer to the AUX bus at probe
> > time. It's really weird to say that we can't do a transfer on it
> > yet... As you said, this is a little sideband bus. It should be able
> > to be used without all the full blown infra of the rest of the driver.
>
> Yes, I have that feeling too. However I also have a feeling that just
> powering up the PHY before the bus probe is ... a hack. There are no
> obvious stopgaps for the driver not to power it down later.
> >>>
>
> Lets go back to why we need to power up the PHY before the bus probe.
>
> We need to power up PHY before bus probe because panel-eDP tries to read
> the EDID in probe() for the panel_id. Not get_modes().
>
> So doug, I didnt follow your comment that panel-eDP only does EDID read
> in get_modes()
>
> panel_id = drm_edid_get_panel_id(panel->ddc);
> if (!panel_id) {
> dev_err(dev, "Couldn't identify panel via EDID\n");
> ret = -EIO;
> goto exit;
> }
>
> If we do not need this part, we really dont need to power up the PHY
> before the probe(). The hack which dmitry was referring to.
>
> So this is boiling down to why or how panel-eDP was originally designed.
Well, it's not just panel-edp. It boils down to the DP-AUX bus design
vs DRM design. However in Doug's defense I should admit that I can not
come up with a significantly cleaner solution.
Just to emphasise (or to repeat): for all other display
panels/bridges, we either do not use a sidechannel bus or the
sidechannel bus (i2c, spi, platform) is managed outside of the DRM
framework. Thus it's possible to create the source in the drm's driver
probe path and then in the component's bind() path check (and return
an error) if the sink device wasn't yet probed successfully. The
source can then either communicate with the sink using the sidechannel
bus provided elsewhere or (e.g. in case of purely DSI panel), defer
communication till the moment the display path is fully available
(after encoder enablement).
F
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi,
On Thu, Apr 7, 2022 at 3:03 PM Abhinav Kumar wrote:
>
> Hi Doug
>
> Thanks for the response, some comments below.
>
> Abhinav
> On 4/7/2022 1:47 PM, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Apr 7, 2022 at 1:11 PM Abhinav Kumar
> > wrote:
> >>
> >> Hi Doug and Dmitry
> >>
> >> Sorry, but I caught up on this email just now.
> >>
> >> Some comments below.
> >>
> >> Thanks
> >>
> >> Abhinav
> >> On 4/7/2022 10:07 AM, Doug Anderson wrote:
> >>> Hi,
> >>>
> >>> On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC)
> >>> wrote:
>
> Hi Dmitry and Doug,
>
> > Hi,
> >
> > On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov
> > wrote:
> >>
> >> On 05/04/2022 20:02, Doug Anderson wrote:
> >>> Hi,
> >>>
> >>> On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
> >>> wrote:
> > 3. For DP and eDP HPD means something a little different.
> > Essentially there are two concepts: a) is a display physically
> > connected and b) is the display powered up and ready. For DP, the
> > two are really tied together. From the kernel's point of view you
> > never "power down" a DP display and you can't detect that it's
> > physically connected until it's ready. Said another way, on you
> > tie "is a display there" to the HPD line and the moment a display
> > is there it's ready for you to do AUX transfers. For eDP, in the
> > lowest power state of a display it _won't_ assert its "HPD"
> > signal. However, it's still physically present. For eDP you simply
> > have to _assume_ it's present without any actual proof since you
> > can't get proof until you power it up. Thus for eDP, you report
> > that the display is there as soon as we're asked. We can't _talk_
> > to the display yet, though. So in get_modes() we need to be able
> > to power the display on enough to talk over the AUX channel to it.
> > As part of this, we wait for the signal named "HPD" which really
> > means
> > "panel finished powering on" in this context.
> >
> > NOTE: for aux transfer, we don't have the _display_ pipe and
> > clocks running. We only have enough stuff running to do the AUX
> > transfer.
> > We're not clocking out pixels. We haven't fully powered on the
> > display. The AUX transfer is designed to be something that can be
> > done early _before_ you turn on the display.
> >
> >
> > OK, so basically that was a longwinded way of saying: yes, we
> > could avoid the AUX transfer in probe, but we can't wait all the
> > way to enable. We have to be able to transfer in get_modes(). If
> > you think that's helpful I think it'd be a pretty easy patch to
> > write even if it would look a tad bit awkward IMO. Let me know if
> > you want me to post it up.
>
> I think it would be a good idea. At least it will allow us to
> judge, which is the more correct way.
> >>>
> >>> I'm still happy to prototype this, but the more I think about it the
> >>> more it feels like a workaround for the Qualcomm driver. The eDP
> >>> panel driver is actually given a pointer to the AUX bus at probe
> >>> time. It's really weird to say that we can't do a transfer on it
> >>> yet... As you said, this is a little sideband bus. It should be able
> >>> to be used without all the full blown infra of the rest of the driver.
> >>
> >> Yes, I have that feeling too. However I also have a feeling that just
> >> powering up the PHY before the bus probe is ... a hack. There are no
> >> obvious stopgaps for the driver not to power it down later.
> >
> >>
> >> Lets go back to why we need to power up the PHY before the bus probe.
> >>
> >> We need to power up PHY before bus probe because panel-eDP tries to read
> >> the EDID in probe() for the panel_id. Not get_modes().
> >>
> >> So doug, I didnt follow your comment that panel-eDP only does EDID read
> >> in get_modes()
> >>
> >> panel_id = drm_edid_get_panel_id(panel->ddc);
> >> if (!panel_id) {
> >> dev_err(dev, "Couldn't identify panel via EDID\n");
> >> ret = -EIO;
> >> goto exit;
> >> }
> >>
> >> If we do not need this part, we really dont need to power up the PHY
> >> before the probe(). The hack which dmitry was referring to.
> >
> > Right. ...so we _could_ remove the above from the panel-edp probe and
> > defer it to get_modes() and it wouldn't be that hard. ...but:
> >
> > 1. It feels like a hack to work around the Qualcomm driver. The way
> > the AUX bus is designed is that a pointer to the AUX bus is passed to
> > the panel-edp probe. It seems kinda strange to say that the panel
> > isn't allowed to do transfers with the pointer that's passed in.
> >
>
> And thats why to s
[PATCH -next] drm/amd/display: remove unneeded semicolon
Eliminate the following coccicheck warning:
./drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c:477:2-3:
Unneeded semicolon
Reported-by: Abaci Robot
Signed-off-by: Yang Li
---
drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
index 8be4c1970628..707dce6486b9 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
@@ -474,7 +474,7 @@ static void dcn315_clk_mgr_helper_populate_bw_params(
bw_params->clk_table.entries[i].dispclk_mhz = max_dispclk;
bw_params->clk_table.entries[i].dppclk_mhz = max_dppclk;
bw_params->clk_table.entries[i].wck_ratio = 1;
- };
+ }
/* Make sure to include at least one entry and highest pstate */
if (max_pstate != min_pstate) {
--
2.20.1.7.g153144c
Re: [PATCH] drm/atomic-helpers: remove legacy_cursor_update hacks
On Thu, Apr 7, 2022 at 3:59 PM Abhinav Kumar wrote:
>
> Hi Rob and Daniel
>
> On 4/7/2022 3:51 PM, Rob Clark wrote:
> > On Wed, Apr 6, 2022 at 6:27 PM Jessica Zhang
> > wrote:
> >>
> >>
> >>
> >> On 3/31/2022 8:20 AM, Daniel Vetter wrote:
> >>> The stuff never really worked, and leads to lots of fun because it
> >>> out-of-order frees atomic states. Which upsets KASAN, among other
> >>> things.
> >>>
> >>> For async updates we now have a more solid solution with the
> >>> ->atomic_async_check and ->atomic_async_commit hooks. Support for that
> >>> for msm and vc4 landed. nouveau and i915 have their own commit
> >>> routines, doing something similar.
> >>>
> >>> For everyone else it's probably better to remove the use-after-free
> >>> bug, and encourage folks to use the async support instead. The
> >>> affected drivers which register a legacy cursor plane and don't either
> >>> use the new async stuff or their own commit routine are: amdgpu,
> >>> atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
> >>>
> >>> Inspired by an amdgpu bug report.
> >>>
> >>> v2: Drop RFC, I think with amdgpu converted over to use
> >>> atomic_async_check/commit done in
> >>>
> >>> commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
> >>> Author: Nicholas Kazlauskas
> >>> Date: Wed Dec 5 14:59:07 2018 -0500
> >>>
> >>> drm/amd/display: Add fast path for cursor plane updates
> >>>
> >>> we don't have any driver anymore where we have userspace expecting
> >>> solid legacy cursor support _and_ they are using the atomic helpers in
> >>> their fully glory. So we can retire this.
> >>>
> >>> v3: Paper over msm and i915 regression. The complete_all is the only
> >>> thing missing afaict.
> >>>
> >>> v4: Fixup i915 fixup ...
> >>>
> >>> References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> >>> References:
> >>> https://lore.kernel.org/all/20220221134155.125447-9-max...@cerno.tech/
> >>> References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> >>> Cc: Maxime Ripard
> >>> Tested-by: Maxime Ripard
> >>> Cc: mikita.lip...@amd.com
> >>> Cc: Michel Dänzer
> >>> Cc: harry.wentl...@amd.com
> >>> Cc: Rob Clark
> >>
> >> Hey Rob,
> >>
> >> I saw your tested-by and reviewed-by tags on Patchwork. Just curious,
> >> what device did you test on?
> >
> > I was testing on strongbad.. v5.18-rc1 + patches (notably, revert
> > 80253168dbfd ("drm: of: Lookup if child node has panel or bridge")
> >
> > I think the display setup shouldn't be significantly different than
> > limozeen (ie. it's an eDP panel). But I didn't do much start/stop
> > ui.. I was mostly looking to make sure cursor movements weren't
> > causing fps drops ;-)
> >
> > BR,
> > -R
>
> start ui/ stop ui is a basic operation for us to use IGT on msm-next.
> So we cannot let that break.
>
> I think we need to check whats causing this splat.
>
> Can we hold back this change till then?
Can you reproduce on v5.18-rc1 (plus 80253168dbfd)? I'm running a
loop of stop ui / start ui and hasn't triggered a splat yet.
Otherwise maybe you can addr2line to figure out where it crashed?
BR,
-R
> Thanks
>
> Abhinav
> >
> >> I'm hitting several instances of this error when doing a start/stop ui
> >> on Lazor Chromebook with this patch:
> >>
> >> [ 3092.608322] CPU: 2 PID: 18579 Comm: DrmThread Tainted: GW
> >>5.17.0-rc2-lockdep-00089-g7f17ab7bf567 #155
> >> e5912cd286513b064a82a38938b3fdef86b079aa
> >> [ 3092.622880] Hardware name: Google Lazor Limozeen without Touchscreen
> >> (rev4) (DT)
> >> [ 3092.630492] pstate: 8049 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS
> >> BTYPE=--)
> >> [ 3092.637664] pc : dpu_crtc_atomic_flush+0x9c/0x144
> >> [ 3092.642523] lr : dpu_crtc_atomic_flush+0x60/0x144
> >> [ 3092.647379] sp : ffc00c1e3760
> >> [ 3092.650805] x29: ffc00c1e3760 x28: ff80985dd800 x27:
> >> 0425
> >> [ 3092.658164] x26: ff80985dc500 x25: ff80985ddc00 x24:
> >> ffdf8ae3b6f0
> >> [ 3092.665522] x23: x22: x21:
> >> ff809b82da00
> >> [ 3092.672890] x20: ff80840e1000 x19: ff80840e2000 x18:
> >> 1000
> >> [ 3092.680255] x17: 0400 x16: 0100 x15:
> >> 003b
> >> [ 3092.687622] x14: x13: 0002 x12:
> >> 0003
> >> [ 3092.694979] x11: ff8084009000 x10: 0040 x9 :
> >> 0040
> >> [ 3092.702340] x8 : 0300 x7 : 000c x6 :
> >> 0004
> >> [ 3092.709698] x5 : 0320 x4 : 0018 x3 :
> >>
> >> [ 3092.717056] x2 : x1 : 7bfb38b2a3a89800 x0 :
> >> ff809a1eb300
> >> [ 3092.724424] Call trace:
> >> [ 3092.726958] dpu_crtc_atomic_flush+0x9c/0x144
> >> [ 3092.731463] drm_atomic_helper_commit_planes+0x1bc/0x1c4
> >> [ 3092.736944] msm_atomic_commit_tail+0x23c/0x3e0
> >> [ 3092.741627] commit_tail+0x7c/0xfc
> >> [ 3092.745145] drm_atomic_helper_commit+0x158/0x15c
> >> [ 3
Re: [PATCH] drm/atomic-helpers: remove legacy_cursor_update hacks
Hi Rob and Daniel
On 4/7/2022 3:51 PM, Rob Clark wrote:
On Wed, Apr 6, 2022 at 6:27 PM Jessica Zhang wrote:
On 3/31/2022 8:20 AM, Daniel Vetter wrote:
The stuff never really worked, and leads to lots of fun because it
out-of-order frees atomic states. Which upsets KASAN, among other
things.
For async updates we now have a more solid solution with the
->atomic_async_check and ->atomic_async_commit hooks. Support for that
for msm and vc4 landed. nouveau and i915 have their own commit
routines, doing something similar.
For everyone else it's probably better to remove the use-after-free
bug, and encourage folks to use the async support instead. The
affected drivers which register a legacy cursor plane and don't either
use the new async stuff or their own commit routine are: amdgpu,
atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
Inspired by an amdgpu bug report.
v2: Drop RFC, I think with amdgpu converted over to use
atomic_async_check/commit done in
commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
Author: Nicholas Kazlauskas
Date: Wed Dec 5 14:59:07 2018 -0500
drm/amd/display: Add fast path for cursor plane updates
we don't have any driver anymore where we have userspace expecting
solid legacy cursor support _and_ they are using the atomic helpers in
their fully glory. So we can retire this.
v3: Paper over msm and i915 regression. The complete_all is the only
thing missing afaict.
v4: Fixup i915 fixup ...
References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
References:
https://lore.kernel.org/all/20220221134155.125447-9-max...@cerno.tech/
References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
Cc: Maxime Ripard
Tested-by: Maxime Ripard
Cc: mikita.lip...@amd.com
Cc: Michel Dänzer
Cc: harry.wentl...@amd.com
Cc: Rob Clark
Hey Rob,
I saw your tested-by and reviewed-by tags on Patchwork. Just curious,
what device did you test on?
I was testing on strongbad.. v5.18-rc1 + patches (notably, revert
80253168dbfd ("drm: of: Lookup if child node has panel or bridge")
I think the display setup shouldn't be significantly different than
limozeen (ie. it's an eDP panel). But I didn't do much start/stop
ui.. I was mostly looking to make sure cursor movements weren't
causing fps drops ;-)
BR,
-R
start ui/ stop ui is a basic operation for us to use IGT on msm-next.
So we cannot let that break.
I think we need to check whats causing this splat.
Can we hold back this change till then?
Thanks
Abhinav
I'm hitting several instances of this error when doing a start/stop ui
on Lazor Chromebook with this patch:
[ 3092.608322] CPU: 2 PID: 18579 Comm: DrmThread Tainted: GW
5.17.0-rc2-lockdep-00089-g7f17ab7bf567 #155
e5912cd286513b064a82a38938b3fdef86b079aa
[ 3092.622880] Hardware name: Google Lazor Limozeen without Touchscreen
(rev4) (DT)
[ 3092.630492] pstate: 8049 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 3092.637664] pc : dpu_crtc_atomic_flush+0x9c/0x144
[ 3092.642523] lr : dpu_crtc_atomic_flush+0x60/0x144
[ 3092.647379] sp : ffc00c1e3760
[ 3092.650805] x29: ffc00c1e3760 x28: ff80985dd800 x27:
0425
[ 3092.658164] x26: ff80985dc500 x25: ff80985ddc00 x24:
ffdf8ae3b6f0
[ 3092.665522] x23: x22: x21:
ff809b82da00
[ 3092.672890] x20: ff80840e1000 x19: ff80840e2000 x18:
1000
[ 3092.680255] x17: 0400 x16: 0100 x15:
003b
[ 3092.687622] x14: x13: 0002 x12:
0003
[ 3092.694979] x11: ff8084009000 x10: 0040 x9 :
0040
[ 3092.702340] x8 : 0300 x7 : 000c x6 :
0004
[ 3092.709698] x5 : 0320 x4 : 0018 x3 :
[ 3092.717056] x2 : x1 : 7bfb38b2a3a89800 x0 :
ff809a1eb300
[ 3092.724424] Call trace:
[ 3092.726958] dpu_crtc_atomic_flush+0x9c/0x144
[ 3092.731463] drm_atomic_helper_commit_planes+0x1bc/0x1c4
[ 3092.736944] msm_atomic_commit_tail+0x23c/0x3e0
[ 3092.741627] commit_tail+0x7c/0xfc
[ 3092.745145] drm_atomic_helper_commit+0x158/0x15c
[ 3092.749998] drm_atomic_commit+0x60/0x74
[ 3092.754055] drm_atomic_helper_update_plane+0x100/0x110
[ 3092.759449] __setplane_atomic+0x11c/0x120
[ 3092.763685] drm_mode_cursor_universal+0x188/0x22c
[ 3092.768633] drm_mode_cursor_common+0x120/0x1f8
[ 3092.773310] drm_mode_cursor_ioctl+0x68/0x8c
[ 3092.21] drm_ioctl_kernel+0xe8/0x168
[ 3092.781770] drm_ioctl+0x320/0x370
[ 3092.785289] drm_compat_ioctl+0x40/0xdc
[ 3092.789257] __arm64_compat_sys_ioctl+0xe0/0x150
[ 3092.794030] invoke_syscall+0x80/0x114
[ 3092.797905] el0_svc_common.constprop.3+0xc4/0xf8
[ 3092.802765] do_el0_svc_compat+0x2c/0x54
[ 3092.806811] el0_svc_compat+0x4c/0xe4
[ 3092.810598] el0t_32_sync_handler+0xc4/0xf4
[ 3092.814914] el0t_32_sync+0x174/0x178
[ 3092.818701] irq event stamp: 55940
[ 3092.822217] hardirqs last enabled at (
Re: [PATCH] drm/atomic-helpers: remove legacy_cursor_update hacks
On Wed, Apr 6, 2022 at 6:27 PM Jessica Zhang wrote:
>
>
>
> On 3/31/2022 8:20 AM, Daniel Vetter wrote:
> > The stuff never really worked, and leads to lots of fun because it
> > out-of-order frees atomic states. Which upsets KASAN, among other
> > things.
> >
> > For async updates we now have a more solid solution with the
> > ->atomic_async_check and ->atomic_async_commit hooks. Support for that
> > for msm and vc4 landed. nouveau and i915 have their own commit
> > routines, doing something similar.
> >
> > For everyone else it's probably better to remove the use-after-free
> > bug, and encourage folks to use the async support instead. The
> > affected drivers which register a legacy cursor plane and don't either
> > use the new async stuff or their own commit routine are: amdgpu,
> > atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
> >
> > Inspired by an amdgpu bug report.
> >
> > v2: Drop RFC, I think with amdgpu converted over to use
> > atomic_async_check/commit done in
> >
> > commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
> > Author: Nicholas Kazlauskas
> > Date: Wed Dec 5 14:59:07 2018 -0500
> >
> > drm/amd/display: Add fast path for cursor plane updates
> >
> > we don't have any driver anymore where we have userspace expecting
> > solid legacy cursor support _and_ they are using the atomic helpers in
> > their fully glory. So we can retire this.
> >
> > v3: Paper over msm and i915 regression. The complete_all is the only
> > thing missing afaict.
> >
> > v4: Fixup i915 fixup ...
> >
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> > References:
> > https://lore.kernel.org/all/20220221134155.125447-9-max...@cerno.tech/
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> > Cc: Maxime Ripard
> > Tested-by: Maxime Ripard
> > Cc: mikita.lip...@amd.com
> > Cc: Michel Dänzer
> > Cc: harry.wentl...@amd.com
> > Cc: Rob Clark
>
> Hey Rob,
>
> I saw your tested-by and reviewed-by tags on Patchwork. Just curious,
> what device did you test on?
I was testing on strongbad.. v5.18-rc1 + patches (notably, revert
80253168dbfd ("drm: of: Lookup if child node has panel or bridge")
I think the display setup shouldn't be significantly different than
limozeen (ie. it's an eDP panel). But I didn't do much start/stop
ui.. I was mostly looking to make sure cursor movements weren't
causing fps drops ;-)
BR,
-R
> I'm hitting several instances of this error when doing a start/stop ui
> on Lazor Chromebook with this patch:
>
> [ 3092.608322] CPU: 2 PID: 18579 Comm: DrmThread Tainted: GW
> 5.17.0-rc2-lockdep-00089-g7f17ab7bf567 #155
> e5912cd286513b064a82a38938b3fdef86b079aa
> [ 3092.622880] Hardware name: Google Lazor Limozeen without Touchscreen
> (rev4) (DT)
> [ 3092.630492] pstate: 8049 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [ 3092.637664] pc : dpu_crtc_atomic_flush+0x9c/0x144
> [ 3092.642523] lr : dpu_crtc_atomic_flush+0x60/0x144
> [ 3092.647379] sp : ffc00c1e3760
> [ 3092.650805] x29: ffc00c1e3760 x28: ff80985dd800 x27:
> 0425
> [ 3092.658164] x26: ff80985dc500 x25: ff80985ddc00 x24:
> ffdf8ae3b6f0
> [ 3092.665522] x23: x22: x21:
> ff809b82da00
> [ 3092.672890] x20: ff80840e1000 x19: ff80840e2000 x18:
> 1000
> [ 3092.680255] x17: 0400 x16: 0100 x15:
> 003b
> [ 3092.687622] x14: x13: 0002 x12:
> 0003
> [ 3092.694979] x11: ff8084009000 x10: 0040 x9 :
> 0040
> [ 3092.702340] x8 : 0300 x7 : 000c x6 :
> 0004
> [ 3092.709698] x5 : 0320 x4 : 0018 x3 :
>
> [ 3092.717056] x2 : x1 : 7bfb38b2a3a89800 x0 :
> ff809a1eb300
> [ 3092.724424] Call trace:
> [ 3092.726958] dpu_crtc_atomic_flush+0x9c/0x144
> [ 3092.731463] drm_atomic_helper_commit_planes+0x1bc/0x1c4
> [ 3092.736944] msm_atomic_commit_tail+0x23c/0x3e0
> [ 3092.741627] commit_tail+0x7c/0xfc
> [ 3092.745145] drm_atomic_helper_commit+0x158/0x15c
> [ 3092.749998] drm_atomic_commit+0x60/0x74
> [ 3092.754055] drm_atomic_helper_update_plane+0x100/0x110
> [ 3092.759449] __setplane_atomic+0x11c/0x120
> [ 3092.763685] drm_mode_cursor_universal+0x188/0x22c
> [ 3092.768633] drm_mode_cursor_common+0x120/0x1f8
> [ 3092.773310] drm_mode_cursor_ioctl+0x68/0x8c
> [ 3092.21] drm_ioctl_kernel+0xe8/0x168
> [ 3092.781770] drm_ioctl+0x320/0x370
> [ 3092.785289] drm_compat_ioctl+0x40/0xdc
> [ 3092.789257] __arm64_compat_sys_ioctl+0xe0/0x150
> [ 3092.794030] invoke_syscall+0x80/0x114
> [ 3092.797905] el0_svc_common.constprop.3+0xc4/0xf8
> [ 3092.802765] do_el0_svc_compat+0x2c/0x54
> [ 3092.806811] el0_svc_compat+0x4c/0xe4
> [ 3092.810598] el0t_32_sync_handler+0xc4/0xf4
> [ 3092.814914] el0t_32_sync+0x174/0x178
> [ 3092.818701] irq event stamp: 55940
> [ 3092.822217] hardirqs
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi Doug
Thanks for the response, some comments below.
Abhinav
On 4/7/2022 1:47 PM, Doug Anderson wrote:
Hi,
On Thu, Apr 7, 2022 at 1:11 PM Abhinav Kumar wrote:
Hi Doug and Dmitry
Sorry, but I caught up on this email just now.
Some comments below.
Thanks
Abhinav
On 4/7/2022 10:07 AM, Doug Anderson wrote:
Hi,
On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC)
wrote:
Hi Dmitry and Doug,
Hi,
On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov
wrote:
On 05/04/2022 20:02, Doug Anderson wrote:
Hi,
On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
wrote:
3. For DP and eDP HPD means something a little different.
Essentially there are two concepts: a) is a display physically
connected and b) is the display powered up and ready. For DP, the
two are really tied together. From the kernel's point of view you
never "power down" a DP display and you can't detect that it's
physically connected until it's ready. Said another way, on you
tie "is a display there" to the HPD line and the moment a display
is there it's ready for you to do AUX transfers. For eDP, in the
lowest power state of a display it _won't_ assert its "HPD"
signal. However, it's still physically present. For eDP you simply
have to _assume_ it's present without any actual proof since you
can't get proof until you power it up. Thus for eDP, you report
that the display is there as soon as we're asked. We can't _talk_
to the display yet, though. So in get_modes() we need to be able
to power the display on enough to talk over the AUX channel to it.
As part of this, we wait for the signal named "HPD" which really means
"panel finished powering on" in this context.
NOTE: for aux transfer, we don't have the _display_ pipe and
clocks running. We only have enough stuff running to do the AUX
transfer.
We're not clocking out pixels. We haven't fully powered on the
display. The AUX transfer is designed to be something that can be
done early _before_ you turn on the display.
OK, so basically that was a longwinded way of saying: yes, we
could avoid the AUX transfer in probe, but we can't wait all the
way to enable. We have to be able to transfer in get_modes(). If
you think that's helpful I think it'd be a pretty easy patch to
write even if it would look a tad bit awkward IMO. Let me know if
you want me to post it up.
I think it would be a good idea. At least it will allow us to
judge, which is the more correct way.
I'm still happy to prototype this, but the more I think about it the
more it feels like a workaround for the Qualcomm driver. The eDP
panel driver is actually given a pointer to the AUX bus at probe
time. It's really weird to say that we can't do a transfer on it
yet... As you said, this is a little sideband bus. It should be able
to be used without all the full blown infra of the rest of the driver.
Yes, I have that feeling too. However I also have a feeling that just
powering up the PHY before the bus probe is ... a hack. There are no
obvious stopgaps for the driver not to power it down later.
Lets go back to why we need to power up the PHY before the bus probe.
We need to power up PHY before bus probe because panel-eDP tries to read
the EDID in probe() for the panel_id. Not get_modes().
So doug, I didnt follow your comment that panel-eDP only does EDID read
in get_modes()
panel_id = drm_edid_get_panel_id(panel->ddc);
if (!panel_id) {
dev_err(dev, "Couldn't identify panel via EDID\n");
ret = -EIO;
goto exit;
}
If we do not need this part, we really dont need to power up the PHY
before the probe(). The hack which dmitry was referring to.
Right. ...so we _could_ remove the above from the panel-edp probe and
defer it to get_modes() and it wouldn't be that hard. ...but:
1. It feels like a hack to work around the Qualcomm driver. The way
the AUX bus is designed is that a pointer to the AUX bus is passed to
the panel-edp probe. It seems kinda strange to say that the panel
isn't allowed to do transfers with the pointer that's passed in.
And thats why to satisfy the requirements of passing an initialized AUX,
sankeerth is delaying devm_of_dp_aux_populate_ep_devices() till PHY is
initialized which seems reasonable to satisfy the probe() time requirements.
Even if we move to pm_runtime(), yes I agree it will club all the
resources needed to control AUX in one place but you will still have to
initialize PHY before probe() under the hood of pm_runtime().
So how will it help this cause?
We just have to accept that initializing PHY is a requirement to use AUX
and before calling panel-eDP's probe(), we have to have an initialized AUX.
So we are not working around the driver but just satisfying the hardware
requirements to be able to satisfy panel-edp's and
drm_panel_dp_aux_backlight()'s aux bus requirements.
2. There's a second place where we might do an AUX transfer at probe
time which is when we're using the DP AUX backli
[PATCH] drm/radeon: change cayman_default_state table from global to static
cayman_default_state and cayman_default_size are only
used in ni.c. Single file symbols should be static.
So move their definitions to cayman_blit_shaders.h
and change their storage-class-specifier to static.
Remove unneeded cayman_blit_shader.c
cayman_ps/vs definitions were removed with
commit 4f8629675800 ("drm/radeon/kms: remove r6xx+ blit copy routines")
So their declarations in cayman_blit_shader.h
are not needed, so remove them.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/radeon/Makefile | 2 +-
drivers/gpu/drm/radeon/cayman_blit_shaders.c | 320 ---
drivers/gpu/drm/radeon/cayman_blit_shaders.h | 294 -
3 files changed, 290 insertions(+), 326 deletions(-)
delete mode 100644 drivers/gpu/drm/radeon/cayman_blit_shaders.c
diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile
index 664381f4eb07..2425a3612d6c 100644
--- a/drivers/gpu/drm/radeon/Makefile
+++ b/drivers/gpu/drm/radeon/Makefile
@@ -42,7 +42,7 @@ radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \
r200.o radeon_legacy_tv.o r600_cs.o r600_blit_shaders.o \
radeon_pm.o atombios_dp.o r600_hdmi.o dce3_1_afmt.o \
evergreen.o evergreen_cs.o evergreen_blit_shaders.o \
- evergreen_hdmi.o radeon_trace_points.o ni.o cayman_blit_shaders.o \
+ evergreen_hdmi.o radeon_trace_points.o ni.o \
atombios_encoders.o radeon_semaphore.o radeon_sa.o atombios_i2c.o si.o \
radeon_prime.o cik.o cik_blit_shaders.o \
r600_dpm.o rs780_dpm.o rv6xx_dpm.o rv770_dpm.o rv730_dpm.o rv740_dpm.o \
diff --git a/drivers/gpu/drm/radeon/cayman_blit_shaders.c
b/drivers/gpu/drm/radeon/cayman_blit_shaders.c
deleted file mode 100644
index 9fec4d09f383..
--- a/drivers/gpu/drm/radeon/cayman_blit_shaders.c
+++ /dev/null
@@ -1,320 +0,0 @@
-/*
- * Copyright 2010 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
- * DEALINGS IN THE SOFTWARE.
- *
- * Authors:
- * Alex Deucher
- */
-
-#include
-#include
-#include
-
-/*
- * evergreen cards need to use the 3D engine to blit data which requires
- * quite a bit of hw state setup. Rather than pull the whole 3D driver
- * (which normally generates the 3D state) into the DRM, we opt to use
- * statically generated state tables. The register state and shaders
- * were hand generated to support blitting functionality. See the 3D
- * driver or documentation for descriptions of the registers and
- * shader instructions.
- */
-
-const u32 cayman_default_state[] =
-{
- 0xc0066900,
- 0x,
- 0x0060, /* DB_RENDER_CONTROL */
- 0x, /* DB_COUNT_CONTROL */
- 0x, /* DB_DEPTH_VIEW */
- 0x002a, /* DB_RENDER_OVERRIDE */
- 0x, /* DB_RENDER_OVERRIDE2 */
- 0x, /* DB_HTILE_DATA_BASE */
-
- 0xc0026900,
- 0x000a,
- 0x, /* DB_STENCIL_CLEAR */
- 0x, /* DB_DEPTH_CLEAR */
-
- 0xc0036900,
- 0x000f,
- 0x, /* DB_DEPTH_INFO */
- 0x, /* DB_Z_INFO */
- 0x, /* DB_STENCIL_INFO */
-
- 0xc0016900,
- 0x0080,
- 0x, /* PA_SC_WINDOW_OFFSET */
-
- 0xc00d6900,
- 0x0083,
- 0x, /* PA_SC_CLIPRECT_RULE */
- 0x, /* PA_SC_CLIPRECT_0_TL */
- 0x20002000, /* PA_SC_CLIPRECT_0_BR */
- 0x,
- 0x20002000,
- 0x,
- 0x20002000,
- 0x,
- 0x20002000,
- 0x, /* PA_SC_EDGERULE */
- 0x, /* PA_SU_HARDWARE_SCREEN_OFFSET */
- 0x000f, /* CB_TARGET_MASK */
- 0x000f, /* CB_SHADER_MASK */
-
- 0xc0226900,
- 0x0094,
- 0x8000, /* PA_SC_VPORT_SCISSOR_0_TL */
- 0x20002000, /* PA_SC_VPORT_SCISSOR_0_BR */
- 0x8000,
- 0x20002000,
- 0x8000,
- 0x20002000,
-
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Thu, Apr 7, 2022 at 1:43 PM Hans de Goede wrote: > > Hi Simon, > > On 4/7/22 18:51, Simon Ser wrote: > > Very nice plan! Big +1 for the overall approach. > > Thanks. > > > On Thursday, April 7th, 2022 at 17:38, Hans de Goede > > wrote: > > > >> The drm_connector brightness properties > >> === > >> > >> bl_brightness: rw 0-int32_max property controlling the brightness setting > >> of the connected display. The actual maximum of this will be less then > >> int32_max and is given in bl_brightness_max. > > > > Do we need to split this up into two props for sw/hw state? The privacy > > screen > > stuff needed this, but you're pretty familiar with that. :) > > Luckily that won't be necessary, since the privacy-screen is a security > feature the firmware/embedded-controller may refuse our requests > (may temporarily lock-out changes) and/or may make changes without > us requesting them itself. Neither is really the case with the > brightness setting of displays. > > >> bl_brightness_max: ro 0-int32_max property giving the actual maximum > >> of the display's brightness setting. This will report 0 when brightness > >> control is not available (yet). > > > > I don't think we actually need that one. Integer KMS props all have a > > range which can be fetched via drmModeGetProperty. The max can be > > exposed via this range. Example with the existing alpha prop: > > > > "alpha": range [0, UINT16_MAX] = 65535 > > Right, I already knew that, which is why I explicitly added a range > to the props already. The problem is that the range must be set > before registering the connector and when the backlight driver > only shows up (much) later during boot then we don't know the > range when registering the connector. I guess we could "patch-up" > the range later. But AFAIK that would be a bit of abuse of the > property API as the range is intended to never change, not > even after hotplug uevents. At least atm there is no infra > in the kernel to change the range later. > > Which is why I added an explicit bl_brightness_max property > of which the value gives the actual effective maximum of the > brightness. > > I did consider using the range for this and updating it > on the fly I think nothing is really preventing us from > doing so, but it very much feels like abusing the generic > properties API. > > >> bl_brightness_0_is_min_brightness: ro, boolean > >> When this is set to true then it is safe to set brightness to 0 > >> without worrying that this completely turns the backlight off causing > >> the screen to become unreadable. When this is false setting brightness > >> to 0 may turn the backlight off, but this is not guaranteed. > >> This will e.g. be true when directly driving a PWM and the video-BIOS > >> has provided a minimum (non 0) duty-cycle below which the driver will > >> never go. > > > > Hm. It's quite unfortunate that it's impossible to have strong guarantees > > here. > > > > Is there any way we can avoid this prop? > > Not really, the problem is that we really don't know if 0 is off > or min-brightness. In the given example where we actually never go > down to a duty-cycle of 0% because the video BIOS tables tell us > not to, we can be certain that setting the brightness prop to 0 > will not turn of the backlight, since we then set the duty-cycle > to the VBT provided minimum. Note the intend here is to only set > the boolean to true if the VBT provided minimum is _not_ 0, 0 > just means the vendor did not bother to provide a minimum. > > Currently e.g. GNOME never goes lower then something like 5% > of brightness_max to avoid accidentally turning the screen off. > > Turning the screen off is quite bad to do on e.g. tablets where > the GUI is the only way to undo the brightness change and now > the user can no longer see the GUI. > > The idea behind this boolean is to give e.g. GNOME a way to > know that it is safe to go down to 0% and for it to use > the entire range. Why not just make it policy that 0 is defined as minimum brightness, not off, and have all drivers conform to that? Alex > > > For instance if we can guarantee that the min level won't turn the screen > > completely off we could make the range start from 1 instead of 0. > > Or allow -1 to mean "minimum value, maybe completely off". > > Right, the problem is we really don't know and when the range is > e.g. 0-65535 then something like 1 will almost always still just > turn the screen completely off. There will be a value of say like > 150 or some such which is then the actual minimum value to still > get the backlight to light up at all. The problem is we have > no clue what the actual minimum is. And if the PWM output does > not directly drive the LEDs but is used as an input for some > LED backlight driver chip, that chip itself may have a lookup > table (which may also take care of doing perceived brightness > mapping) and may guarantee a minimum backlight even when given > a 0% duty cycle
Re: [Intel-gfx] [PATCH 1/1] drm/i915/uc: Use platform specific defaults for GuC/HuC enabling
On 4/7/2022 08:49, Tvrtko Ursulin wrote: On 03/06/2021 17:48, Matthew Brost wrote: From: John Harrison The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms. Signed-off-by: John Harrison Signed-off-by: Matthew Brost Reviewed-by: Daniele Ceraolo Spurio --- drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. " - "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)"); + "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)"); i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. " diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \ - param(int, enable_guc, 0, 0400) \ + param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \ What is the BKM to use this with multi-GPU setups? Specifically I have a TGL+DG1 laptop (off the shelf) and want to have GuC with DG1 only. If I pass i915.enable_guc=3 it seems it wants to enable it for TGL as well and wedges the GPU if it can't? I don't think there is one. Module parameters are driver global and therefore apply to all cards in the system, both discrete and integrated. The '-1' default can and does have different meanings for each card. So in the TGL+DG1 case, TGL should default to execlist and DG1 should already be defaulting to GuC. So the -1 setting should do what you want. But if you did need to override for one specific card only then I think you would need to do that with a code change and rebuild. John. Regards, Tvrtko
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi,
On Thu, Apr 7, 2022 at 1:11 PM Abhinav Kumar wrote:
>
> Hi Doug and Dmitry
>
> Sorry, but I caught up on this email just now.
>
> Some comments below.
>
> Thanks
>
> Abhinav
> On 4/7/2022 10:07 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC)
> > wrote:
> >>
> >> Hi Dmitry and Doug,
> >>
> >>> Hi,
> >>>
> >>> On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov
> >>> wrote:
>
> On 05/04/2022 20:02, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
> > wrote:
> >>> 3. For DP and eDP HPD means something a little different.
> >>> Essentially there are two concepts: a) is a display physically
> >>> connected and b) is the display powered up and ready. For DP, the
> >>> two are really tied together. From the kernel's point of view you
> >>> never "power down" a DP display and you can't detect that it's
> >>> physically connected until it's ready. Said another way, on you
> >>> tie "is a display there" to the HPD line and the moment a display
> >>> is there it's ready for you to do AUX transfers. For eDP, in the
> >>> lowest power state of a display it _won't_ assert its "HPD"
> >>> signal. However, it's still physically present. For eDP you simply
> >>> have to _assume_ it's present without any actual proof since you
> >>> can't get proof until you power it up. Thus for eDP, you report
> >>> that the display is there as soon as we're asked. We can't _talk_
> >>> to the display yet, though. So in get_modes() we need to be able
> >>> to power the display on enough to talk over the AUX channel to it.
> >>> As part of this, we wait for the signal named "HPD" which really means
> >>> "panel finished powering on" in this context.
> >>>
> >>> NOTE: for aux transfer, we don't have the _display_ pipe and
> >>> clocks running. We only have enough stuff running to do the AUX
> >>> transfer.
> >>> We're not clocking out pixels. We haven't fully powered on the
> >>> display. The AUX transfer is designed to be something that can be
> >>> done early _before_ you turn on the display.
> >>>
> >>>
> >>> OK, so basically that was a longwinded way of saying: yes, we
> >>> could avoid the AUX transfer in probe, but we can't wait all the
> >>> way to enable. We have to be able to transfer in get_modes(). If
> >>> you think that's helpful I think it'd be a pretty easy patch to
> >>> write even if it would look a tad bit awkward IMO. Let me know if
> >>> you want me to post it up.
> >>
> >> I think it would be a good idea. At least it will allow us to
> >> judge, which is the more correct way.
> >
> > I'm still happy to prototype this, but the more I think about it the
> > more it feels like a workaround for the Qualcomm driver. The eDP
> > panel driver is actually given a pointer to the AUX bus at probe
> > time. It's really weird to say that we can't do a transfer on it
> > yet... As you said, this is a little sideband bus. It should be able
> > to be used without all the full blown infra of the rest of the driver.
>
> Yes, I have that feeling too. However I also have a feeling that just
> powering up the PHY before the bus probe is ... a hack. There are no
> obvious stopgaps for the driver not to power it down later.
> >>>
>
> Lets go back to why we need to power up the PHY before the bus probe.
>
> We need to power up PHY before bus probe because panel-eDP tries to read
> the EDID in probe() for the panel_id. Not get_modes().
>
> So doug, I didnt follow your comment that panel-eDP only does EDID read
> in get_modes()
>
> panel_id = drm_edid_get_panel_id(panel->ddc);
> if (!panel_id) {
> dev_err(dev, "Couldn't identify panel via EDID\n");
> ret = -EIO;
> goto exit;
> }
>
> If we do not need this part, we really dont need to power up the PHY
> before the probe(). The hack which dmitry was referring to.
Right. ...so we _could_ remove the above from the panel-edp probe and
defer it to get_modes() and it wouldn't be that hard. ...but:
1. It feels like a hack to work around the Qualcomm driver. The way
the AUX bus is designed is that a pointer to the AUX bus is passed to
the panel-edp probe. It seems kinda strange to say that the panel
isn't allowed to do transfers with the pointer that's passed in.
2. There's a second place where we might do an AUX transfer at probe
time which is when we're using the DP AUX backlight. There we call
drm_panel_dp_aux_backlight(). Conceivably this too could be deferred
until the get_modes(), but now it feels even more like a hack. We're
going to be registering the backlight in the first call to
get_modes()? That's, ummm, unexpected. We could look at perhaps
breaking the "DP AUX backlight" in two parts also, but that gets
involved. I think w
[PATCH] drm/msm: Fix range size vs end confusion
From: Rob Clark
The fourth param is size, rather than range_end.
Note that we could increase the address space size if we had a way to
prevent buffers from spanning a 4G split, mostly just to avoid fw bugs
with 64b math.
Fixes: 84c31ee16f90 ("drm/msm/a6xx: Add support for per-instance pagetables")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 17de46fc4bf2..80d57608b34a 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1742,7 +1742,7 @@ a6xx_create_private_address_space(struct msm_gpu *gpu)
return ERR_CAST(mmu);
return msm_gem_address_space_create(mmu,
- "gpu", 0x1ULL, 0x1ULL);
+ "gpu", 0x1ULL, SZ_4G);
}
static uint32_t a6xx_get_rptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
--
2.35.1
[RESEND PATCH] drm/armada: drop unneeded MODULE_ALIAS
The MODULE_DEVICE_TABLE already creates proper alias for platform
driver. Having another MODULE_ALIAS causes the alias to be duplicated.
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/armada/armada_drv.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/armada/armada_drv.c
b/drivers/gpu/drm/armada/armada_drv.c
index 0643887800b4..0a819a4908b5 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -285,4 +285,3 @@ module_exit(armada_drm_exit);
MODULE_AUTHOR("Russell King ");
MODULE_DESCRIPTION("Armada DRM Driver");
MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:armada-drm");
--
2.32.0
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi Doug and Dmitry
Sorry, but I caught up on this email just now.
Some comments below.
Thanks
Abhinav
On 4/7/2022 10:07 AM, Doug Anderson wrote:
Hi,
On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC)
wrote:
Hi Dmitry and Doug,
Hi,
On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov
wrote:
On 05/04/2022 20:02, Doug Anderson wrote:
Hi,
On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
wrote:
3. For DP and eDP HPD means something a little different.
Essentially there are two concepts: a) is a display physically
connected and b) is the display powered up and ready. For DP, the
two are really tied together. From the kernel's point of view you
never "power down" a DP display and you can't detect that it's
physically connected until it's ready. Said another way, on you
tie "is a display there" to the HPD line and the moment a display
is there it's ready for you to do AUX transfers. For eDP, in the
lowest power state of a display it _won't_ assert its "HPD"
signal. However, it's still physically present. For eDP you simply
have to _assume_ it's present without any actual proof since you
can't get proof until you power it up. Thus for eDP, you report
that the display is there as soon as we're asked. We can't _talk_
to the display yet, though. So in get_modes() we need to be able
to power the display on enough to talk over the AUX channel to it.
As part of this, we wait for the signal named "HPD" which really means
"panel finished powering on" in this context.
NOTE: for aux transfer, we don't have the _display_ pipe and
clocks running. We only have enough stuff running to do the AUX
transfer.
We're not clocking out pixels. We haven't fully powered on the
display. The AUX transfer is designed to be something that can be
done early _before_ you turn on the display.
OK, so basically that was a longwinded way of saying: yes, we
could avoid the AUX transfer in probe, but we can't wait all the
way to enable. We have to be able to transfer in get_modes(). If
you think that's helpful I think it'd be a pretty easy patch to
write even if it would look a tad bit awkward IMO. Let me know if
you want me to post it up.
I think it would be a good idea. At least it will allow us to
judge, which is the more correct way.
I'm still happy to prototype this, but the more I think about it the
more it feels like a workaround for the Qualcomm driver. The eDP
panel driver is actually given a pointer to the AUX bus at probe
time. It's really weird to say that we can't do a transfer on it
yet... As you said, this is a little sideband bus. It should be able
to be used without all the full blown infra of the rest of the driver.
Yes, I have that feeling too. However I also have a feeling that just
powering up the PHY before the bus probe is ... a hack. There are no
obvious stopgaps for the driver not to power it down later.
Lets go back to why we need to power up the PHY before the bus probe.
We need to power up PHY before bus probe because panel-eDP tries to read
the EDID in probe() for the panel_id. Not get_modes().
So doug, I didnt follow your comment that panel-eDP only does EDID read
in get_modes()
panel_id = drm_edid_get_panel_id(panel->ddc);
if (!panel_id) {
dev_err(dev, "Couldn't identify panel via EDID\n");
ret = -EIO;
goto exit;
}
If we do not need this part, we really dont need to power up the PHY
before the probe(). The hack which dmitry was referring to.
So this is boiling down to why or how panel-eDP was originally designed.
This is why I think we need to move to Runtime PM to manage this. Basically:
1. When an AUX transfer happens, you grab a PM runtime reference that
_that_ powers up the PHY.
This will not be trivial and needs to be scoped out as sankeerth said
but if the above is the only concern, why do we need to do this? There
seems to be an explanation why we are doing this and its not a hack.
How would Dmitry's rework address this? We need some RFC to conclude on
that first.
2. At the end of the AUX transfer function, you do a "put_autosuspend".
Then it becomes not a hack, right?
pm runtime ops needs to be implemented for both eDP and DP. This change
take good amount of planning and code changes as it affects DP also.
Because this patch series consist of basic eDP changes for SC7280 bootup,
shall we take this pm_runtime implementation in subsequent patch series?
Dmitry is the real decision maker here, but in my opinion it would be
OK to get something landed first that worked OK and wasn't taking us
too far in the wrong direction and then we could get a follow up patch
to move to pm_runtime.
I would say the discussion changed into a direction of implementing
pm-runtime because the current patch series does what it takes to adhere
to panel-eDP's design along with aux bus requirements of PHY needing to
be on.
So doug, to answer your questions here:
"So I guess the net result i
[PATCH 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver
These are declared in the ssd130x-i2c transport driver but the information
is not I2C specific and could be used by other SSD130x transport drivers.
Move them to the ssd130x core driver and just set the OF device entries to
an ID that could be used to lookup the correct device into from an array.
While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
bytes. Since even though are used by the I2C interface, it could also be
useful for other transport protocols such as SPI.
Suggested-by: Chen-Yu Tsai
Signed-off-by: Javier Martinez Canillas
---
drivers/gpu/drm/solomon/ssd130x-i2c.c | 51 ---
drivers/gpu/drm/solomon/ssd130x.c | 60 +--
drivers/gpu/drm/solomon/ssd130x.h | 13 ++
3 files changed, 78 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c
b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index a469679548f8..aa6cc2cb54f9 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -53,76 +53,43 @@ static void ssd130x_i2c_shutdown(struct i2c_client *client)
ssd130x_shutdown(ssd130x);
}
-static struct ssd130x_deviceinfo ssd130x_sh1106_deviceinfo = {
- .default_vcomh = 0x40,
- .default_dclk_div = 1,
- .default_dclk_frq = 5,
- .page_mode_only = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1305_deviceinfo = {
- .default_vcomh = 0x34,
- .default_dclk_div = 1,
- .default_dclk_frq = 7,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1306_deviceinfo = {
- .default_vcomh = 0x20,
- .default_dclk_div = 1,
- .default_dclk_frq = 8,
- .need_chargepump = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1307_deviceinfo = {
- .default_vcomh = 0x20,
- .default_dclk_div = 2,
- .default_dclk_frq = 12,
- .need_pwm = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1309_deviceinfo = {
- .default_vcomh = 0x34,
- .default_dclk_div = 1,
- .default_dclk_frq = 10,
-};
-
static const struct of_device_id ssd130x_of_match[] = {
{
.compatible = "sinowealth,sh1106-i2c",
- .data = &ssd130x_sh1106_deviceinfo,
+ .data = SH1106_ID,
},
{
.compatible = "solomon,ssd1305-i2c",
- .data = &ssd130x_ssd1305_deviceinfo,
+ .data = (void *)SSD1305_ID,
},
{
.compatible = "solomon,ssd1306-i2c",
- .data = &ssd130x_ssd1306_deviceinfo,
+ .data = (void *)SSD1306_ID,
},
{
.compatible = "solomon,ssd1307-i2c",
- .data = &ssd130x_ssd1307_deviceinfo,
+ .data = (void *)SSD1307_ID,
},
{
.compatible = "solomon,ssd1309-i2c",
- .data = &ssd130x_ssd1309_deviceinfo,
+ .data = (void *)SSD1309_ID,
},
/* Deprecated but remain for backward compatibility */
{
.compatible = "solomon,ssd1305fb-i2c",
- .data = &ssd130x_ssd1305_deviceinfo,
+ .data = (void *)SSD1305_ID,
},
{
.compatible = "solomon,ssd1306fb-i2c",
- .data = &ssd130x_ssd1306_deviceinfo,
+ .data = (void *)SSD1306_ID,
},
{
.compatible = "solomon,ssd1307fb-i2c",
- .data = &ssd130x_ssd1307_deviceinfo,
+ .data = (void *)SSD1307_ID,
},
{
.compatible = "solomon,ssd1309fb-i2c",
- .data = &ssd130x_ssd1309_deviceinfo,
+ .data = (void *)SSD1309_ID,
},
{ /* sentinel */ }
};
diff --git a/drivers/gpu/drm/solomon/ssd130x.c
b/drivers/gpu/drm/solomon/ssd130x.c
index a7e784518c69..1f00fd3c0023 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -39,11 +39,9 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
-#define SSD130X_DATA 0x40
-#define SSD130X_COMMAND0x80
-
#define SSD130X_PAGE_COL_START_LOW 0x00
#define SSD130X_PAGE_COL_START_HIGH0x10
+
#define SSD130X_SET_ADDRESS_MODE 0x20
#define SSD130X_SET_COL_RANGE 0x21
#define SSD130X_SET_PAGE_RANGE 0x22
@@ -94,6 +92,55 @@
#define MAX_CONTRAST 255
+static struct ssd130x_deviceinfo ssd130x_variants[] = {
+ {
+ .default_vcomh = 0x40,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 5,
+ .page_mode_only = 1,
+ },
+ {
+ .variant = SSD1305_ID,
+ .default_vcomh = 0x34,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 7,
+ },
+ {
+ .variant = SSD1306_ID,
+ .default_vcomh = 0x20,
+ .default_dclk_div = 1,
+
[PATCH 5/5] drm/solomon: Add SSD130x OLED displays SPI support
The ssd130x driver only provides the core support for these devices but it
does not have any bus transport logic. Add a driver to interface over SPI.
There is a difference in the communication protocol when using 4-wire SPI
instead of I2C. For the latter, a control byte that contains a D/C# field
has to be sent. This field tells the controller whether the data has to be
written to the command register or to the graphics display data memory.
But for 4-wire SPI that control byte is not used, instead a real D/C# line
must be pulled HIGH for commands data and LOW for graphics display data.
For this reason the standard SPI regmap can't be used and a custom .write
bus handler is needed.
Signed-off-by: Javier Martinez Canillas
---
drivers/gpu/drm/solomon/Kconfig | 9 ++
drivers/gpu/drm/solomon/Makefile | 1 +
drivers/gpu/drm/solomon/ssd130x-spi.c | 184 ++
3 files changed, 194 insertions(+)
create mode 100644 drivers/gpu/drm/solomon/ssd130x-spi.c
diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig
index 8c0a0c788385..e170716d976b 100644
--- a/drivers/gpu/drm/solomon/Kconfig
+++ b/drivers/gpu/drm/solomon/Kconfig
@@ -20,3 +20,12 @@ config DRM_SSD130X_I2C
I2C bus.
If M is selected the module will be called ssd130x-i2c.
+
+config DRM_SSD130X_SPI
+ tristate "DRM support for Solomon SSD130X OLED displays (SPI bus)"
+ depends on DRM_SSD130X && SPI
+ select REGMAP
+ help
+ Say Y here if the SSD130x OLED display is connected via SPI bus.
+
+ If M is selected the module will be called ssd130x-spi.
diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile
index 4bfc5acb0447..b5fc792257d7 100644
--- a/drivers/gpu/drm/solomon/Makefile
+++ b/drivers/gpu/drm/solomon/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_DRM_SSD130X) += ssd130x.o
obj-$(CONFIG_DRM_SSD130X_I2C) += ssd130x-i2c.o
+obj-$(CONFIG_DRM_SSD130X_SPI) += ssd130x-spi.o
diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c
b/drivers/gpu/drm/solomon/ssd130x-spi.c
new file mode 100644
index ..c3a2e7ba67a1
--- /dev/null
+++ b/drivers/gpu/drm/solomon/ssd130x-spi.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for Solomon SSD130X OLED displays (SPI bus)
+ *
+ * Copyright 2022 Red Hat Inc.
+ * Authors: Javier Martinez Canillas
+ */
+#include
+#include
+
+#include "ssd130x.h"
+
+#define DRIVER_NAME"ssd130x-spi"
+#define DRIVER_DESC"DRM driver for Solomon SSD130X OLED displays (SPI)"
+
+struct ssd130x_spi_transport {
+ struct spi_device *spi;
+ struct gpio_desc *dc;
+};
+
+static const struct regmap_config ssd130x_spi_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+/*
+ * The regmap bus .write handler, it is just a wrapper around spi_write()
+ * but toggling the Data/Command control pin (D/C#). Since for 4-wire SPI
+ * a D/C# pin is used, in contrast with I2C where a control byte is sent,
+ * prior to every data byte, that contains a bit with the D/C# value.
+ *
+ * These control bytes are considered registers by the ssd130x core driver
+ * and can be used by the ssd130x SPI driver to determine if the data sent
+ * is for a command register or for the Graphic Display Data RAM (GDDRAM).
+ */
+static int ssd130x_spi_write(void *context, const void *data, size_t count)
+{
+ struct ssd130x_spi_transport *t = context;
+ struct spi_device *spi = t->spi;
+ const u8 *reg = data;
+
+ if (*reg == SSD130X_COMMAND)
+ gpiod_set_value_cansleep(t->dc, 0);
+
+ if (*reg == SSD130X_DATA)
+ gpiod_set_value_cansleep(t->dc, 1);
+
+ /* Remove the control byte since is not used by the 4-wire SPI */
+ return spi_write(spi, ((u8 *)data) + 1, count - 1);
+}
+
+/* The ssd130x driver does not read registers but regmap expects a .read */
+static int ssd130x_spi_read(void *context, const void *reg, size_t reg_size,
+ void *val, size_t val_size)
+{
+ return 0;
+}
+
+/*
+ * A custom bus is needed due the special write that toggles a D/C# pin,
+ * another option could be to just have a .reg_write() callback but that
+ * will prevent to do data writes in bulk.
+ *
+ * Once the regmap API is extended to support defining a bulk write handler
+ * in the struct regmap_config, this can be simplified and the bus dropped.
+ */
+static struct regmap_bus regmap_ssd130x_spi_bus = {
+ .write = ssd130x_spi_write,
+ .read = ssd130x_spi_read,
+};
+
+static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev)
+{
+ struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
+
+ if (IS_ERR(dc))
+ return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get
dc gpio\n"));
+
+ return dc;
+}
+
+static int ssd130x_spi_probe(struct spi_device *spi)
+{
+ struct ssd130x_spi_transport *t;
+ struct ssd130x_device *ssd130x;
+
[PATCH 3/5] drm/solomon: Add ssd130x-i2c compatible strings without an -fb suffix
The current compatible strings for SSD130x I2C controllers contain an -fb
suffix, this seems to indicate that are for a fbdev driver. But the DT is
supposed to describe the hardware and not Linux implementation details.
Let's deprecate those compatible strings and add new ones that don't have
the -fb suffix. The old entries are still kept for backward compatibility.
Signed-off-by: Javier Martinez Canillas
---
drivers/gpu/drm/solomon/ssd130x-i2c.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c
b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index d099b241dd3f..a469679548f8 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -91,6 +91,23 @@ static const struct of_device_id ssd130x_of_match[] = {
.compatible = "sinowealth,sh1106-i2c",
.data = &ssd130x_sh1106_deviceinfo,
},
+ {
+ .compatible = "solomon,ssd1305-i2c",
+ .data = &ssd130x_ssd1305_deviceinfo,
+ },
+ {
+ .compatible = "solomon,ssd1306-i2c",
+ .data = &ssd130x_ssd1306_deviceinfo,
+ },
+ {
+ .compatible = "solomon,ssd1307-i2c",
+ .data = &ssd130x_ssd1307_deviceinfo,
+ },
+ {
+ .compatible = "solomon,ssd1309-i2c",
+ .data = &ssd130x_ssd1309_deviceinfo,
+ },
+ /* Deprecated but remain for backward compatibility */
{
.compatible = "solomon,ssd1305fb-i2c",
.data = &ssd130x_ssd1305_deviceinfo,
--
2.35.1
[PATCH 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers
The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
add to the schema the compatible strings, properties and examples for SPI.
Signed-off-by: Javier Martinez Canillas
---
.../bindings/display/solomon,ssd1307fb.yaml | 89 +++
1 file changed, 71 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index 46207f2c12b8..05e7975296a7 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -31,6 +31,15 @@ properties:
- solomon,ssd1307-i2c
- solomon,ssd1309-i2c
+ # SSD130x SPI controllers
+ - items:
+ - enum:
+ - sinowealth,sh1106-spi
+ - solomon,ssd1305-spi
+ - solomon,ssd1306-spi
+ - solomon,ssd1307-spi
+ - solomon,ssd1309-spi
+
reg:
maxItems: 1
@@ -40,9 +49,14 @@ properties:
reset-gpios:
maxItems: 1
+ dc-gpios:
+maxItems: 1
+
vbat-supply:
description: The supply for VBAT
+ spi-max-frequency: true
+
solomon,height:
$ref: /schemas/types.yaml#/definitions/uint32
default: 16
@@ -148,19 +162,10 @@ allOf:
properties:
compatible:
contains:
-const: sinowealth,sh1106-i2c
-then:
- properties:
-solomon,dclk-div:
- default: 1
-solomon,dclk-frq:
- default: 5
-
- - if:
- properties:
-compatible:
- contains:
-const: solomon,ssd1305-i2c
+enum:
+ - sinowealth,sh1106-i2c
+ - solomon,ssd1305-i2c
+ - solomon,ssd1305-spi
then:
properties:
solomon,dclk-div:
@@ -172,7 +177,9 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1306-i2c
+enum:
+ - solomon,ssd1306-i2c
+ - solomon,ssd1306-spi
then:
properties:
solomon,dclk-div:
@@ -184,7 +191,9 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1307-i2c
+enum:
+ - solomon,ssd1307-i2c
+ - solomon,ssd1307-spi
then:
properties:
solomon,dclk-div:
@@ -198,7 +207,9 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1309-i2c
+enum:
+ - solomon,ssd1309-i2c
+ - solomon,ssd1309-spi
then:
properties:
solomon,dclk-div:
@@ -206,6 +217,21 @@ allOf:
solomon,dclk-frq:
default: 10
+ - if:
+ properties:
+compatible:
+ contains:
+enum:
+ - sinowealth,sh1106-spi
+ - solomon,ssd1305-spi
+ - solomon,ssd1306-spi
+ - solomon,ssd1307-spi
+ - solomon,ssd1309-spi
+then:
+ required:
+- spi-max-frequency
+- dc-gpios
+
additionalProperties: false
examples:
@@ -214,14 +240,14 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
-ssd1307: oled@3c {
+ssd1307_i2c: oled@3c {
compatible = "solomon,ssd1307-i2c";
reg = <0x3c>;
pwms = <&pwm 4 3000>;
reset-gpios = <&gpio2 7>;
};
-ssd1306: oled@3d {
+ssd1306_i2c: oled@3d {
compatible = "solomon,ssd1306-i2c";
reg = <0x3c>;
pwms = <&pwm 4 3000>;
@@ -232,3 +258,30 @@ examples:
solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
};
};
+ - |
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+ssd1307_spi: oled@0 {
+compatible = "solomon,ssd1307-spi";
+reg = <0x0>;
+pwms = <&pwm 4 3000>;
+reset-gpios = <&gpio2 7>;
+dc-gpios = <&gpio2 8>;
+spi-max-frequency = <1000>;
+};
+
+ssd1306_spi: oled@1 {
+compatible = "solomon,ssd1306-spi";
+reg = <0x1>;
+pwms = <&pwm 4 3000>;
+reset-gpios = <&gpio2 7>;
+dc-gpios = <&gpio2 8>;
+spi-max-frequency = <1000>;
+solomon,com-lrremap;
+solomon,com-invdir;
+solomon,com-offset = <32>;
+solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
+};
+};
--
2.35.1
[PATCH 1/5] dt-bindings: display: ssd1307fb: Deprecate fbdev compatible strings
The current compatible strings for SSD130x I2C controllers contain an -fb
suffix, this seems to indicate that are for a fbdev driver. But the DT is
supposed to describe the hardware and not Linux implementation details.
Let's deprecate those compatible strings and add a new enum that contains
compatible strings that don't have a -fb suffix. These will be matched by
the ssd130x-i2c DRM driver.
Signed-off-by: Javier Martinez Canillas
---
.../bindings/display/solomon,ssd1307fb.yaml | 36 ---
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index ade61d502edd..46207f2c12b8 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -12,12 +12,24 @@ maintainers:
properties:
compatible:
-enum:
- - sinowealth,sh1106-i2c
- - solomon,ssd1305fb-i2c
- - solomon,ssd1306fb-i2c
- - solomon,ssd1307fb-i2c
- - solomon,ssd1309fb-i2c
+oneOf:
+ # Deprecated compatible strings
+ - items:
+ - enum:
+ - solomon,ssd1305fb-i2c
+ - solomon,ssd1306fb-i2c
+ - solomon,ssd1307fb-i2c
+ - solomon,ssd1309fb-i2c
+deprecated: true
+
+ # SSD130x I2C controllers
+ - items:
+ - enum:
+ - sinowealth,sh1106-i2c
+ - solomon,ssd1305-i2c
+ - solomon,ssd1306-i2c
+ - solomon,ssd1307-i2c
+ - solomon,ssd1309-i2c
reg:
maxItems: 1
@@ -148,7 +160,7 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1305fb-i2c
+const: solomon,ssd1305-i2c
then:
properties:
solomon,dclk-div:
@@ -160,7 +172,7 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1306fb-i2c
+const: solomon,ssd1306-i2c
then:
properties:
solomon,dclk-div:
@@ -172,7 +184,7 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1307fb-i2c
+const: solomon,ssd1307-i2c
then:
properties:
solomon,dclk-div:
@@ -186,7 +198,7 @@ allOf:
properties:
compatible:
contains:
-const: solomon,ssd1309fb-i2c
+const: solomon,ssd1309-i2c
then:
properties:
solomon,dclk-div:
@@ -203,14 +215,14 @@ examples:
#size-cells = <0>;
ssd1307: oled@3c {
-compatible = "solomon,ssd1307fb-i2c";
+compatible = "solomon,ssd1307-i2c";
reg = <0x3c>;
pwms = <&pwm 4 3000>;
reset-gpios = <&gpio2 7>;
};
ssd1306: oled@3d {
-compatible = "solomon,ssd1306fb-i2c";
+compatible = "solomon,ssd1306-i2c";
reg = <0x3c>;
pwms = <&pwm 4 3000>;
reset-gpios = <&gpio2 7>;
--
2.35.1
[PATCH 0/5] drm/solomon: Add SSD130x OLED displays SPI support
Hello, This series adds a ssd130x-spi driver that provides a 4-wire SPI transport support for SSD130x OLED controllers that can be accessed through a SPI. The driver is quite similar to existing ssd130x-i2c driver that is used by I2C controllers, but there is a difference in the protocol used by SSD130x depending on the transport used. The details are in patch #4 description. Patch #1 just makes the current ssd130x-i2c compatible strings in the DT binding to be deprecated, and add new ones that don't have an -fb suffix. Patch #2 extends the DT binding with the compatible string and properties needed to support the ssd130x-spi devices. Patch #3 adds the new compatible strings to the OF device ID table in the ssd130x-i2c DRM driver. Patch #4 moves the device info for the different SSD130x variants from the ssd130x-i2c transport driver to the ssd130x core driver. Finally patch #5 adds the ssd130x-spi DRM driver for the OLED controllers that come with a 4-wire SPI interface, instead of an I2C interface. Best regards, Javier Javier Martinez Canillas (5): dt-bindings: display: ssd1307fb: Deprecate fbdev compatible strings dt-bindings: display: ssd1307fb: Extend schema for SPI controllers drm/solomon: Add ssd130x-i2c compatible strings without an -fb suffix drm/solomon: Move device info from ssd130x-i2c to the core driver drm/solomon: Add SSD130x OLED displays SPI support .../bindings/display/solomon,ssd1307fb.yaml | 117 --- drivers/gpu/drm/solomon/Kconfig | 9 + drivers/gpu/drm/solomon/Makefile | 1 + drivers/gpu/drm/solomon/ssd130x-i2c.c | 60 +++--- drivers/gpu/drm/solomon/ssd130x-spi.c | 184 ++ drivers/gpu/drm/solomon/ssd130x.c | 60 +- drivers/gpu/drm/solomon/ssd130x.h | 13 ++ 7 files changed, 376 insertions(+), 68 deletions(-) create mode 100644 drivers/gpu/drm/solomon/ssd130x-spi.c -- 2.35.1
Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
On 4/4/22 10:47, Zheyu Ma wrote:
> The userspace program could pass any values to the driver through
> ioctl() interface. If the driver doesn't check the value of 'pixclock',
> it may cause divide error.
>
> Fix this by checking whether 'pixclock' is zero in the function
> i740fb_check_var().
>
> The following log reveals it:
>
> divide error: [#1] PREEMPT SMP KASAN PTI
> RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline]
> RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739
> Call Trace:
> fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036
> do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112
> fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:874 [inline]
>
> Signed-off-by: Zheyu Ma
Hello Zheyu,
I've applied the patches #2-#7 of this series, but left
out this specific patch (for now).
As discussed on the mailing list we can try to come up with a
better fix (to round up the pixclock when it's invalid).
If not, I will apply this one later.
Thanks!
Helge
> ---
> drivers/video/fbdev/i740fb.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
> index 52cce0db8bd3..b595437a5752 100644
> --- a/drivers/video/fbdev/i740fb.c
> +++ b/drivers/video/fbdev/i740fb.c
> @@ -657,6 +657,9 @@ static int i740fb_decode_var(const struct
> fb_var_screeninfo *var,
>
> static int i740fb_check_var(struct fb_var_screeninfo *var, struct fb_info
> *info)
> {
> + if (!var->pixclock)
> + return -EINVAL;
> +
> switch (var->bits_per_pixel) {
> case 8:
> var->red.offset = var->green.offset = var->blue.offset = 0;
Re: [PATCH] video: fbdev: Fix missing of_node_put in imxfb_probe
On 4/7/22 11:01, cgel@gmail.com wrote:
> From: Lv Ruyi
>
> of_parse_phandle returns node pointer with refcount incremented,
> use of_node_put() on it when done.
>
> Reported-by: Zeal Robot
> Signed-off-by: Lv Ruyi
applied.
Thanks!
Helge
> ---
> drivers/video/fbdev/imxfb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
> index 68288756..a2f644c97f28 100644
> --- a/drivers/video/fbdev/imxfb.c
> +++ b/drivers/video/fbdev/imxfb.c
> @@ -925,10 +925,12 @@ static int imxfb_probe(struct platform_device *pdev)
> sizeof(struct imx_fb_videomode), GFP_KERNEL);
> if (!fbi->mode) {
> ret = -ENOMEM;
> + of_node_put(display_np);
> goto failed_of_parse;
> }
>
> ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
> + of_node_put(display_np);
> if (ret)
> goto failed_of_parse;
> }
Re: [PATCH 1/2] drm/dp: Export drm_dp_dpcd_access()
On Thu, 07 Apr 2022, Imre Deak wrote:
> The next patch needs a way to read a DPCD register without the preceding
> wake-up read in drm_dp_dpcd_read(). Export drm_dp_dpcd_access() to allow
> this.
I think I'd rather you added a special "probe" function for this
specific purpose. I think drm_dp_dpcd_access() is a too generic low
level function to export, and runs the risk of complicating any
potential future refactoring of the DP AUX code.
Something like this:
ssize_t drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset);
And you could reuse that for the wakeup in drm_dp_dpcd_read() too.
BR,
Jani.
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/dp/drm_dp.c| 19 +--
> include/drm/dp/drm_dp_helper.h | 2 ++
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/dp/drm_dp.c b/drivers/gpu/drm/dp/drm_dp.c
> index 580016a1b9eb7..8b181314edcbe 100644
> --- a/drivers/gpu/drm/dp/drm_dp.c
> +++ b/drivers/gpu/drm/dp/drm_dp.c
> @@ -470,8 +470,22 @@ drm_dp_dump_access(const struct drm_dp_aux *aux,
> * Both native and I2C-over-AUX transactions are supported.
> */
>
> -static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
> - unsigned int offset, void *buffer, size_t size)
> +/**
> + * drm_dp_dpcd_access() - read or write a series of bytes from/to the DPCD
> + * @aux: DisplayPort AUX channel (SST)
> + * @request: DP_AUX_NATIVE_READ or DP_AUX_NATIVE_WRITE
> + * @offset: address of the (first) register to read or write
> + * @buffer: buffer with the register values to write or the register values
> read
> + * @size: number of bytes in @buffer
> + *
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. This is a low-level function only for SST sinks and cases
> + * where calling drm_dp_dpcd_read()/write() is not possible (for instance due
> + * to the wake-up register read in drm_dp_dpcd_read()). For all other cases
> + * the latter functions should be used.
> + */
> +int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
> +unsigned int offset, void *buffer, size_t size)
> {
> struct drm_dp_aux_msg msg;
> unsigned int retry, native_reply;
> @@ -526,6 +540,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
> request,
> mutex_unlock(&aux->hw_mutex);
> return ret;
> }
> +EXPORT_SYMBOL(drm_dp_dpcd_access);
>
> /**
> * drm_dp_dpcd_read() - read a series of bytes from the DPCD
> diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h
> index 1eccd97419436..7cf6e83434a8c 100644
> --- a/include/drm/dp/drm_dp_helper.h
> +++ b/include/drm/dp/drm_dp_helper.h
> @@ -2053,6 +2053,8 @@ struct drm_dp_aux {
> bool is_remote;
> };
>
> +int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
> +unsigned int offset, void *buffer, size_t size);
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
>void *buffer, size_t size);
> ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
--
Jani Nikula, Intel Open Source Graphics Center
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Thu, 7 Apr 2022 17:38:59 +0200 Hans de Goede said: Below you covered our usual /sys/class/backlight device friends... what about DDC monitor controls? These function similarly but just remotely control a screen via I2C and also suffer from the same problems of "need root" and "have to do some fun in mapping them to a given screen". Otherwise I welcome this de-uglification of the backlight device and putting it together with the drm device that controls that monitor. Just to make life more fun ... DDC does much more than backlight controls. It can control essentially anything that is in the OSD for your monitor (contrast, brightness, backlight, sharpness, color temperatures, input to display (DP vs HDMI vs DVI etc.), an for extra fun points can even tel you the current rotation state of your monitor. All of these do make sense to live as drm connector properties too. Perhaps not a first iteration but something to consider in this design. > As discussed already several times in the past: > https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/ > > https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/ > > The current userspace API for brightness control offered by > /sys/class/backlight devices has various issues, the biggest 2 being: > > 1. There is no way to map the backlight device to a specific >display-output / panel (1) > 2. Controlling the brightness requires root-rights requiring >desktop-environments to use suid-root helpers for this. > > As already discussed on various conference's hallway tracks > and as has been proposed on the dri-devel list once before (2), > it seems that there is consensus that the best way to to solve these > 2 issues is to add support for controlling a video-output's brightness > through properties on the drm_connector. > > This RFC outlines my plan to try and actually implement this, > which has 3 phases: > > > Phase 1: Stop registering multiple /sys/class/backlight devs for a single > display > = > > On x86 there can be multiple firmware + direct-hw-access methods > for controlling the backlight and in some cases the kernel registers > multiple backlight-devices for a single internal laptop LCD panel: > > a) i915 and nouveau unconditionally register their "native" backlight dev >even on devices where /sys/class/backlight/acpi_video0 must be used >to control the backlight, relying on userspace to prefer the "firmware" >acpi_video0 device over "native" devices. > b) amdgpu and nouveau rely on the acpi_video driver initializing before >them, which currently causes /sys/class/backlight/acpi_video0 to usually >show up and then they register their own native backlight driver after >which the drivers/acpi/video_detect.c code unregisters the acpi_video0 >device. This means that userspace briefly sees 2 devices and the >disappearing of acpi_video0 after a brief time confuses the systemd >backlight level save/restore code, see e.g.: >https://bbs.archlinux.org/viewtopic.php?id=269920 > > I already have a pretty detailed plan to tackle this, which I will > post in a separate RFC email. I plan to start working on this right > away, as it will be good to have this fixed regardless. > > > Phase 2: Add drm_connector properties mirroring the matching backlight device > = > > The plan is to add a drm_connector helper function, which optionally takes > a pointer to the backlight device for the GPU's native backlight device, > which will then mirror the backlight settings from the backlight device > in a set of read/write brightness* properties on the connector. > > This function can then be called by GPU drivers for the drm_connector for > the internal panel and it will then take care of everything. When there > is no native GPU backlight device, or when it should not be used then > (on x86) the helper will use the acpi_video_get_backlight_type() to > determine which backlight-device should be used instead and it will find > + mirror that one. > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > Once most userspace has moved over to using the new drm_connector > brightness props, a Kconfig option can be added to stop exporting > the backlight-devices under /sys/class/backlight. The plan is to > just disable the sysfs interface and keep the existing backlight-device > internal kernel abstraction as is, since some abstraction for (non GPU > native) backlight devices will be necessary regardless. > > An alternative to disabling the sysfs class entirely, would be > to allow setting it to read-only through Kconfig. > > > What scale to use for the drm_connector bl_brightness property? > === > > The tricky part of this plan is
[PATCH 2/2] drm: bridge: icn6211: Add DSI lane count DT property parsing
The driver currently hard-codes DSI lane count to two, however the chip
is capable of operating in 1..4 DSI lanes mode. Parse 'data-lanes' DT
property and program the result into DSI_CTRL register.
Signed-off-by: Marek Vasut
Cc: Jagan Teki
Cc: Laurent Pinchart
Cc: Maxime Ripard
Cc: Robert Foss
Cc: Sam Ravnborg
Cc: Thomas Zimmermann
To: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/bridge/chipone-icn6211.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c
b/drivers/gpu/drm/bridge/chipone-icn6211.c
index 6ce0633d4c089..e2b599a44275c 100644
--- a/drivers/gpu/drm/bridge/chipone-icn6211.c
+++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
@@ -395,6 +395,11 @@ static void chipone_atomic_enable(struct drm_bridge
*bridge,
/* dsi specific sequence */
chipone_writeb(icn, SYNC_EVENT_DLY, 0x80);
chipone_writeb(icn, HFP_MIN, hfp & 0xff);
+
+ /* DSI data lane count */
+ chipone_writeb(icn, DSI_CTRL,
+ DSI_CTRL_UNKNOWN | DSI_CTRL_DSI_LANES(icn->dsi->lanes - 1));
+
chipone_writeb(icn, MIPI_PD_CK_LANE, 0xa0);
chipone_writeb(icn, PLL_CTRL(12), 0xff);
chipone_writeb(icn, MIPI_PN_SWAP, 0x00);
@@ -480,9 +485,23 @@ static void chipone_mode_set(struct drm_bridge *bridge,
static int chipone_dsi_attach(struct chipone *icn)
{
struct mipi_dsi_device *dsi = icn->dsi;
- int ret;
+ struct device *dev = icn->dev;
+ struct device_node *endpoint;
+ int dsi_lanes, ret;
+
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+ dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
+ of_node_put(endpoint);
+
+ /*
+* If the 'data-lanes' property does not exist in DT or is invalid,
+* default to previously hard-coded behavior, which was 4 data lanes.
+*/
+ if (dsi_lanes >= 1 && dsi_lanes <= 4)
+ icn->dsi->lanes = dsi_lanes;
+ else
+ icn->dsi->lanes = 4;
- dsi->lanes = 4;
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
--
2.35.1
[PATCH 1/2] dt-bindings: display: bridge: icn6211: Document DSI data-lanes property
It is necessary to specify the number of connected/used DSI data lanes when using the DSI input port of this bridge. Document the 'data-lanes' property of the DSI input port. Signed-off-by: Marek Vasut Cc: Jagan Teki Cc: Laurent Pinchart Cc: Maxime Ripard Cc: Rob Herring Cc: Robert Foss Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: devicet...@vger.kernel.org To: dri-devel@lists.freedesktop.org --- NOTE: This is consistent with all the other DSI panel and bridge bindings which document 'data-lanes' property, all of which already use OF graph and have the 'data-lanes' property in the port@N subnode, see: $ git grep -l data-lanes Documentation/devicetree/bindings/display/ --- .../display/bridge/chipone,icn6211.yaml| 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml index 7257fd0ae4da8..4f0b7c71313c3 100644 --- a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml +++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml @@ -41,10 +41,26 @@ properties: properties: port@0: -$ref: /schemas/graph.yaml#/properties/port +$ref: /schemas/graph.yaml#/$defs/port-base +unevaluatedProperties: false description: Video port for MIPI DSI input +properties: + endpoint: +$ref: /schemas/media/video-interfaces.yaml# +unevaluatedProperties: false + +properties: + data-lanes: +description: array of physical DSI data lane indexes. +minItems: 1 +items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + port@1: $ref: /schemas/graph.yaml#/properties/port description: -- 2.35.1
[PATCH 1/2] drm/dp: Export drm_dp_dpcd_access()
The next patch needs a way to read a DPCD register without the preceding
wake-up read in drm_dp_dpcd_read(). Export drm_dp_dpcd_access() to allow
this.
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Imre Deak
---
drivers/gpu/drm/dp/drm_dp.c| 19 +--
include/drm/dp/drm_dp_helper.h | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/dp/drm_dp.c b/drivers/gpu/drm/dp/drm_dp.c
index 580016a1b9eb7..8b181314edcbe 100644
--- a/drivers/gpu/drm/dp/drm_dp.c
+++ b/drivers/gpu/drm/dp/drm_dp.c
@@ -470,8 +470,22 @@ drm_dp_dump_access(const struct drm_dp_aux *aux,
* Both native and I2C-over-AUX transactions are supported.
*/
-static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
- unsigned int offset, void *buffer, size_t size)
+/**
+ * drm_dp_dpcd_access() - read or write a series of bytes from/to the DPCD
+ * @aux: DisplayPort AUX channel (SST)
+ * @request: DP_AUX_NATIVE_READ or DP_AUX_NATIVE_WRITE
+ * @offset: address of the (first) register to read or write
+ * @buffer: buffer with the register values to write or the register values
read
+ * @size: number of bytes in @buffer
+ *
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure. This is a low-level function only for SST sinks and cases
+ * where calling drm_dp_dpcd_read()/write() is not possible (for instance due
+ * to the wake-up register read in drm_dp_dpcd_read()). For all other cases
+ * the latter functions should be used.
+ */
+int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
+ unsigned int offset, void *buffer, size_t size)
{
struct drm_dp_aux_msg msg;
unsigned int retry, native_reply;
@@ -526,6 +540,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
request,
mutex_unlock(&aux->hw_mutex);
return ret;
}
+EXPORT_SYMBOL(drm_dp_dpcd_access);
/**
* drm_dp_dpcd_read() - read a series of bytes from the DPCD
diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h
index 1eccd97419436..7cf6e83434a8c 100644
--- a/include/drm/dp/drm_dp_helper.h
+++ b/include/drm/dp/drm_dp_helper.h
@@ -2053,6 +2053,8 @@ struct drm_dp_aux {
bool is_remote;
};
+int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
+ unsigned int offset, void *buffer, size_t size);
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
void *buffer, size_t size);
ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
--
2.30.2
RE: [PATCH] drm/amdgpu: Ensure HDA function is suspended before ASIC reset
[Public]
> -Original Message-
> From: Alex Deucher
> Sent: Thursday, April 7, 2022 12:08
> To: Kai-Heng Feng
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Pan, Xinhui ; David
> Airlie ; Maling list - DRI developers de...@lists.freedesktop.org>; LKML ; amd-
> gfx list ; Chiu, Solomon
> ; Limonciello, Mario
> ; Quan, Evan ;
> Tuikov, Luben
> Subject: Re: [PATCH] drm/amdgpu: Ensure HDA function is suspended
> before ASIC reset
>
> On Thu, Apr 7, 2022 at 8:21 AM Kai-Heng Feng
> wrote:
> >
> > DP/HDMI audio on AMD PRO VII stops working after S3:
> > [ 149.450391] amdgpu :63:00.0: amdgpu: MODE1 reset
> > [ 149.450395] amdgpu :63:00.0: amdgpu: GPU mode1 reset
> > [ 149.450494] amdgpu :63:00.0: amdgpu: GPU psp mode1 reset
> > [ 149.983693] snd_hda_intel :63:00.1: refused to change power state
> from D0 to D3hot
> > [ 150.003439] amdgpu :63:00.0: refused to change power state from
> D0 to D3hot
> > ...
> > [ 155.432975] snd_hda_intel :63:00.1: CORB reset timeout#2, CORBRP
> = 65535
>
> As an aside, shouldn't device links order this properly already? I
> thought that was the whole point of them. We have quirks in PCI
> quirks.c to create device links for all GPU integrated peripherals
> (audio, usb, ucsi).
The movement from D0->D3 only happens in noirq though in pci_pm_suspend_noirq.
So if device links only order within a suspend phase then resetting during that
phase means the future phase won't have ground to stand on.
IOW I think device links + this commit are needed to get the order right with a
reset.
>
> Alex
>
> >
> > The offending commit is daf8de0874ab5b ("drm/amdgpu: always reset the
> asic in
> > suspend (v2)"). Commit 34452ac3038a7 ("drm/amdgpu: don't use BACO for
> > reset in S3 ") doesn't help, so the issue is something different.
> >
> > Assuming that to make HDA resume to D0 fully realized, it needs to be
> > successfully put to D3 first. And this guesswork proves working, by
> > moving amdgpu_asic_reset() to noirq callback, so it's called after HDA
> > function is in D3.
> >
> > Fixes: daf8de0874ab5b ("drm/amdgpu: always reset the asic in suspend
> (v2)")
> > Signed-off-by: Kai-Heng Feng
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 --
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index bb1c025d90019..31f7229e7ea89 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -2323,18 +2323,23 @@ static int amdgpu_pmops_suspend(struct
> device *dev)
> > {
> > struct drm_device *drm_dev = dev_get_drvdata(dev);
> > struct amdgpu_device *adev = drm_to_adev(drm_dev);
> > - int r;
> >
> > if (amdgpu_acpi_is_s0ix_active(adev))
> > adev->in_s0ix = true;
> > else
> > adev->in_s3 = true;
> > - r = amdgpu_device_suspend(drm_dev, true);
> > - if (r)
> > - return r;
> > + return amdgpu_device_suspend(drm_dev, true);
> > +}
> > +
> > +static int amdgpu_pmops_suspend_noirq(struct device *dev)
> > +{
> > + struct drm_device *drm_dev = dev_get_drvdata(dev);
> > + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> > +
> > if (!adev->in_s0ix)
> > - r = amdgpu_asic_reset(adev);
> > - return r;
> > + return amdgpu_asic_reset(adev);
> > +
> > + return 0;
> > }
> >
> > static int amdgpu_pmops_resume(struct device *dev)
> > @@ -2575,6 +2580,7 @@ static const struct dev_pm_ops amdgpu_pm_ops
> = {
> > .prepare = amdgpu_pmops_prepare,
> > .complete = amdgpu_pmops_complete,
> > .suspend = amdgpu_pmops_suspend,
> > + .suspend_noirq = amdgpu_pmops_suspend_noirq,
> > .resume = amdgpu_pmops_resume,
> > .freeze = amdgpu_pmops_freeze,
> > .thaw = amdgpu_pmops_thaw,
> > --
> > 2.34.1
> >
Re: [Intel-gfx] [PATCH v2 21/22] drm/edid: Extract drm_edid_decode_mfg_id()
On Tue, 05 Apr 2022, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Make the PNPID decoding available for other users.
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä
Reviewed-by: Jani Nikula
> ---
> include/drm/drm_edid.h | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index b7e170584000..5e9d7fcda8e7 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -508,6 +508,22 @@ static inline u8 drm_eld_get_conn_type(const uint8_t
> *eld)
> return eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_CONN_TYPE_MASK;
> }
>
> +/**
> + * drm_edid_decode_mfg_id - Decode the manufacturer ID
> + * @mfg_id: The manufacturer ID
> + * @vend: A 4-byte buffer to store the 3-letter vendor string plus a '\0'
> + * termination
> + */
> +static inline const char *drm_edid_decode_mfg_id(u16 mfg_id, char vend[4])
> +{
> + vend[0] = '@' + ((mfg_id >> 10) & 0x1f);
> + vend[1] = '@' + ((mfg_id >> 5) & 0x1f);
> + vend[2] = '@' + ((mfg_id >> 0) & 0x1f);
> + vend[3] = '\0';
> +
> + return vend;
> +}
> +
> /**
> * drm_edid_encode_panel_id - Encode an ID for matching against
> drm_edid_get_panel_id()
> * @vend_chr_0: First character of the vendor string.
> @@ -548,10 +564,7 @@ static inline u8 drm_eld_get_conn_type(const uint8_t
> *eld)
> static inline void drm_edid_decode_panel_id(u32 panel_id, char vend[4], u16
> *product_id)
> {
> *product_id = (u16)(panel_id & 0x);
> - vend[0] = '@' + ((panel_id >> 26) & 0x1f);
> - vend[1] = '@' + ((panel_id >> 21) & 0x1f);
> - vend[2] = '@' + ((panel_id >> 16) & 0x1f);
> - vend[3] = '\0';
> + drm_edid_decode_mfg_id(panel_id >> 16, vend);
> }
>
> bool drm_probe_ddc(struct i2c_adapter *adapter);
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 1/4] drm/fourcc: Introduce format modifiers for DG2 render and media compression
Seems my first mail didn't come through so here's second time for this patch:
Reviewed-by: Juha-Pekka Heikkila
On Mon, Apr 4, 2022 at 4:39 PM Imre Deak wrote:
>
> From: Matt Roper
>
> The render/media engines on DG2 unify render compression and media
> compression into a single format for the first time, using the Tile 4
> layout for main surfaces. The compression algorithm is different from
> any previous platform and the display engine must still be configured to
> decompress either a render or media compressed surface; as such, we
> need new RC and MC framebuffer modifiers to represent buffers in this
> format.
>
> v2: Clarify modifier layout description.
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Matt Roper
> Signed-off-by: Imre Deak
> Acked-by: Nanley Chery
> ---
> include/uapi/drm/drm_fourcc.h | 22 ++
> 1 file changed, 22 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index b73fe6797fc37..4a5117715db3c 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -583,6 +583,28 @@ extern "C" {
> */
> #define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 9)
>
> +/*
> + * Intel color control surfaces (CCS) for DG2 render compression.
> + *
> + * The main surface is Tile 4 and at plane index 0. The CCS data is stored
> + * outside of the GEM object in a reserved memory area dedicated for the
> + * storage of the CCS data for all RC/RC_CC/MC compressible GEM objects. The
> + * main surface pitch is required to be a multiple of four Tile 4 widths.
> + */
> +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10)
> +
> +/*
> + * Intel color control surfaces (CCS) for DG2 media compression.
> + *
> + * The main surface is Tile 4 and at plane index 0. For semi-planar formats
> + * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
> + * 0 and 1, respectively. The CCS for all planes are stored outside of the
> + * GEM object in a reserved memory area dedicated for the storage of the
> + * CCS data for all RC/RC_CC/MC compressible GEM objects. The main surface
> + * pitch is required to be a multiple of four Tile 4 widths.
> + */
> +#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 11)
> +
> /*
> * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
> *
> --
> 2.30.2
>
Re: [PATCH v8, 16/17] media: mediatek: vcodec: support stateless VP9 decoding
Le mercredi 06 avril 2022 à 15:23 -0400, Nicolas Dufresne a écrit :
> Hi Yunfei,
>
> Le jeudi 31 mars 2022 à 10:48 +0800, Yunfei Dong a écrit :
> > Add support for VP9 decoding using the stateless API,
> > as supported by MT8192. And the drivers is lat and core architecture.
> >
> > Signed-off-by: George Sun
> > Signed-off-by: Xiaoyong Lu
> > Signed-off-by: Yunfei Dong
> > Reviewed-by: AngeloGioacchino Del Regno
> >
>
> Reviewed-by should be dropped when large rework happens. In this case, the
> probability updated has been rewritten to use the common code (thanks for
> porting it). Unfortunately, running fluster tests shows massive regression
> (was
> 275/303) before):
>
>Ran 34/303 tests successfully
>
> H.264 (91/135) and VP9 (59/61) are same as before. Any idea ? What was your
> test
> results ?
Build warnings were badly fixed in my tree. I'll comment inline, but everything
was catched by the CI, a V9 will be neede to finish cleanup build and doc
warnings. Note that Xiaoyong Lu also had crop info reading, I don't know if this
is needed.
>
> > ---
> > changed compare with v7:
> > Using upstream interface to update vp9 prob tables.
> > ---
> > .../media/platform/mediatek/vcodec/Makefile |1 +
> > .../vcodec/mtk_vcodec_dec_stateless.c | 26 +-
> > .../platform/mediatek/vcodec/mtk_vcodec_drv.h |1 +
> > .../vcodec/vdec/vdec_vp9_req_lat_if.c | 2072 +
> > .../platform/mediatek/vcodec/vdec_drv_if.c|4 +
> > .../platform/mediatek/vcodec/vdec_drv_if.h|1 +
> > 6 files changed, 2102 insertions(+), 3 deletions(-)
> > create mode 100644
> > drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> >
> > diff --git a/drivers/media/platform/mediatek/vcodec/Makefile
> > b/drivers/media/platform/mediatek/vcodec/Makefile
> > index b457daf2d196..93e7a343b5b0 100644
> > --- a/drivers/media/platform/mediatek/vcodec/Makefile
> > +++ b/drivers/media/platform/mediatek/vcodec/Makefile
> > @@ -9,6 +9,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \
> > vdec/vdec_vp8_if.o \
> > vdec/vdec_vp8_req_if.o \
> > vdec/vdec_vp9_if.o \
> > + vdec/vdec_vp9_req_lat_if.o \
> > vdec/vdec_h264_req_if.o \
> > vdec/vdec_h264_req_common.o \
> > vdec/vdec_h264_req_multi_if.o \
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> > index 3208f834ff80..a4735e67d39e 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> > @@ -91,13 +91,28 @@ static const struct mtk_stateless_control
> > mtk_stateless_controls[] = {
> > .max = V4L2_MPEG_VIDEO_VP8_PROFILE_3,
> > },
> > .codec_type = V4L2_PIX_FMT_VP8_FRAME,
> > - }
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_STATELESS_VP9_FRAME,
> > + },
> > + .codec_type = V4L2_PIX_FMT_VP9_FRAME,
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
> > + .min = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> > + .def = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> > + .max = V4L2_MPEG_VIDEO_VP9_PROFILE_3,
> > + },
> > + .codec_type = V4L2_PIX_FMT_VP9_FRAME,
> > + },
> > };
> >
> > #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> >
> > -static struct mtk_video_fmt mtk_video_formats[4];
> > -static struct mtk_codec_framesizes mtk_vdec_framesizes[2];
> > +static struct mtk_video_fmt mtk_video_formats[5];
> > +static struct mtk_codec_framesizes mtk_vdec_framesizes[3];
> >
> > static struct mtk_video_fmt default_out_format;
> > static struct mtk_video_fmt default_cap_format;
> > @@ -338,6 +353,7 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
> > switch (fourcc) {
> > case V4L2_PIX_FMT_H264_SLICE:
> > case V4L2_PIX_FMT_VP8_FRAME:
> > + case V4L2_PIX_FMT_VP9_FRAME:
> > mtk_video_formats[count_formats].fourcc = fourcc;
> > mtk_video_formats[count_formats].type = MTK_FMT_DEC;
> > mtk_video_formats[count_formats].num_planes = 1;
> > @@ -385,6 +401,10 @@ static void mtk_vcodec_get_supported_formats(struct
> > mtk_vcodec_ctx *ctx)
> > mtk_vcodec_add_formats(V4L2_PIX_FMT_VP8_FRAME, ctx);
> > out_format_count++;
> > }
> > + if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_VP9_FRAME) {
> > + mtk_vcodec_add_formats(V4L2_PIX_FMT_VP9_FRAME, ctx);
> > + out_format_count++;
> > + }
> >
> > if (cap_format_count)
> > default_cap_format = mtk_video_formats[cap_format_count - 1];
> > diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> > b/drivers/media/platform/mediatek/vcodec/mtk_vcod
Re: [PATCH] drm/tegra: Stop using iommu_present()
On 4/6/22 21:06, Robin Murphy wrote:
> On 2022-04-06 15:32, Dmitry Osipenko wrote:
>> On 4/5/22 17:19, Robin Murphy wrote:
>>> Remove the pointless check. host1x_drm_wants_iommu() cannot return true
>>> unless an IOMMU exists for the host1x platform device, which at the
>>> moment
>>> means the iommu_present() test could never fail.
>>>
>>> Signed-off-by: Robin Murphy
>>> ---
>>> drivers/gpu/drm/tegra/drm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>>> index 9464f522e257..bc4321561400 100644
>>> --- a/drivers/gpu/drm/tegra/drm.c
>>> +++ b/drivers/gpu/drm/tegra/drm.c
>>> @@ -1149,7 +1149,7 @@ static int host1x_drm_probe(struct
>>> host1x_device *dev)
>>> goto put;
>>> }
>>> - if (host1x_drm_wants_iommu(dev) &&
>>> iommu_present(&platform_bus_type)) {
>>> + if (host1x_drm_wants_iommu(dev)) {
>>> tegra->domain = iommu_domain_alloc(&platform_bus_type);
>>> if (!tegra->domain) {
>>> err = -ENOMEM;
>>
>> host1x_drm_wants_iommu() returns true if there is no IOMMU for the
>> host1x platform device of Tegra20/30 SoCs.
>
> Ah, apparently this is another example of what happens when I write
> patches late on a Friday night...
>
> So on second look, what we want to ascertain here is whether dev has an
> IOMMU, but only if the host1x parent is not addressing-limited, either
> because it can also use the IOMMU, or because all possible addresses are
> small enough anyway, right?
Yes
> Are we specifically looking for the host1x
> having a DMA-API-managed domain, or can that also end up using the
> tegra->domain or another unmanaged domain too?
We have host1x DMA that could have:
1. No IOMMU domain, depending on kernel/DT config
2. Managed domain, on newer SoCs
3. Unmanaged domain, on older SoCs
We have Tegra DRM devices which can:
1. Be attached to a shared unmanaged tegra->domain, on older SoCs
2. Have own managed domains, on newer SoCs
> I can't quite figure out
> from the comments whether it's physical addresses, IOVAs, or both that
> we're concerned with here.
Tegra DRM allocates buffers and submits jobs to h/w using host1x's
channel DMA. DRM framebuffers' addresses are inserted into host1x
command buffers by kernel driver and addresses beyond 32bit space need
to be treated specially, we don't support such addresses in upstream.
IOMMU AS is limited to 32bits on Tegra in upstream kernel for pre-T186
SoCs, it hides 64bit addresses from host1x. Post-T186 SoCs have extra
features that allow kernel driver not to bother about addresses.
For newer ARM64 SoCs there is assumption in the Tegra drivers that IOMMU
always presents, to simplify things.
Re: [PATCH] drm/amdgpu: Fix code with incorrect enum type
On Thu, Apr 7, 2022 at 2:21 AM Christian König wrote: > > Am 06.04.22 um 18:50 schrieb Grigory Vasilyev: > > Instead of the 'amdgpu_ring_priority_level' type, > > the 'amdgpu_gfx_pipe_priority' type was used, > > which is an error when setting ring priority. > > This is a minor error, but may cause problems in the future. > > > > Instead of AMDGPU_RING_PRIO_2 = 2, we can use AMDGPU_RING_PRIO_MAX = 3, > > but AMDGPU_RING_PRIO_2 = 2 is used for compatibility with > > AMDGPU_GFX_PIPE_PRIO_HIGH = 2, and not change the behavior of the > > code. > > > > Signed-off-by: Grigory Vasilyev > > Good catch, Acked-by: Christian König > > > --- > > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > > index 5554084ec1f1..9bc26395f833 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > > @@ -1929,7 +1929,7 @@ static int gfx_v8_0_compute_ring_init(struct > > amdgpu_device *adev, int ring_id, > > + ring->pipe; > > > > hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring) ? > > - AMDGPU_GFX_PIPE_PRIO_HIGH : AMDGPU_RING_PRIO_DEFAULT; > > + AMDGPU_RING_PRIO_2 : AMDGPU_RING_PRIO_DEFAULT; gfx_v8_0.c, gfx_v9_0.c, gfx_v10_0.c all do this. Care to fix them all up? Alex > > /* type-2 packets are deprecated on MEC, use type-3 instead */ > > r = amdgpu_ring_init(adev, ring, 1024, &adev->gfx.eop_irq, irq_type, > >hw_prio, NULL); >
Re: [RFC] drm/kms: control display brightness through drm_connector properties
Hi Simon, On 4/7/22 18:51, Simon Ser wrote: > Very nice plan! Big +1 for the overall approach. Thanks. > On Thursday, April 7th, 2022 at 17:38, Hans de Goede > wrote: > >> The drm_connector brightness properties >> === >> >> bl_brightness: rw 0-int32_max property controlling the brightness setting >> of the connected display. The actual maximum of this will be less then >> int32_max and is given in bl_brightness_max. > > Do we need to split this up into two props for sw/hw state? The privacy screen > stuff needed this, but you're pretty familiar with that. :) Luckily that won't be necessary, since the privacy-screen is a security feature the firmware/embedded-controller may refuse our requests (may temporarily lock-out changes) and/or may make changes without us requesting them itself. Neither is really the case with the brightness setting of displays. >> bl_brightness_max: ro 0-int32_max property giving the actual maximum >> of the display's brightness setting. This will report 0 when brightness >> control is not available (yet). > > I don't think we actually need that one. Integer KMS props all have a > range which can be fetched via drmModeGetProperty. The max can be > exposed via this range. Example with the existing alpha prop: > > "alpha": range [0, UINT16_MAX] = 65535 Right, I already knew that, which is why I explicitly added a range to the props already. The problem is that the range must be set before registering the connector and when the backlight driver only shows up (much) later during boot then we don't know the range when registering the connector. I guess we could "patch-up" the range later. But AFAIK that would be a bit of abuse of the property API as the range is intended to never change, not even after hotplug uevents. At least atm there is no infra in the kernel to change the range later. Which is why I added an explicit bl_brightness_max property of which the value gives the actual effective maximum of the brightness. I did consider using the range for this and updating it on the fly I think nothing is really preventing us from doing so, but it very much feels like abusing the generic properties API. >> bl_brightness_0_is_min_brightness: ro, boolean >> When this is set to true then it is safe to set brightness to 0 >> without worrying that this completely turns the backlight off causing >> the screen to become unreadable. When this is false setting brightness >> to 0 may turn the backlight off, but this is not guaranteed. >> This will e.g. be true when directly driving a PWM and the video-BIOS >> has provided a minimum (non 0) duty-cycle below which the driver will >> never go. > > Hm. It's quite unfortunate that it's impossible to have strong guarantees > here. > > Is there any way we can avoid this prop? Not really, the problem is that we really don't know if 0 is off or min-brightness. In the given example where we actually never go down to a duty-cycle of 0% because the video BIOS tables tell us not to, we can be certain that setting the brightness prop to 0 will not turn of the backlight, since we then set the duty-cycle to the VBT provided minimum. Note the intend here is to only set the boolean to true if the VBT provided minimum is _not_ 0, 0 just means the vendor did not bother to provide a minimum. Currently e.g. GNOME never goes lower then something like 5% of brightness_max to avoid accidentally turning the screen off. Turning the screen off is quite bad to do on e.g. tablets where the GUI is the only way to undo the brightness change and now the user can no longer see the GUI. The idea behind this boolean is to give e.g. GNOME a way to know that it is safe to go down to 0% and for it to use the entire range. > For instance if we can guarantee that the min level won't turn the screen > completely off we could make the range start from 1 instead of 0. > Or allow -1 to mean "minimum value, maybe completely off". Right, the problem is we really don't know and when the range is e.g. 0-65535 then something like 1 will almost always still just turn the screen completely off. There will be a value of say like 150 or some such which is then the actual minimum value to still get the backlight to light up at all. The problem is we have no clue what the actual minimum is. And if the PWM output does not directly drive the LEDs but is used as an input for some LED backlight driver chip, that chip itself may have a lookup table (which may also take care of doing perceived brightness mapping) and may guarantee a minimum backlight even when given a 0% duty cycle PWM signal... This prop is sort of orthogonal to the generic change to drm_connector props, so we could also do this later as a follow up change. At a minimum when I code this up this should be in its own commit(s) I believe. But I do think having this will be useful for the above GNOME example. >> bl_brightness_control_method: ro, enum, possible values: >> none: Th
Re: [PATCH] drm/amd/display: Fix indenting mistakes in dcn10_hw_sequencer.c
On Thu, Apr 7, 2022 at 1:32 PM Harry Wentland wrote:
>
>
>
> On 2022-04-07 12:07, Alex Deucher wrote:
> > Actually this just causes another warning. Dropped for now. More below.
> >
> > On Thu, Apr 7, 2022 at 11:52 AM Alex Deucher wrote:
> >>
> >> Applied. Thanks!
> >>
> >> Alex
> >>
> >> On Thu, Apr 7, 2022 at 10:18 AM Harry Wentland
> >> wrote:
> >>>
> >>>
> >>>
> >>> On 2022-04-07 02:00, Haowen Bai wrote:
> Smatch reports the following:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2174
> dcn10_enable_vblanks_synchronization() warn: if statement not indented
>
> Signed-off-by: Haowen Bai
> >>>
> >>> Reviewed-by: Harry Wentland
> >>>
> >>> Harry
> >>>
> ---
> drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 14
> +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index ee22f4422d26..3c338b85040c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2172,13 +2172,13 @@ void dcn10_enable_vblanks_synchronization(
> if (master >= 0) {
> for (i = 0; i < group_size; i++) {
> if (i != master &&
> !grouped_pipes[i]->stream->has_non_synchronizable_pclk)
> -
> grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> - grouped_pipes[master]->stream_res.tg,
> - grouped_pipes[i]->stream_res.tg,
> -
> grouped_pipes[master]->stream->timing.pix_clk_100hz,
> -
> grouped_pipes[i]->stream->timing.pix_clk_100hz,
> - get_clock_divider(grouped_pipes[master],
> false),
> - get_clock_divider(grouped_pipes[i],
> false));
> +
> grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> +
> grouped_pipes[master]->stream_res.tg,
> + grouped_pipes[i]->stream_res.tg,
> +
> grouped_pipes[master]->stream->timing.pix_clk_100hz,
> +
> grouped_pipes[i]->stream->timing.pix_clk_100hz,
> +
> get_clock_divider(grouped_pipes[master], false),
> +
> get_clock_divider(grouped_pipes[i], false));
>
> grouped_pipes[i]->stream->vblank_synchronized = true;
> >
> > @Harry Wentland should this last statement be part of the if clause or
> > the for loop?
> >
>
> It should be part of the if clause.
Can one of you send a patch to fix that up?
Thanks!
Alex
>
> Harry
>
> > Alex
> >
> }
> grouped_pipes[master]->stream->vblank_synchronized = true;
> >>>
>
Re: [PATCH] drm/amd/display: Fix indenting mistakes in dcn10_hw_sequencer.c
On 2022-04-07 12:07, Alex Deucher wrote:
> Actually this just causes another warning. Dropped for now. More below.
>
> On Thu, Apr 7, 2022 at 11:52 AM Alex Deucher wrote:
>>
>> Applied. Thanks!
>>
>> Alex
>>
>> On Thu, Apr 7, 2022 at 10:18 AM Harry Wentland
>> wrote:
>>>
>>>
>>>
>>> On 2022-04-07 02:00, Haowen Bai wrote:
Smatch reports the following:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2174
dcn10_enable_vblanks_synchronization() warn: if statement not indented
Signed-off-by: Haowen Bai
>>>
>>> Reviewed-by: Harry Wentland
>>>
>>> Harry
>>>
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 14
+++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index ee22f4422d26..3c338b85040c 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2172,13 +2172,13 @@ void dcn10_enable_vblanks_synchronization(
if (master >= 0) {
for (i = 0; i < group_size; i++) {
if (i != master &&
!grouped_pipes[i]->stream->has_non_synchronizable_pclk)
-
grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
- grouped_pipes[master]->stream_res.tg,
- grouped_pipes[i]->stream_res.tg,
-
grouped_pipes[master]->stream->timing.pix_clk_100hz,
-
grouped_pipes[i]->stream->timing.pix_clk_100hz,
- get_clock_divider(grouped_pipes[master],
false),
- get_clock_divider(grouped_pipes[i], false));
+
grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
+ grouped_pipes[master]->stream_res.tg,
+ grouped_pipes[i]->stream_res.tg,
+
grouped_pipes[master]->stream->timing.pix_clk_100hz,
+
grouped_pipes[i]->stream->timing.pix_clk_100hz,
+
get_clock_divider(grouped_pipes[master], false),
+ get_clock_divider(grouped_pipes[i],
false));
grouped_pipes[i]->stream->vblank_synchronized = true;
>
> @Harry Wentland should this last statement be part of the if clause or
> the for loop?
>
It should be part of the if clause.
Harry
> Alex
>
}
grouped_pipes[master]->stream->vblank_synchronized = true;
>>>
Re: [PATCH] drm/amd/display: cleanup extern usage in function definition
Applied. Thanks!
Alex
On Mon, Apr 4, 2022 at 11:57 AM Harry Wentland wrote:
>
>
>
> On 2022-04-04 11:43, Tom Rix wrote:
> >
> > On 4/4/22 8:22 AM, Harry Wentland wrote:
> >>
> >> On 2022-04-03 10:21, Tom Rix wrote:
> >>> Smatch reports this issue
> >>> hdcp1_execution.c:500:29: warning: function
> >>>'mod_hdcp_hdcp1_dp_execution' with external linkage
> >>>has definition
> >>>
> >> Which branch are you using?
> >
> > linux-next from 4/1
> >
>
> Apologies. I was looking at the wrong function.
>
> Reviewed-by: Harry Wentland
>
> Harry
>
> > Tom
> >
> >>
> >> I don't see the 'extern' on
> >> https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c>>>
> >> Harry
> >>
> >>
> >>> The storage-class-specifier extern is not needed in a
> >>> definition, so remove it.
> >>>
> >>> Signed-off-by: Tom Rix
> >>> ---
> >>> drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c | 6 +++---
> >>> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
> >>> b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
> >>> index 6ec918af3bff..1ddb4f5eac8e 100644
> >>> --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
> >>> +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
> >>> @@ -497,9 +497,9 @@ enum mod_hdcp_status mod_hdcp_hdcp1_execution(struct
> >>> mod_hdcp *hdcp,
> >>> return status;
> >>> }
> >>> -extern enum mod_hdcp_status mod_hdcp_hdcp1_dp_execution(struct
> >>> mod_hdcp *hdcp,
> >>> -struct mod_hdcp_event_context *event_ctx,
> >>> -struct mod_hdcp_transition_input_hdcp1 *input)
> >>> +enum mod_hdcp_status mod_hdcp_hdcp1_dp_execution(struct mod_hdcp *hdcp,
> >>> + struct mod_hdcp_event_context *event_ctx,
> >>> + struct mod_hdcp_transition_input_hdcp1 *input)
> >>> {
> >>> enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
> >>>
> >
>
Re: [PATCH v2 18/19] Revert "fbdev: Prevent probing generic drivers if a FB is already registered"
On Tue, Apr 05, 2022 at 07:29:22PM +0200, Daniel Vetter wrote: > On Tue, 5 Apr 2022 at 18:45, Greg KH wrote: > > > > On Tue, Apr 05, 2022 at 06:12:59PM +0200, Daniel Vetter wrote: > > > On Tue, Apr 05, 2022 at 03:33:17PM +0200, Greg KH wrote: > > > > On Tue, Apr 05, 2022 at 03:24:40PM +0200, Geert Uytterhoeven wrote: > > > > > Hi Daniel, > > > > > > > > > > On Tue, Apr 5, 2022 at 1:48 PM Daniel Vetter wrote: > > > > > > On Tue, 5 Apr 2022 at 11:52, Javier Martinez Canillas > > > > > > wrote: > > > > > > > On 4/5/22 11:24, Daniel Vetter wrote: > > > > > > > > On Tue, 5 Apr 2022 at 11:19, Javier Martinez Canillas > > > > > > > >> This is how I think that work, please let me know if you see > > > > > > > >> something > > > > > > > >> wrong in my logic: > > > > > > > >> > > > > > > > >> 1) A PCI device of OF device is registered for the GPU, this > > > > > > > >> attempt to > > > > > > > >>match a registered driver but no driver was registered that > > > > > > > >> match yet. > > > > > > > >> > > > > > > > >> 2) The efifb driver is built-in, will be initialized according > > > > > > > >> to the link > > > > > > > >>order of the objects under drivers/video and the fbdev > > > > > > > >> driver is registered. > > > > > > > >> > > > > > > > >>There is no platform device or PCI/OF device registered > > > > > > > >> that matches. > > > > > > > >> > > > > > > > >> 3) The DRM driver is built-in, will be initialized according > > > > > > > >> to the link > > > > > > > >>order of the objects under drivers/gpu and the DRM driver > > > > > > > >> is registered. > > > > > > > >> > > > > > > > >>This matches the device registered in (1) and the DRM > > > > > > > >> driver probes. > > > > > > > >> > > > > > > > >> 4) The DRM driver .probe kicks out any conflicting DRM drivers > > > > > > > >> and pdev > > > > > > > >>before registering the DRM device. > > > > > > > >> > > > > > > > >>There are no conflicting drivers or platform device at this > > > > > > > >> point. > > > > > > > >> > > > > > > > >> 5) Latter at some point the drivers/firmware/sysfb.c init > > > > > > > >> function is > > > > > > > >>executed, and this registers a platform device for the > > > > > > > >> generic fb. > > > > > > > >> > > > > > > > >>This device matches the efifb driver registered in (2) and > > > > > > > >> the fbdev > > > > > > > >>driver probes. > > > > > > > >> > > > > > > > >>Since that happens *after* the DRM driver already matched, > > > > > > > >> probed > > > > > > > >>and registered the DRM device, that is a bug and what the > > > > > > > >> reverted > > > > > > > >>patch worked around. > > > > > > > >> > > > > > > > >> So we need to prevent (5) if (1) and (3) already happened. > > > > > > > >> Having a flag > > > > > > > >> set in the fbdev core somewhere when > > > > > > > >> remove_conflicting_framebuffers() > > > > > > > >> is called could be a solution indeed. > > > > > > > >> > > > > > > > >> That is, the fbdev core needs to know that a DRM driver > > > > > > > >> already probed > > > > > > > >> and make register_framebuffer() fail if info->flag & > > > > > > > >> FBINFO_MISC_FIRMWARE > > > > > > > >> > > > > > > > >> I can attempt to write a patch for that. > > > > > > > > > > > > > > > > Ah yeah that could be an issue. I think the right fix is to > > > > > > > > replace > > > > > > > > the platform dev unregister with a sysfb_unregister() function > > > > > > > > in > > > > > > > > sysfb.c, which is synced with a common lock with the sysfb_init > > > > > > > > function and a small boolean. I think I can type that up > > > > > > > > quickly for > > > > > > > > v3. > > > > > > > > > > > > > > It's more complicated than that since sysfb is just *one* of the > > > > > > > several > > > > > > > places where platform devices can be registered for video devices. > > > > > > > > > > > > > > For instance, the vga16fb driver registers its own platform > > > > > > > device in > > > > > > > its module_init() function so that can also happen after the > > > > > > > conflicting > > > > > > > framebuffers (and associated devices) were removed by a DRM > > > > > > > driver probe. > > > > > > > > > > > > > > I tried to minimize the issue for that particular driver with > > > > > > > commit: > > > > > > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0499f419b76f > > > > > > > > > > > > > > But the point stands, it all boils down to the fact that you have > > > > > > > two > > > > > > > different subsystems registering video drivers and they don't > > > > > > > know all > > > > > > > about each other to take a proper decision. > > > > > > > > > > > > > > Right now the drm_aperture_remove_conflicting_framebuffers() call > > > > > > > signals > > > > > > > in one direction from DRM to fbdev but there isn't a > > > > > > > communication in the > > > > > > > other direction, from fbdev to DRM. > > > > > > > > > > >
Re: [PATCH] drm/amdgpu: Ensure HDA function is suspended before ASIC reset
On Thu, Apr 7, 2022 at 8:21 AM Kai-Heng Feng
wrote:
>
> DP/HDMI audio on AMD PRO VII stops working after S3:
> [ 149.450391] amdgpu :63:00.0: amdgpu: MODE1 reset
> [ 149.450395] amdgpu :63:00.0: amdgpu: GPU mode1 reset
> [ 149.450494] amdgpu :63:00.0: amdgpu: GPU psp mode1 reset
> [ 149.983693] snd_hda_intel :63:00.1: refused to change power state from
> D0 to D3hot
> [ 150.003439] amdgpu :63:00.0: refused to change power state from D0 to
> D3hot
> ...
> [ 155.432975] snd_hda_intel :63:00.1: CORB reset timeout#2, CORBRP =
> 65535
As an aside, shouldn't device links order this properly already? I
thought that was the whole point of them. We have quirks in PCI
quirks.c to create device links for all GPU integrated peripherals
(audio, usb, ucsi).
Alex
>
> The offending commit is daf8de0874ab5b ("drm/amdgpu: always reset the asic in
> suspend (v2)"). Commit 34452ac3038a7 ("drm/amdgpu: don't use BACO for
> reset in S3 ") doesn't help, so the issue is something different.
>
> Assuming that to make HDA resume to D0 fully realized, it needs to be
> successfully put to D3 first. And this guesswork proves working, by
> moving amdgpu_asic_reset() to noirq callback, so it's called after HDA
> function is in D3.
>
> Fixes: daf8de0874ab5b ("drm/amdgpu: always reset the asic in suspend (v2)")
> Signed-off-by: Kai-Heng Feng
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index bb1c025d90019..31f7229e7ea89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2323,18 +2323,23 @@ static int amdgpu_pmops_suspend(struct device *dev)
> {
> struct drm_device *drm_dev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(drm_dev);
> - int r;
>
> if (amdgpu_acpi_is_s0ix_active(adev))
> adev->in_s0ix = true;
> else
> adev->in_s3 = true;
> - r = amdgpu_device_suspend(drm_dev, true);
> - if (r)
> - return r;
> + return amdgpu_device_suspend(drm_dev, true);
> +}
> +
> +static int amdgpu_pmops_suspend_noirq(struct device *dev)
> +{
> + struct drm_device *drm_dev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +
> if (!adev->in_s0ix)
> - r = amdgpu_asic_reset(adev);
> - return r;
> + return amdgpu_asic_reset(adev);
> +
> + return 0;
> }
>
> static int amdgpu_pmops_resume(struct device *dev)
> @@ -2575,6 +2580,7 @@ static const struct dev_pm_ops amdgpu_pm_ops = {
> .prepare = amdgpu_pmops_prepare,
> .complete = amdgpu_pmops_complete,
> .suspend = amdgpu_pmops_suspend,
> + .suspend_noirq = amdgpu_pmops_suspend_noirq,
> .resume = amdgpu_pmops_resume,
> .freeze = amdgpu_pmops_freeze,
> .thaw = amdgpu_pmops_thaw,
> --
> 2.34.1
>
Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi, On Thu, Apr 7, 2022 at 7:19 AM Sankeerth Billakanti (QUIC) wrote: > > Hi Dmitry and Doug, > > > Hi, > > > > On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov > > wrote: > > > > > > On 05/04/2022 20:02, Doug Anderson wrote: > > > > Hi, > > > > > > > > On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov > > > > wrote: > > > >>> 3. For DP and eDP HPD means something a little different. > > > >>> Essentially there are two concepts: a) is a display physically > > > >>> connected and b) is the display powered up and ready. For DP, the > > > >>> two are really tied together. From the kernel's point of view you > > > >>> never "power down" a DP display and you can't detect that it's > > > >>> physically connected until it's ready. Said another way, on you > > > >>> tie "is a display there" to the HPD line and the moment a display > > > >>> is there it's ready for you to do AUX transfers. For eDP, in the > > > >>> lowest power state of a display it _won't_ assert its "HPD" > > > >>> signal. However, it's still physically present. For eDP you simply > > > >>> have to _assume_ it's present without any actual proof since you > > > >>> can't get proof until you power it up. Thus for eDP, you report > > > >>> that the display is there as soon as we're asked. We can't _talk_ > > > >>> to the display yet, though. So in get_modes() we need to be able > > > >>> to power the display on enough to talk over the AUX channel to it. > > > >>> As part of this, we wait for the signal named "HPD" which really means > > "panel finished powering on" in this context. > > > >>> > > > >>> NOTE: for aux transfer, we don't have the _display_ pipe and > > > >>> clocks running. We only have enough stuff running to do the AUX > > transfer. > > > >>> We're not clocking out pixels. We haven't fully powered on the > > > >>> display. The AUX transfer is designed to be something that can be > > > >>> done early _before_ you turn on the display. > > > >>> > > > >>> > > > >>> OK, so basically that was a longwinded way of saying: yes, we > > > >>> could avoid the AUX transfer in probe, but we can't wait all the > > > >>> way to enable. We have to be able to transfer in get_modes(). If > > > >>> you think that's helpful I think it'd be a pretty easy patch to > > > >>> write even if it would look a tad bit awkward IMO. Let me know if > > > >>> you want me to post it up. > > > >> > > > >> I think it would be a good idea. At least it will allow us to > > > >> judge, which is the more correct way. > > > > > > > > I'm still happy to prototype this, but the more I think about it the > > > > more it feels like a workaround for the Qualcomm driver. The eDP > > > > panel driver is actually given a pointer to the AUX bus at probe > > > > time. It's really weird to say that we can't do a transfer on it > > > > yet... As you said, this is a little sideband bus. It should be able > > > > to be used without all the full blown infra of the rest of the driver. > > > > > > Yes, I have that feeling too. However I also have a feeling that just > > > powering up the PHY before the bus probe is ... a hack. There are no > > > obvious stopgaps for the driver not to power it down later. > > > > This is why I think we need to move to Runtime PM to manage this. Basically: > > > > 1. When an AUX transfer happens, you grab a PM runtime reference that > > _that_ powers up the PHY. > > > > 2. At the end of the AUX transfer function, you do a "put_autosuspend". > > > > Then it becomes not a hack, right? > > > > > > pm runtime ops needs to be implemented for both eDP and DP. This change > take good amount of planning and code changes as it affects DP also. > > Because this patch series consist of basic eDP changes for SC7280 bootup, > shall we take this pm_runtime implementation in subsequent patch series? Dmitry is the real decision maker here, but in my opinion it would be OK to get something landed first that worked OK and wasn't taking us too far in the wrong direction and then we could get a follow up patch to move to pm_runtime.
Re: [PATCH 1/2] drm/i915: fix broken build
On 07/04/2022 17:49, Christian König wrote:
Am 07.04.22 um 18:45 schrieb Matthew Auld:
I guess this was missed in the conversion or something.
Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
Signed-off-by: Matthew Auld
Cc: Christian König
Cc: Daniel Vetter
My best guess is that this is a rebase/merge conflict. I'm 100% sure
i915 was compiling fine before I pushed the patch.
That was my thinking also, but building drm-misc-next I get the same error:
drivers/gpu/drm/i915/i915_deps.c: In function ‘i915_deps_add_resv’:
drivers/gpu/drm/i915/i915_deps.c:229:46: error: implicit conversion from
‘enum ’ to ‘enum dma_resv_usage’ [-Werror=enum-conversion]
229 | dma_resv_for_each_fence(&iter, resv, true, fence) {
| ^~~~
./include/linux/dma-resv.h:297:47: note: in definition of macro
‘dma_resv_for_each_fence’
297 | for (dma_resv_iter_begin(cursor, obj, usage), \
| ^
Anyway Reviewed-by: Christian König for the
series.
Thanks.
Thanks,
Christian.
---
drivers/gpu/drm/i915/i915_deps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_deps.c
b/drivers/gpu/drm/i915/i915_deps.c
index 999210b37325..297b8e4e42ee 100644
--- a/drivers/gpu/drm/i915/i915_deps.c
+++ b/drivers/gpu/drm/i915/i915_deps.c
@@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps,
struct dma_resv *resv,
struct dma_fence *fence;
dma_resv_assert_held(resv);
- dma_resv_for_each_fence(&iter, resv, true, fence) {
+ dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true),
fence) {
int ret = i915_deps_add_dependency(deps, fence, ctx);
if (ret)
Re: [PATCH 2/2] drm/i915/buddy: sanity check the size
|Reviewed-by: Nirmoy Das |
On 4/7/2022 1:06 PM, Matthew Auld wrote:
Ensure we check that the size is compatible with the requested
page_size. For tiny objects that are automatically annotated with
TTM_PL_FLAG_CONTIGUOUS(since they fit within a single page), we
currently end up silently overriding the min_page_size, which ends up
hiding bugs elsewhere.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Nirmoy Das
---
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 8e4e3f72c1ef..a5109548abc0 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -70,6 +70,7 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
min_page_size = bo->page_alignment << PAGE_SHIFT;
GEM_BUG_ON(min_page_size < mm->chunk_size);
+ GEM_BUG_ON(!IS_ALIGNED(size, min_page_size));
if (place->fpfn + bman_res->base.num_pages != place->lpfn &&
place->flags & TTM_PL_FLAG_CONTIGUOUS) {
Re: [PATCH v0 02/10] dt-bindings: display: imx: add binding for i.MX8MP HDMI TX
On Thu, Apr 07, 2022 at 11:15:26AM +0200, Lucas Stach wrote: > Am Mittwoch, dem 06.04.2022 um 15:08 -0500 schrieb Rob Herring: > > On Wed, 06 Apr 2022 18:01:15 +0200, Lucas Stach wrote: > > > The HDMI TX controller on the i.MX8MP SoC is a Synopsys designware IP > > > core with a little bit of SoC integration around it. > > > > > > Signed-off-by: Lucas Stach > > > --- > > > .../bindings/display/imx/fsl,imx8mp-hdmi.yaml | 72 +++ > > > 1 file changed, 72 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi.yaml > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > Error: > > Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi.example.dts:36.45-46 > > syntax error > > FATAL ERROR: Unable to parse input tree > > make[1]: *** [scripts/Makefile.lib:364: > > Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi.example.dtb] > > Error 1 > > make[1]: *** Waiting for unfinished jobs > > make: *** [Makefile:1401: dt_binding_check] Error 2 > > > > doc reference errors (make refcheckdocs): > > > > See https://patchwork.ozlabs.org/patch/ > > > > This check can fail if there are any dependencies. The base for a patch > > series is generally the most recent rc1. > > > Those failures are caused by the example referencing the power domain > defines, that are only added in a dependency of this series. They build > fine with all the dependencies applied, so please don't let this bot > failure prevent you from looking at the actual bindings. I review the failures. It otherwise looks fine. Rob
Re: [PATCH v8, 16/17] media: mediatek: vcodec: support stateless VP9 decoding
Le jeudi 31 mars 2022 à 10:48 +0800, Yunfei Dong a écrit :
> Add support for VP9 decoding using the stateless API,
> as supported by MT8192. And the drivers is lat and core architecture.
>
> Signed-off-by: George Sun
> Signed-off-by: Xiaoyong Lu
> Signed-off-by: Yunfei Dong
> Reviewed-by: AngeloGioacchino Del Regno
>
> ---
> changed compare with v7:
> Using upstream interface to update vp9 prob tables.
> ---
> .../media/platform/mediatek/vcodec/Makefile |1 +
> .../vcodec/mtk_vcodec_dec_stateless.c | 26 +-
> .../platform/mediatek/vcodec/mtk_vcodec_drv.h |1 +
> .../vcodec/vdec/vdec_vp9_req_lat_if.c | 2072 +
> .../platform/mediatek/vcodec/vdec_drv_if.c|4 +
> .../platform/mediatek/vcodec/vdec_drv_if.h|1 +
> 6 files changed, 2102 insertions(+), 3 deletions(-)
> create mode 100644
> drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
>
> diff --git a/drivers/media/platform/mediatek/vcodec/Makefile
> b/drivers/media/platform/mediatek/vcodec/Makefile
> index b457daf2d196..93e7a343b5b0 100644
> --- a/drivers/media/platform/mediatek/vcodec/Makefile
> +++ b/drivers/media/platform/mediatek/vcodec/Makefile
> @@ -9,6 +9,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \
> vdec/vdec_vp8_if.o \
> vdec/vdec_vp8_req_if.o \
> vdec/vdec_vp9_if.o \
> + vdec/vdec_vp9_req_lat_if.o \
> vdec/vdec_h264_req_if.o \
> vdec/vdec_h264_req_common.o \
> vdec/vdec_h264_req_multi_if.o \
> diff --git
> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> index 3208f834ff80..a4735e67d39e 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
> @@ -91,13 +91,28 @@ static const struct mtk_stateless_control
> mtk_stateless_controls[] = {
> .max = V4L2_MPEG_VIDEO_VP8_PROFILE_3,
> },
> .codec_type = V4L2_PIX_FMT_VP8_FRAME,
> - }
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_STATELESS_VP9_FRAME,
> + },
> + .codec_type = V4L2_PIX_FMT_VP9_FRAME,
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
> + .min = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> + .def = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> + .max = V4L2_MPEG_VIDEO_VP9_PROFILE_3,
> + },
> + .codec_type = V4L2_PIX_FMT_VP9_FRAME,
> + },
> };
>
> #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
>
> -static struct mtk_video_fmt mtk_video_formats[4];
> -static struct mtk_codec_framesizes mtk_vdec_framesizes[2];
> +static struct mtk_video_fmt mtk_video_formats[5];
> +static struct mtk_codec_framesizes mtk_vdec_framesizes[3];
>
> static struct mtk_video_fmt default_out_format;
> static struct mtk_video_fmt default_cap_format;
> @@ -338,6 +353,7 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
> switch (fourcc) {
> case V4L2_PIX_FMT_H264_SLICE:
> case V4L2_PIX_FMT_VP8_FRAME:
> + case V4L2_PIX_FMT_VP9_FRAME:
> mtk_video_formats[count_formats].fourcc = fourcc;
> mtk_video_formats[count_formats].type = MTK_FMT_DEC;
> mtk_video_formats[count_formats].num_planes = 1;
> @@ -385,6 +401,10 @@ static void mtk_vcodec_get_supported_formats(struct
> mtk_vcodec_ctx *ctx)
> mtk_vcodec_add_formats(V4L2_PIX_FMT_VP8_FRAME, ctx);
> out_format_count++;
> }
> + if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_VP9_FRAME) {
> + mtk_vcodec_add_formats(V4L2_PIX_FMT_VP9_FRAME, ctx);
> + out_format_count++;
> + }
>
> if (cap_format_count)
> default_cap_format = mtk_video_formats[cap_format_count - 1];
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> index 2ba1c19f07b6..a29041a0b7e0 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> @@ -355,6 +355,7 @@ enum mtk_vdec_format_types {
> MTK_VDEC_FORMAT_MT21C = 0x40,
> MTK_VDEC_FORMAT_H264_SLICE = 0x100,
> MTK_VDEC_FORMAT_VP8_FRAME = 0x200,
> + MTK_VDEC_FORMAT_VP9_FRAME = 0x400,
> };
>
> /**
> diff --git
> a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> new file mode 100644
> index ..d63399085b9b
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> @@ -0,0 +1,2072 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021 MediaTek Inc.
> + * Author: George
[PATCH] drm/msm/dp: add fail safe mode outside of event_mutex context
There is possible circular locking dependency detected on event_mutex.
To break this possible circular locking, this patch move setting fail
safe mode out of event_mutex scope.
Fixes: d4aca422539c ("drm/msm/dp: always add fail-safe mode into connector
mode list")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 6 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 20 ++--
drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
3 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 178b774..a42732b 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -580,6 +580,12 @@ static int dp_hpd_plug_handle(struct dp_display_private
*dp, u32 data)
dp->dp_display.connector_type, state);
mutex_unlock(&dp->event_mutex);
+ /*
+* add fail safe mode outside event_mutex scope
+* to avoid potiential circular lock with drm thread
+*/
+ dp_panel_add_fail_safe_mode(dp->dp_display.connector);
+
/* uevent will complete connection part */
return 0;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index f141872..26c3653 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -151,6 +151,15 @@ static int dp_panel_update_modes(struct drm_connector
*connector,
return rc;
}
+void dp_panel_add_fail_safe_mode(struct drm_connector *connector)
+{
+ /* fail safe edid */
+ mutex_lock(&connector->dev->mode_config.mutex);
+ if (drm_add_modes_noedid(connector, 640, 480))
+ drm_set_preferred_mode(connector, 640, 480);
+ mutex_unlock(&connector->dev->mode_config.mutex);
+}
+
int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
struct drm_connector *connector)
{
@@ -207,16 +216,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
goto end;
}
- /* fail safe edid */
- mutex_lock(&connector->dev->mode_config.mutex);
- if (drm_add_modes_noedid(connector, 640, 480))
- drm_set_preferred_mode(connector, 640, 480);
- mutex_unlock(&connector->dev->mode_config.mutex);
- } else {
- /* always add fail-safe mode as backup mode */
- mutex_lock(&connector->dev->mode_config.mutex);
- drm_add_modes_noedid(connector, 640, 480);
- mutex_unlock(&connector->dev->mode_config.mutex);
+ dp_panel_add_fail_safe_mode(connector);
}
if (panel->aux_cfg_update_done) {
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9023e5b..99739ea 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -59,6 +59,7 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel);
int dp_panel_deinit(struct dp_panel *dp_panel);
int dp_panel_timing_cfg(struct dp_panel *dp_panel);
void dp_panel_dump_regs(struct dp_panel *dp_panel);
+void dp_panel_add_fail_safe_mode(struct drm_connector *connector);
int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
struct drm_connector *connector);
u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel, u32 mode_max_bpp,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [RFC] drm/kms: control display brightness through drm_connector properties
Very nice plan! Big +1 for the overall approach. On Thursday, April 7th, 2022 at 17:38, Hans de Goede wrote: > The drm_connector brightness properties > === > > bl_brightness: rw 0-int32_max property controlling the brightness setting > of the connected display. The actual maximum of this will be less then > int32_max and is given in bl_brightness_max. Do we need to split this up into two props for sw/hw state? The privacy screen stuff needed this, but you're pretty familiar with that. :) > bl_brightness_max: ro 0-int32_max property giving the actual maximum > of the display's brightness setting. This will report 0 when brightness > control is not available (yet). I don't think we actually need that one. Integer KMS props all have a range which can be fetched via drmModeGetProperty. The max can be exposed via this range. Example with the existing alpha prop: "alpha": range [0, UINT16_MAX] = 65535 > bl_brightness_0_is_min_brightness: ro, boolean > When this is set to true then it is safe to set brightness to 0 > without worrying that this completely turns the backlight off causing > the screen to become unreadable. When this is false setting brightness > to 0 may turn the backlight off, but this is not guaranteed. > This will e.g. be true when directly driving a PWM and the video-BIOS > has provided a minimum (non 0) duty-cycle below which the driver will > never go. Hm. It's quite unfortunate that it's impossible to have strong guarantees here. Is there any way we can avoid this prop? For instance if we can guarantee that the min level won't turn the screen completely off we could make the range start from 1 instead of 0. Or allow -1 to mean "minimum value, maybe completely off". > bl_brightness_control_method: ro, enum, possible values: > none: The GPU driver expects brightness control to be provided by another > driver and that driver has not loaded yet. > unknown: The underlying control mechanism is unknown. > pwm: The brightness property directly controls the duty-cycle of a PWM > output. > firmware: The brightness is controlled through firmware calls. > DDC/CI: The brightness is controlled through the DDC/CI protocol. > gmux: The brightness is controlled by the GMUX. > Note this enum may be extended in the future, so other values may > be read, these should be treated as "unknown". > > When brightness control becomes available after being reported > as not available before (bl_brightness_control_method=="none") > a uevent with CONNECTOR= and > > PROPERTY= will be generated > > at this point all the properties must be re-read. > > When/once brightness control is available then all the read-only > properties are fixed and will never change. > > Besides the "none" value for no driver having loaded yet, > the different bl_brightness_control_method values are intended for > (userspace) heuristics for such things as the brightness setting > linearly controlling electrical power or setting perceived brightness. Can you elaborate? I don't know enough about brightness control to understand all of the implications here.
Re: [PATCH 1/2] drm/i915: fix broken build
Am 07.04.22 um 18:45 schrieb Matthew Auld:
I guess this was missed in the conversion or something.
Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
Signed-off-by: Matthew Auld
Cc: Christian König
Cc: Daniel Vetter
My best guess is that this is a rebase/merge conflict. I'm 100% sure
i915 was compiling fine before I pushed the patch.
Anyway Reviewed-by: Christian König for the
series.
Thanks,
Christian.
---
drivers/gpu/drm/i915/i915_deps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_deps.c b/drivers/gpu/drm/i915/i915_deps.c
index 999210b37325..297b8e4e42ee 100644
--- a/drivers/gpu/drm/i915/i915_deps.c
+++ b/drivers/gpu/drm/i915/i915_deps.c
@@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, struct
dma_resv *resv,
struct dma_fence *fence;
dma_resv_assert_held(resv);
- dma_resv_for_each_fence(&iter, resv, true, fence) {
+ dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), fence) {
int ret = i915_deps_add_dependency(deps, fence, ctx);
if (ret)
[PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence
All of CI is just failing with the following, which prevents loading of
the module:
i915 :03:00.0: [drm] *ERROR* Scratch setup failed
Best guess is that this comes from the pin_map() for the scratch page,
which does an i915_gem_object_wait_moving_fence() somewhere. It looks
like this now calls into dma_resv_wait_timeout() which can return the
remaining timeout, leading to the caller thinking this is an error.
Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
Signed-off-by: Matthew Auld
Cc: Christian König
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 2998d895a6b3..1c88d4121658 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct
drm_i915_gem_object *obj,
int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
bool intr)
{
+ long ret;
+
assert_object_held(obj);
- return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
-intr, MAX_SCHEDULE_TIMEOUT);
+
+ ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
+ intr, MAX_SCHEDULE_TIMEOUT);
+
+ return ret < 0 ? ret : 0;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
--
2.34.1
[PATCH 1/2] drm/i915: fix broken build
I guess this was missed in the conversion or something.
Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
Signed-off-by: Matthew Auld
Cc: Christian König
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/i915_deps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_deps.c b/drivers/gpu/drm/i915/i915_deps.c
index 999210b37325..297b8e4e42ee 100644
--- a/drivers/gpu/drm/i915/i915_deps.c
+++ b/drivers/gpu/drm/i915/i915_deps.c
@@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, struct
dma_resv *resv,
struct dma_fence *fence;
dma_resv_assert_held(resv);
- dma_resv_for_each_fence(&iter, resv, true, fence) {
+ dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), fence) {
int ret = i915_deps_add_dependency(deps, fence, ctx);
if (ret)
--
2.34.1
Re: [PATCH v2] drm/hyperv: Added error message for fb size greater then allocated
On Wed, Apr 6, 2022 at 11:27 PM Saurabh Sengar
wrote:
>
> Added error message when the size of requested framebuffer is more then
> the allocated size by vmbus mmio region for framebuffer
>
> Signed-off-by: Saurabh Sengar
> ---
> v1 -> v2 : Corrected Sign-off
>
> drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> index e82b815..92587f0 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> @@ -123,8 +123,11 @@ static int hyperv_pipe_check(struct
> drm_simple_display_pipe *pipe,
> if (fb->format->format != DRM_FORMAT_XRGB)
> return -EINVAL;
>
> - if (fb->pitches[0] * fb->height > hv->fb_size)
> + if (fb->pitches[0] * fb->height > hv->fb_size) {
> + drm_err(&hv->dev, "hv->hdev, fb size requested by process %s
> for %d X %d (pitch %d) is greater then allocated size %ld\n",
> + current->comm, fb->width, fb->height, fb->pitches[0],
> hv->fb_size);
Any reason to add an error message here. Since this function is called
whenever there is an update, avoid printing an error here.
> return -EINVAL;
> + }
>
> return 0;
> }
> --
> 1.8.3.1
>
[PATCH] drm/i915: Sunset igpu legacy mmap support based on GRAPHICS_VER_FULL
The intent of the version check in the mmap ioctl was to maintain
support for existing platforms (i.e., ADL/RPL and earlier), but drop
support on all future igpu platforms. As we've seen on the dgpu side,
the hardware teams are using a more fine-grained numbering system for IP
version numbers these days, so it's possible the version number
associated with our next igpu could be some form of "12.xx" rather than
13 or higher. Comparing against the full ver.release number will ensure
the intent of the check is maintained no matter what numbering the
hardware teams settle on.
Fixes: d3f3baa3562a ("drm/i915: Reinstate the mmap ioctl for some platforms")
Cc: Thomas Hellström
Cc: Lucas De Marchi
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c3ea243d414d..0c5c43852e24 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -70,7 +70,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
* mmap ioctl is disallowed for all discrete platforms,
* and for all platforms with GRAPHICS_VER > 12.
*/
- if (IS_DGFX(i915) || GRAPHICS_VER(i915) > 12)
+ if (IS_DGFX(i915) || GRAPHICS_VER_FULL(i915) > IP_VER(12, 0))
return -EOPNOTSUPP;
if (args->flags & ~(I915_MMAP_WC))
--
2.34.1
Re: [PATCH] drm/amd/display: Fix indenting mistakes in dcn10_hw_sequencer.c
Actually this just causes another warning. Dropped for now. More below.
On Thu, Apr 7, 2022 at 11:52 AM Alex Deucher wrote:
>
> Applied. Thanks!
>
> Alex
>
> On Thu, Apr 7, 2022 at 10:18 AM Harry Wentland wrote:
> >
> >
> >
> > On 2022-04-07 02:00, Haowen Bai wrote:
> > > Smatch reports the following:
> > > drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2174
> > > dcn10_enable_vblanks_synchronization() warn: if statement not indented
> > >
> > > Signed-off-by: Haowen Bai
> >
> > Reviewed-by: Harry Wentland
> >
> > Harry
> >
> > > ---
> > > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 14
> > > +++---
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > > index ee22f4422d26..3c338b85040c 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > > @@ -2172,13 +2172,13 @@ void dcn10_enable_vblanks_synchronization(
> > > if (master >= 0) {
> > > for (i = 0; i < group_size; i++) {
> > > if (i != master &&
> > > !grouped_pipes[i]->stream->has_non_synchronizable_pclk)
> > > -
> > > grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> > > - grouped_pipes[master]->stream_res.tg,
> > > - grouped_pipes[i]->stream_res.tg,
> > > -
> > > grouped_pipes[master]->stream->timing.pix_clk_100hz,
> > > -
> > > grouped_pipes[i]->stream->timing.pix_clk_100hz,
> > > - get_clock_divider(grouped_pipes[master],
> > > false),
> > > - get_clock_divider(grouped_pipes[i], false));
> > > +
> > > grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> > > +
> > > grouped_pipes[master]->stream_res.tg,
> > > + grouped_pipes[i]->stream_res.tg,
> > > +
> > > grouped_pipes[master]->stream->timing.pix_clk_100hz,
> > > +
> > > grouped_pipes[i]->stream->timing.pix_clk_100hz,
> > > +
> > > get_clock_divider(grouped_pipes[master], false),
> > > + get_clock_divider(grouped_pipes[i],
> > > false));
> > >
> > > grouped_pipes[i]->stream->vblank_synchronized = true;
@Harry Wentland should this last statement be part of the if clause or
the for loop?
Alex
> > > }
> > > grouped_pipes[master]->stream->vblank_synchronized = true;
> >
Re: [PATCH] drm/amdgpu: Ensure HDA function is suspended before ASIC reset
Applied. Thanks!
Alex
On Thu, Apr 7, 2022 at 8:21 AM Kai-Heng Feng
wrote:
>
> DP/HDMI audio on AMD PRO VII stops working after S3:
> [ 149.450391] amdgpu :63:00.0: amdgpu: MODE1 reset
> [ 149.450395] amdgpu :63:00.0: amdgpu: GPU mode1 reset
> [ 149.450494] amdgpu :63:00.0: amdgpu: GPU psp mode1 reset
> [ 149.983693] snd_hda_intel :63:00.1: refused to change power state from
> D0 to D3hot
> [ 150.003439] amdgpu :63:00.0: refused to change power state from D0 to
> D3hot
> ...
> [ 155.432975] snd_hda_intel :63:00.1: CORB reset timeout#2, CORBRP =
> 65535
>
> The offending commit is daf8de0874ab5b ("drm/amdgpu: always reset the asic in
> suspend (v2)"). Commit 34452ac3038a7 ("drm/amdgpu: don't use BACO for
> reset in S3 ") doesn't help, so the issue is something different.
>
> Assuming that to make HDA resume to D0 fully realized, it needs to be
> successfully put to D3 first. And this guesswork proves working, by
> moving amdgpu_asic_reset() to noirq callback, so it's called after HDA
> function is in D3.
>
> Fixes: daf8de0874ab5b ("drm/amdgpu: always reset the asic in suspend (v2)")
> Signed-off-by: Kai-Heng Feng
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index bb1c025d90019..31f7229e7ea89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2323,18 +2323,23 @@ static int amdgpu_pmops_suspend(struct device *dev)
> {
> struct drm_device *drm_dev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(drm_dev);
> - int r;
>
> if (amdgpu_acpi_is_s0ix_active(adev))
> adev->in_s0ix = true;
> else
> adev->in_s3 = true;
> - r = amdgpu_device_suspend(drm_dev, true);
> - if (r)
> - return r;
> + return amdgpu_device_suspend(drm_dev, true);
> +}
> +
> +static int amdgpu_pmops_suspend_noirq(struct device *dev)
> +{
> + struct drm_device *drm_dev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +
> if (!adev->in_s0ix)
> - r = amdgpu_asic_reset(adev);
> - return r;
> + return amdgpu_asic_reset(adev);
> +
> + return 0;
> }
>
> static int amdgpu_pmops_resume(struct device *dev)
> @@ -2575,6 +2580,7 @@ static const struct dev_pm_ops amdgpu_pm_ops = {
> .prepare = amdgpu_pmops_prepare,
> .complete = amdgpu_pmops_complete,
> .suspend = amdgpu_pmops_suspend,
> + .suspend_noirq = amdgpu_pmops_suspend_noirq,
> .resume = amdgpu_pmops_resume,
> .freeze = amdgpu_pmops_freeze,
> .thaw = amdgpu_pmops_thaw,
> --
> 2.34.1
>
Re: [PATCH] drm/amd/display: Fix indenting mistakes in dcn10_hw_sequencer.c
Applied. Thanks!
Alex
On Thu, Apr 7, 2022 at 10:18 AM Harry Wentland wrote:
>
>
>
> On 2022-04-07 02:00, Haowen Bai wrote:
> > Smatch reports the following:
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2174
> > dcn10_enable_vblanks_synchronization() warn: if statement not indented
> >
> > Signed-off-by: Haowen Bai
>
> Reviewed-by: Harry Wentland
>
> Harry
>
> > ---
> > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 14
> > +++---
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > index ee22f4422d26..3c338b85040c 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > @@ -2172,13 +2172,13 @@ void dcn10_enable_vblanks_synchronization(
> > if (master >= 0) {
> > for (i = 0; i < group_size; i++) {
> > if (i != master &&
> > !grouped_pipes[i]->stream->has_non_synchronizable_pclk)
> > - grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> > - grouped_pipes[master]->stream_res.tg,
> > - grouped_pipes[i]->stream_res.tg,
> > -
> > grouped_pipes[master]->stream->timing.pix_clk_100hz,
> > -
> > grouped_pipes[i]->stream->timing.pix_clk_100hz,
> > - get_clock_divider(grouped_pipes[master],
> > false),
> > - get_clock_divider(grouped_pipes[i], false));
> > +
> > grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> > + grouped_pipes[master]->stream_res.tg,
> > + grouped_pipes[i]->stream_res.tg,
> > +
> > grouped_pipes[master]->stream->timing.pix_clk_100hz,
> > +
> > grouped_pipes[i]->stream->timing.pix_clk_100hz,
> > +
> > get_clock_divider(grouped_pipes[master], false),
> > + get_clock_divider(grouped_pipes[i],
> > false));
> > grouped_pipes[i]->stream->vblank_synchronized
> > = true;
> > }
> > grouped_pipes[master]->stream->vblank_synchronized = true;
>
Re: [BUG] fbdev: i740fb: Divide error when ‘var->pixclock’ is zero
On 4/6/22 03:24, Zheyu Ma wrote:
> On Wed, Apr 6, 2022 at 2:23 AM Helge Deller wrote:
>>
>> On 4/5/22 19:46, Ondrej Zary wrote:
>>> On Tuesday 05 April 2022 08:33:57 Helge Deller wrote:
Hello Geert,
On 4/4/22 13:46, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Sun, Apr 3, 2022 at 5:41 PM Helge Deller wrote:
>> On 4/3/22 13:26, Zheyu Ma wrote:
>>> I found a bug in the function i740fb_set_par().
>>
>> Nice catch!
>>
>>> When the user calls the ioctl system call without setting the value to
>>> 'var->pixclock', the driver will throw a divide error.
>>>
>>> This bug occurs because the driver uses the value of 'var->pixclock'
>>> without checking it, as the following code snippet show:
>>>
>>> if ((100 / var->pixclock) > DACSPEED8) {
>>> dev_err(info->device, "requested pixclock %i MHz out of range
>>> (max. %i MHz at 8bpp)\n",
>>> 100 / var->pixclock, DACSPEED8);
>>> return -EINVAL;x
>>> }
>>>
>>> We can fix this by checking the value of 'var->pixclock' in the
>>> function i740fb_check_var() similar to commit
>>> b36b242d4b8ea178f7fd038965e3cac7f30c3f09, or we should set the lowest
>>> supported value when this field is zero.
>>> I have no idea about which solution is better.
>>
>> Me neither.
>> I think a solution like commit b36b242d4b8ea178f7fd038965e3cac7f30c3f09
>> is sufficient.
>>
>> Note that i740fb_set_par() is called in i740fb_resume() as well.
>> Since this doesn't comes form userspace I think adding a check for
>> the return value there isn't necessary.
>>
>> Would you mind sending a patch like
>> b36b242d4b8ea178f7fd038965e3cac7f30c3f09 ?
>
> When passed an invalid value, .check_var() is supposed to
> round up the invalid to a valid value, if possible.
I don't disagree.
The main problem probably is: what is the next valid value?
This needs to be analyzed on a per-driver base and ideally tested.
Right now a division-by-zero is tiggered which is probably more worse.
>>>
>>> I still have an i740 card so I can test it.
>>
>> Good. Someone wants to come up with a proposed patch?
>
> I have submitted patches for the i740fb driver and other drivers which
> have similar bugs as follows:
> https://lore.kernel.org/all/20220404084723.79089-1-zheyum...@gmail.com/
Yes, I know.
But Ondrej offered to test a patch which would round an invalid pixclock up
instead of just returning EINVAL (which is what your patch does).
So, if someone comes up with such a patch it'd be the preferred solution.
Helge
Re: [Intel-gfx] [PATCH 1/1] drm/i915/uc: Use platform specific defaults for GuC/HuC enabling
On 03/06/2021 17:48, Matthew Brost wrote: From: John Harrison The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms. Signed-off-by: John Harrison Signed-off-by: Matthew Brost Reviewed-by: Daniele Ceraolo Spurio --- drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. " - "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)"); + "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)"); i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. " diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \ - param(int, enable_guc, 0, 0400) \ + param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \ What is the BKM to use this with multi-GPU setups? Specifically I have a TGL+DG1 laptop (off the shelf) and want to have GuC with DG1 only. If I pass i915.enable_guc=3 it seems it wants to enable it for TGL as well and wedges the GPU if it can't? Regards, Tvrtko
[Bug 211807] [drm:drm_dp_mst_dpcd_read] *ERROR* mstb 000000004e6288dd port 3: DPCD read on addr 0x60 for 1 bytes NAKed
https://bugzilla.kernel.org/show_bug.cgi?id=211807 Fiona Buckner (pheonix...@gmail.com) changed: What|Removed |Added CC||pheonix...@gmail.com --- Comment #21 from Fiona Buckner (pheonix...@gmail.com) --- Seeing this on Ubuntu 22.04 pre-release. Ubuntu Kernel 5.15.0-23-generic. > # dmesg | grep i915 > [1.776895] i915 :00:02.0: [drm] VT-d active for gfx access > [1.776899] fb0: switching to i915 from EFI VGA > [1.776936] i915 :00:02.0: vgaarb: deactivate vga console > [1.780003] i915 :00:02.0: vgaarb: changed VGA decodes: > olddecodes=io+mem,decodes=io+mem:owns=io+mem > [1.780279] i915 :00:02.0: [drm] Finished loading DMC firmware > i915/kbl_dmc_ver1_04.bin (v1.4) > [2.675796] i915 :00:02.0: [drm] [ENCODER:113:DDI C/PHY C] is > disabled/in DSI mode with an ungated DDI clock, gate it > [2.681117] i915 :00:02.0: [drm] [ENCODER:120:DDI D/PHY D] is > disabled/in DSI mode with an ungated DDI clock, gate it > [3.015399] [drm] Initialized i915 1.6.0 20201103 for :00:02.0 on > minor 0 > [3.068397] fbcon: i915drmfb (fb0) is primary device > [3.427857] i915 :00:02.0: [drm] fb0: i915drmfb frame buffer device > [ 25.814835] snd_hda_intel :00:1f.3: bound :00:02.0 (ops > i915_audio_component_bind_ops [i915]) > [ 26.218387] mei_hdcp :00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: > bound :00:02.0 (ops i915_hdcp_component_ops [i915]) > [ 52.090727] mei_hdcp :00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: > bound :00:02.0 (ops i915_hdcp_component_ops [i915]) > [ 84.800055] i915 :00:02.0: [drm] *ERROR* mstb 0a940675 port 2: > DPCD read on addr 0x4b0 for 1 bytes NAKed > [ 84.811156] i915 :00:02.0: [drm] *ERROR* mstb 0a940675 port 3: > DPCD read on addr 0x4b0 for 1 bytes NAKed > [ 4814.933704] audit: type=1400 audit(1649342645.891:72): apparmor="DENIED" > operation="open" profile="snap.chromium.chromium" > name="/proc/sys/dev/i915/perf_stream_paranoid" pid=44648 comm="chrome" > requested_mask="r" denied_mask="r" fsuid=1000 ouid=0 > [ 7139.378349] i915 :00:02.0: [drm] *ERROR* mstb 0a940675 port 2: > DPCD read on addr 0x4b0 for 1 bytes NAKed > [ 7139.388678] i915 :00:02.0: [drm] *ERROR* mstb 0a940675 port 3: > DPCD read on addr 0x4b0 for 1 bytes NAKed Dell CPU: Intel® Core™ i7-9750H CPU @ 2.60GHz × 12 GPU: Quadro T1000/PCIe/SSE2 / Quadro T1000/PCIe/SSE2 (forcing the Nvidia gpu through Nvidia Optimus because the Intel igpu isn't enough for dual 4k monitors) Dell WD19TB dock connected to the USBC Thunderbolt port. 2x HP z27 monitors. One connected to the USBC port on the dock, the other to the Display Port. Researching this error, I found this thread: https://lists.freedesktop.org/archives/dri-devel/2022-February/342776.html > This is normal (although not great TBH, I'm not sure we should be printing an > error message for that), it's the result of fwupd trying to probe the MST hub > to see if it's a specific Dell dock that can receive updates over DP aux, but > it's not smart enough to know it doesn't need to poke the DP aux ranges of > downstream branches or non-MST ports in general. > Would definitely accept patches to make this a non-error, or at least make > this a non-error when the read/writes come from userspace - Lyude Paul It might be good to speak with Lyude about it as she might be the expert on it. I hope this helps. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[RFC] drm/kms: control display brightness through drm_connector properties
As discussed already several times in the past: https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/ https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/ The current userspace API for brightness control offered by /sys/class/backlight devices has various issues, the biggest 2 being: 1. There is no way to map the backlight device to a specific display-output / panel (1) 2. Controlling the brightness requires root-rights requiring desktop-environments to use suid-root helpers for this. As already discussed on various conference's hallway tracks and as has been proposed on the dri-devel list once before (2), it seems that there is consensus that the best way to to solve these 2 issues is to add support for controlling a video-output's brightness through properties on the drm_connector. This RFC outlines my plan to try and actually implement this, which has 3 phases: Phase 1: Stop registering multiple /sys/class/backlight devs for a single display = On x86 there can be multiple firmware + direct-hw-access methods for controlling the backlight and in some cases the kernel registers multiple backlight-devices for a single internal laptop LCD panel: a) i915 and nouveau unconditionally register their "native" backlight dev even on devices where /sys/class/backlight/acpi_video0 must be used to control the backlight, relying on userspace to prefer the "firmware" acpi_video0 device over "native" devices. b) amdgpu and nouveau rely on the acpi_video driver initializing before them, which currently causes /sys/class/backlight/acpi_video0 to usually show up and then they register their own native backlight driver after which the drivers/acpi/video_detect.c code unregisters the acpi_video0 device. This means that userspace briefly sees 2 devices and the disappearing of acpi_video0 after a brief time confuses the systemd backlight level save/restore code, see e.g.: https://bbs.archlinux.org/viewtopic.php?id=269920 I already have a pretty detailed plan to tackle this, which I will post in a separate RFC email. I plan to start working on this right away, as it will be good to have this fixed regardless. Phase 2: Add drm_connector properties mirroring the matching backlight device = The plan is to add a drm_connector helper function, which optionally takes a pointer to the backlight device for the GPU's native backlight device, which will then mirror the backlight settings from the backlight device in a set of read/write brightness* properties on the connector. This function can then be called by GPU drivers for the drm_connector for the internal panel and it will then take care of everything. When there is no native GPU backlight device, or when it should not be used then (on x86) the helper will use the acpi_video_get_backlight_type() to determine which backlight-device should be used instead and it will find + mirror that one. Phase 3: Deprecate /sys/class/backlight uAPI Once most userspace has moved over to using the new drm_connector brightness props, a Kconfig option can be added to stop exporting the backlight-devices under /sys/class/backlight. The plan is to just disable the sysfs interface and keep the existing backlight-device internal kernel abstraction as is, since some abstraction for (non GPU native) backlight devices will be necessary regardless. An alternative to disabling the sysfs class entirely, would be to allow setting it to read-only through Kconfig. What scale to use for the drm_connector bl_brightness property? === The tricky part of this plan is phase 2 and then esp. defining what the new brightness properties will look like and how they will work. The biggest challenge here is to decide on a fixed scale for the main brightness property, say 0-65535, using scaling where the actual hw scale is different, or if this should simply be a 1:1 mirror of the current backlight interface, with the actual hw scale / brightness_max value exposed as a drm_connector property. 1:1 advantages / 0-65535 disadvantages - Userspace will likely move over to the connector-props quite slowly and we can expect various userspace bits, esp. also custom user scripts, to keep using the old uAPI for a long time. Using the 2 APIs are intermixed is fine when using a 1:1 brightness scale mapping. But if we end up doing a scaling round-trip all the time then eventually the brightness is going do drift. This can even happen if the user never changes the brightness when userspace saves it over suspend/resume or reboots. - Almost all laptops have brightness up/down hotkeys. E.g GNOME decides on a step size for the hotkeys by doing min(brightness_max/20, 1). Some of
[PATCH 1/1] drm/i915: Inherit submitter nice when scheduling requests
From: Tvrtko Ursulin
Inherit submitter nice at point of request submission to account for long
running processes getting either externally or self re-niced.
This accounts for the current processing landscape where computational
pipelines are composed of CPU and GPU parts working in tandem.
Nice value will only apply to requests which originate from user contexts
and have default context priority. This is to avoid disturbing any
application made choices of low and high (batch processing and latency
sensitive compositing). In this case nice value adjusts the effective
priority in the narrow band of -19 to +20 around
I915_CONTEXT_DEFAULT_PRIORITY.
This means that userspace using the context priority uapi directly has a
wider range of possible adjustments (in practice that only applies to
execlists platforms - with GuC there are only three priority buckets), but
in all cases nice adjustment has the expected effect: positive nice
lowering the scheduling priority and negative nice raising it.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 50cbc8b4885b..2d5e71029d7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -3043,6 +3043,14 @@ static int eb_request_add(struct i915_execbuffer *eb,
struct i915_request *rq,
/* Check that the context wasn't destroyed before submission */
if (likely(!intel_context_is_closed(eb->context))) {
attr = eb->gem_context->sched;
+ /*
+* Inherit process nice when scheduling user contexts but only
+* if context has the default priority to avoid touching
+* contexts where GEM uapi has been used to explicitly lower
+* or elevate it.
+*/
+ if (attr.priority == I915_CONTEXT_DEFAULT_PRIORITY)
+ attr.priority = -task_nice(current);
} else {
/* Serialise with context_close via the add_to_timeline */
i915_request_set_error_once(rq, -ENOENT);
--
2.32.0
[PATCH 0/1] Inherit GPU scheduling priority from process nice
From: Tvrtko Ursulin Current processing landscape seems to be more and more composed of pipelines where computations are done on multiple hardware devices. Furthermore some of the non-CPU devices, like in this case many GPUs supported by the i915 driver, actually support priority based scheduling which is currently rather inaccessible to the user (in terms of being able to control it from the outside). >From these two statements a question arises on how to allow for a simple, effective and consolidated user experience. In other words why user would not be able to do something like: $ nice ffmmpeg ...transcode my videos... $ my-favourite-game And have the nice hint apply to GPU parts of the transcode pipeline as well? This would in fact follow the approach taken by kernel's block scheduler where ionice is by default inherited from process nice. This series implements the same idea by inheriting context creator and batch buffer submitted nice value as context nice. To avoid influencing GPU scheduling aware clients this is done only one for contexts where userspace hasn't explicitly specified a non-default scheduling priority The approach is completely compatible with GuC and drm/scheduler since all support at least low/normal/high priority levels with just the granularity of available control differing. In other words with GuC scheduling there is no difference between nice 5 and 10, both would map to low priority, but the default case of positive or negative nice, versus nice 0, is still correctly propagated to the firmware scheduler. With the series applied I simulated the scenario of a background GPU task running simultaneously with an interactive client, varying the former's nice value. Simulating a non-interactive GPU background task was: vblank_mode=0 nice -n glxgears -geometry 1600x800 Interactive client was simulated with: gem_wsim -w ~/test.wsim -r 300 -v # (This one is self-capped at ~60fps.) These were the results on DG1, first with execlists (default): Background nice | Interactive FPS ---+ | 59 0 | 35 10 | 42 As we can see running the background load with nice 10 can somewhat help the performance of the interactive/foreground task. (Although to be noted is that without the fair scheduler completed there are possible starvation issues depending on the workload which cannot be fixed by this patch.) Now results with GuC (although it is not default on DG1): Background nice | Interactive FPS ---+ | 58 0 | 26 10 | 25 Unfortunately GuC is not showing any change (25 vs 26 is rounding/run error). But reverse mesurement with background client at nice 0 and foreground at nice -10 does give 40 FPS proving the priority adjustment does work. (Same reverse test gives 46 FPS with execlists). What is happening with GuC here is something to be looked at since it seems normal-vs-low GuC priority time slices differently than normal-vs-high. Normal does not seem to be preferred over low, in this test at least. v2: * Moved notifier outside task_rq_lock. * Some improvements and restructuring on the i915 side of the series. v3: * Dropped task nice notifier - inheriting nice on request submit time is good enough. v4: * Realisation came that this can be heavily simplified and only one simple patch is enough to achieve the desired behaviour. * Fixed the priority adjustment location to actually worked after rebase! * Re-done the benchmarking. v5: * I am sending wrong files out yet again (v4), apologies for the spam.. Tvrtko Ursulin (1): drm/i915: Inherit submitter nice when scheduling requests drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 1 file changed, 8 insertions(+) -- 2.32.0
Re: [PATCH v7 04/12] clk: Always clamp the rounded rate
On Thu, Apr 07, 2022 at 08:27:39AM +, Yassine Oudjana wrote: > On Thursday, April 7th, 2022 at 12:08 PM, Maxime Ripard > wrote: > > I've been piling up few fixes already for other platforms, could you > > also test ? > > > > https://github.com/mripard/linux/tree/rpi/clk-improvements-more-fixes > > Alright, will test. I've pushed another version of my branch a few minutes ago, so make sure you pulled the last version if you want to test it :) Thanks! Maxime signature.asc Description: PGP signature
[PATCH 1/1] drm/i915: Inherit submitter nice when scheduling requests
From: Tvrtko Ursulin
Inherit submitter nice at point of request submission to account for long
running processes getting either externally or self re-niced.
This accounts for the current processing landscape where computational
pipelines are composed of CPU and GPU parts working in tandem.
Nice value will only apply to requests which originate from user contexts
and have default context priority. This is to avoid disturbing any
application made choices of low and high (batch processing and latency
sensitive compositing). In this case nice value adjusts the effective
priority in the narrow band of -19 to +20 around
I915_CONTEXT_DEFAULT_PRIORITY.
This means that userspace using the context priority uapi directly has a
wider range of possible adjustments (in practice that only applies to
execlists platforms - with GuC there are only three priority buckets), but
in all cases nice adjustment has the expected effect: positive nice
lowering the scheduling priority and negative nice raising it.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_request.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 582770360ad1..e5cfa073d8f0 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1811,8 +1811,17 @@ void i915_request_add(struct i915_request *rq)
/* XXX placeholder for selftests */
rcu_read_lock();
ctx = rcu_dereference(rq->context->gem_context);
- if (ctx)
+ if (ctx) {
attr = ctx->sched;
+ /*
+* Inherit process nice when scheduling user contexts but only
+* if context has the default priority to avoid touching
+* contexts where GEM uapi has been used to explicitly lower
+* or elevate it.
+*/
+ if (attr.priority == I915_CONTEXT_DEFAULT_PRIORITY)
+ attr.priority = -task_nice(current);
+ }
rcu_read_unlock();
__i915_request_queue(rq, &attr);
--
2.32.0
[PATCH 0/1] Inherit GPU scheduling priority from process nice
From: Tvrtko Ursulin Current processing landscape seems to be more and more composed of pipelines where computations are done on multiple hardware devices. Furthermore some of the non-CPU devices, like in this case many GPUs supported by the i915 driver, actually support priority based scheduling which is currently rather inaccessible to the user (in terms of being able to control it from the outside). >From these two statements a question arises on how to allow for a simple, effective and consolidated user experience. In other words why user would not be able to do something like: $ nice ffmmpeg ...transcode my videos... $ my-favourite-game And have the nice hint apply to GPU parts of the transcode pipeline as well? This would in fact follow the approach taken by kernel's block scheduler where ionice is by default inherited from process nice. This series implements the same idea by inheriting context creator and batch buffer submitted nice value as context nice. To avoid influencing GPU scheduling aware clients this is done only one for contexts where userspace hasn't explicitly specified a non-default scheduling priority The approach is completely compatible with GuC and drm/scheduler since all support at least low/normal/high priority levels with just the granularity of available control differing. In other words with GuC scheduling there is no difference between nice 5 and 10, both would map to low priority, but the default case of positive or negative nice, versus nice 0, is still correctly propagated to the firmware scheduler. With the series applied I simulated the scenario of a background GPU task running simultaneously with an interactive client, varying the former's nice value. Simulating a non-interactive GPU background task was: vblank_mode=0 nice -n glxgears -geometry 1600x800 Interactive client was simulated with: gem_wsim -w ~/test.wsim -r 300 -v # (This one is self-capped at ~60fps.) These were the results on DG1, first with execlists (default): Background nice | Interactive FPS ---+ | 59 0 | 35 10 | 42 As we can see running the background load with nice 10 can somewhat help the performance of the interactive/foreground task. (Although to be noted is that without the fair scheduler completed there are possible starvation issues depending on the workload which cannot be fixed by this patch.) Now results with GuC (although it is not default on DG1): Background nice | Interactive FPS ---+ | 58 0 | 26 10 | 25 Unfortunately GuC is not showing any change (25 vs 26 is rounding/run error). But reverse mesurement with background client at nice 0 and foreground at nice -10 does give 40 FPS proving the priority adjustment does work. (Same reverse test gives 46 FPS with execlists). What is happening with GuC here is something to be looked at since it seems normal-vs-low GuC priority time slices differently than normal-vs-high. Normal does not seem to be preferred over low, in this test at least. v2: * Moved notifier outside task_rq_lock. * Some improvements and restructuring on the i915 side of the series. v3: * Dropped task nice notifier - inheriting nice on request submit time is good enough. v4: * Realisation came that this can be heavily simplified and only one simple patch is enough to achieve the desired behaviour. * Fixed the priority adjustment location to actually worked after rebase! * Re-done the benchmarking. Tvrtko Ursulin (1): drm/i915: Inherit submitter nice when scheduling requests drivers/gpu/drm/i915/i915_request.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) -- 2.32.0
[PATCH v2] drm/edid: add EDID block count and size helpers
Add some helpers to figure out the EDID extension block count, block
count, size, pointers to blocks.
Unfortunately, we'll need to cast away the const in a few places where
we actually need to access the data.
v2: fix s/j/i/ introduced in a rebase
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 80 +++---
1 file changed, 57 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 90615e30eaf5..c09ff1efdbc8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1568,6 +1568,38 @@ static const struct drm_display_mode edid_4k_modes[] = {
/*** DDC fetch and block validation ***/
+static int edid_extension_block_count(const struct edid *edid)
+{
+ return edid->extensions;
+}
+
+static int edid_block_count(const struct edid *edid)
+{
+ return edid_extension_block_count(edid) + 1;
+}
+
+static int edid_size_by_blocks(int num_blocks)
+{
+ return num_blocks * EDID_LENGTH;
+}
+
+static int edid_size(const struct edid *edid)
+{
+ return edid_size_by_blocks(edid_block_count(edid));
+}
+
+static const void *edid_block_data(const struct edid *edid, int index)
+{
+ BUILD_BUG_ON(sizeof(*edid) != EDID_LENGTH);
+
+ return edid + index;
+}
+
+static const void *edid_extension_block_data(const struct edid *edid, int
index)
+{
+ return edid_block_data(edid, index + 1);
+}
+
static const u8 edid_header[] = {
0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
};
@@ -1654,8 +1686,8 @@ bool drm_edid_are_equal(const struct edid *edid1, const
struct edid *edid2)
return false;
if (edid1) {
- edid1_len = EDID_LENGTH * (1 + edid1->extensions);
- edid2_len = EDID_LENGTH * (1 + edid2->extensions);
+ edid1_len = edid_size(edid1);
+ edid2_len = edid_size(edid2);
if (edid1_len != edid2_len)
return false;
@@ -1865,14 +1897,16 @@ EXPORT_SYMBOL(drm_edid_block_valid);
bool drm_edid_is_valid(struct edid *edid)
{
int i;
- u8 *raw = (u8 *)edid;
if (!edid)
return false;
- for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
+ for (i = 0; i < edid_block_count(edid); i++) {
+ void *block = (void *)edid_block_data(edid, i);
+
+ if (!drm_edid_block_valid(block, i, true, NULL))
return false;
+ }
return true;
}
@@ -1885,13 +1919,13 @@ static struct edid *edid_filter_invalid_blocks(const
struct edid *edid,
int valid_extensions = edid->extensions - invalid_blocks;
int i;
- new = kmalloc_array(valid_extensions + 1, EDID_LENGTH, GFP_KERNEL);
+ new = kmalloc(edid_size_by_blocks(valid_extensions + 1), GFP_KERNEL);
if (!new)
goto out;
dest_block = new;
- for (i = 0; i <= edid->extensions; i++) {
- const void *block = edid + i;
+ for (i = 0; i < edid_block_count(edid); i++) {
+ const void *block = edid_block_data(edid, i);
if (edid_block_valid(block, i == 0))
memcpy(dest_block++, block, EDID_LENGTH);
@@ -2101,7 +2135,7 @@ struct edid *drm_do_get_edid(struct drm_connector
*connector,
void *context)
{
enum edid_block_status status;
- int j, invalid_blocks = 0;
+ int i, invalid_blocks = 0;
struct edid *edid, *new;
edid = drm_get_override_edid(connector);
@@ -2133,20 +2167,20 @@ struct edid *drm_do_get_edid(struct drm_connector
*connector,
goto fail;
}
- if (edid->extensions == 0)
+ if (!edid_extension_block_count(edid) == 0)
goto ok;
- new = krealloc(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL);
+ new = krealloc(edid, edid_size(edid), GFP_KERNEL);
if (!new)
goto fail;
edid = new;
- for (j = 1; j <= edid->extensions; j++) {
- void *block = edid + j;
+ for (i = 1; i < edid_block_count(edid); i++) {
+ void *block = (void *)edid_block_data(edid, i);
- status = edid_block_read(block, j, read_block, context);
+ status = edid_block_read(block, i, read_block, context);
- edid_block_status_print(status, block, j);
+ edid_block_status_print(status, block, i);
if (!edid_block_status_valid(status, edid_block_tag(block))) {
if (status == EDID_BLOCK_READ_FAIL)
@@ -2156,7 +2190,7 @@ struct edid *drm_do_get_edid(struct drm_connector
*connector,
}
if (invalid_blocks) {
- connector_bad_edid(connector, edid, edid->extensions + 1);
+ connector_bad_edid(connector, edid, edid_block_count(edid))
Re: [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g
On Thu, 07 Apr 2022, Zhi Wang wrote:
> diff --git a/drivers/gpu/drm/i915/intel_gvt.h
> b/drivers/gpu/drm/i915/intel_gvt.h
> index d7d3fb6186fd..7665d7cf0bdd 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.h
> +++ b/drivers/gpu/drm/i915/intel_gvt.h
> @@ -26,7 +26,17 @@
>
> struct drm_i915_private;
>
> +#include
You only need . Please add it before the forward
declaration above.
> +
> #ifdef CONFIG_DRM_I915_GVT
> +
> +struct intel_gvt_mmio_table_iter {
> + struct drm_i915_private *i915;
> + void *data;
> + int (*handle_mmio_cb)(struct intel_gvt_mmio_table_iter *iter,
> + u32 offset, u32 size);
> +};
> +
> int intel_gvt_init(struct drm_i915_private *dev_priv);
> void intel_gvt_driver_remove(struct drm_i915_private *dev_priv);
> int intel_gvt_init_device(struct drm_i915_private *dev_priv);
> @@ -34,6 +44,7 @@ void intel_gvt_clean_device(struct drm_i915_private
> *dev_priv);
> int intel_gvt_init_host(void);
> void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv);
> void intel_gvt_resume(struct drm_i915_private *dev_priv);
> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter);
> #else
> static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
> {
> @@ -51,6 +62,16 @@ static inline void intel_gvt_sanitize_options(struct
> drm_i915_private *dev_priv)
> static inline void intel_gvt_resume(struct drm_i915_private *dev_priv)
> {
> }
> +
> +unsigned long intel_gvt_get_device_type(struct drm_i915_private *i915)
> +{
> + return 0;
> +}
The CONFIG_DRM_I915_GVT=y counterpart for this is in mmio.h. Should be
both in the same header.
> +
> +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter)
> +{
> + return 0;
> +}
> #endif
>
> #endif /* _INTEL_GVT_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> new file mode 100644
> index ..d29491a6d209
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> @@ -0,0 +1,1290 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "i915_reg.h"
> +#include "display/vlv_dsi_pll_regs.h"
> +#include "gt/intel_gt_regs.h"
> +#include "intel_mchbar_regs.h"
> +#include "i915_pvinfo.h"
> +#include "intel_gvt.h"
> +#include "gvt/gvt.h"
Generally we have the include lists sorted.
Other than the nitpicks above, the series is
Acked-by: Jani Nikula
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v9 00/23] drm/rockchip: RK356x VOP2 support
> Wiadomość napisana przez Sascha Hauer w dniu > 07.04.2022, o godz. 12:16: > > > Yes, and it raises a few more ;) pls see at end of email: DRI state with playback > >> >> player: >> >> 2022-04-06 17:52:26.424487 I Display: Geometry: 1920x1080+0+0 Size(Qt): >> 930mmx530mm >> 2022-04-06 17:52:26.424922 I /dev/dri/card0 Qt EGLFS/KMS Fd:5 Crtc id:49 >> Connector id:51 Atomic: 1 >> 2022-04-06 17:52:26.425061 I /dev/dri/card0: Authenticated >> 2022-04-06 17:52:26.534362 I /dev/dri/card0: Found 3 planes; 3 for this CRTC >> 2022-04-06 17:52:26.534384 I /dev/dri/card0: Selected Plane #37 Overlay for >> video >> 2022-04-06 17:52:26.534430 I /dev/dri/card0: Supported DRM video formats: >> NV12,NV16,NV24,YVYU,VYUY >> 2022-04-06 17:52:26.534437 I /dev/dri/card0: Selected Plane #43 Overlay for >> GUI >> 2022-04-06 17:52:26.534480 I /dev/dri/card0: DRM device retrieved from Qt >> 2022-04-06 17:52:26.534489 I /dev/dri/card0: Multi-plane setup: Requested: 1 >> Setup: 1 >> >> so: >> plane #37 is where video is drawing >> plane #43 is GUI/OSD >> >> >> dri state: >> >> root@Myth-Frontend-06c7e973c2f1:~ # cat /sys/kernel/debug/dri/0/state >> plane[31]: Smart0-win0 >>crtc=video_port0 >>fb=58 >>allocated by = mythfrontend >>refcount=2 >>format=XR24 little-endian (0x34325258) >>modifier=0x0 >>size=1920x1080 >>layers: >>size[0]=1920x1080 >>pitch[0]=7680 >>offset[0]=0 >>obj[0]: >>name=0 >>refcount=4 >>start= >>size=8294400 >>imported=no >>crtc-pos=1920x1080+0+0 >>src-pos=1920.00x1080.00+0.00+0.00 >>rotation=1 >>normalized-zpos=0 >>color-encoding=ITU-R BT.601 YCbCr >>color-range=YCbCr limited range > > Ok, this seems to be the base plane. > >> plane[37]: Esmart0-win0 >>crtc=(null) > > crtc=null? Did you capture the state without a video playing? Otherwise > I would expect a crtc associated here. Indeed. This was from player sitting in UI (no playback). Pls see at bottom of email state with video playback (it has crtc=video_port0) > >>fb=0 >>crtc-pos=1920x1080+0+0 >>src-pos=1920.00x1080.00+0.00+0.00 >>rotation=1 >>normalized-zpos=0 >>color-encoding=ITU-R BT.601 YCbCr >>color-range=YCbCr limited range >> plane[43]: Cluster0-win0 >>crtc=(null) > > This plane is selected for OSD by your application. The cluster windows > can't show a regular linear framebuffer, they can only do AFBC. You'll > see that in modetest: > > in_formats blob decoded: > XR24: ARM_BLOCK_SIZE=16x16, >ARM_BLOCK_SIZE=16x16,MODE=SPARSE >ARM_BLOCK_SIZE=16x16,MODE=YTR >ARM_BLOCK_SIZE=16x16,MODE=CBR >ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE >ARM_BLOCK_SIZE=16x16,MODE=SPARSE|CBR >ARM_BLOCK_SIZE=16x16,MODE=YTR|CBR >ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE|CBR >ARM_BLOCK_SIZE=16x16,MODE=YTR|SPLIT|SPARSE > ... > > The other windows show "XR24: LINEAR" here. Does your application use > the GPU to render the OSD? Yes. > Otherwise I doubt your application can > handle this format, so it should not use this layer. > >>fb=0 >>crtc-pos=0x0+0+0 >>src-pos=0.00x0.00+0.00+0.00 >>rotation=1 >>normalized-zpos=0 > > I would be interested in this output when the player is actually playing > something. Pls see at bottom. > This normalized-zpos puzzles me a bit. I'm not surprised :-). You are puzzled probably because rk35xx current VOP2 code requires - from me - to force Z-position = 0 in Qt if I want to have GUI visible on screen. Without this screen is black. This seems to be i think - another issue to resolve (no any other SoC requires this). I'm not sure where issue is - but as i need to do this only on VOP2 - i think there is somewhere something not right in VOP2 code. > Normally it should be > unique over all enabled planes for a CRTC. Maybe 0 is ok here because > it's currently not associated to any CRTC. It is because of me setting it to 0 (see explanations above) > > DRI state with video playback: root@Myth-Frontend-06c7e973c2f1:~ # cat /sys/kernel/debug/dri/0/state plane[31]: Smart0-win0 crtc=video_port0 fb=55 allocated by = mythfrontend refcount=2 format=XR24 little-endian (0x34325258) modifier=0x0 size=1920x1080 l
Re: [PATCH v2] drm/amd/display: don't ignore alpha property on pre-multiplied mode
Patch merged to amd-staging-drm-next. Thanks a lot! On 2022-04-05 15:32, Simon Ser wrote: I've tested this patch and it fixes my bug [1]. Thanks! Tested-by: Simon Ser [1]: https://gitlab.freedesktop.org/drm/amd/-/issues/1734>
RE: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
Hi Dmitry and Doug, > Hi, > > On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov > wrote: > > > > On 05/04/2022 20:02, Doug Anderson wrote: > > > Hi, > > > > > > On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov > > > wrote: > > >>> 3. For DP and eDP HPD means something a little different. > > >>> Essentially there are two concepts: a) is a display physically > > >>> connected and b) is the display powered up and ready. For DP, the > > >>> two are really tied together. From the kernel's point of view you > > >>> never "power down" a DP display and you can't detect that it's > > >>> physically connected until it's ready. Said another way, on you > > >>> tie "is a display there" to the HPD line and the moment a display > > >>> is there it's ready for you to do AUX transfers. For eDP, in the > > >>> lowest power state of a display it _won't_ assert its "HPD" > > >>> signal. However, it's still physically present. For eDP you simply > > >>> have to _assume_ it's present without any actual proof since you > > >>> can't get proof until you power it up. Thus for eDP, you report > > >>> that the display is there as soon as we're asked. We can't _talk_ > > >>> to the display yet, though. So in get_modes() we need to be able > > >>> to power the display on enough to talk over the AUX channel to it. > > >>> As part of this, we wait for the signal named "HPD" which really means > "panel finished powering on" in this context. > > >>> > > >>> NOTE: for aux transfer, we don't have the _display_ pipe and > > >>> clocks running. We only have enough stuff running to do the AUX > transfer. > > >>> We're not clocking out pixels. We haven't fully powered on the > > >>> display. The AUX transfer is designed to be something that can be > > >>> done early _before_ you turn on the display. > > >>> > > >>> > > >>> OK, so basically that was a longwinded way of saying: yes, we > > >>> could avoid the AUX transfer in probe, but we can't wait all the > > >>> way to enable. We have to be able to transfer in get_modes(). If > > >>> you think that's helpful I think it'd be a pretty easy patch to > > >>> write even if it would look a tad bit awkward IMO. Let me know if > > >>> you want me to post it up. > > >> > > >> I think it would be a good idea. At least it will allow us to > > >> judge, which is the more correct way. > > > > > > I'm still happy to prototype this, but the more I think about it the > > > more it feels like a workaround for the Qualcomm driver. The eDP > > > panel driver is actually given a pointer to the AUX bus at probe > > > time. It's really weird to say that we can't do a transfer on it > > > yet... As you said, this is a little sideband bus. It should be able > > > to be used without all the full blown infra of the rest of the driver. > > > > Yes, I have that feeling too. However I also have a feeling that just > > powering up the PHY before the bus probe is ... a hack. There are no > > obvious stopgaps for the driver not to power it down later. > > This is why I think we need to move to Runtime PM to manage this. Basically: > > 1. When an AUX transfer happens, you grab a PM runtime reference that > _that_ powers up the PHY. > > 2. At the end of the AUX transfer function, you do a "put_autosuspend". > > Then it becomes not a hack, right? > > pm runtime ops needs to be implemented for both eDP and DP. This change take good amount of planning and code changes as it affects DP also. Because this patch series consist of basic eDP changes for SC7280 bootup, shall we take this pm_runtime implementation in subsequent patch series? > > A cleaner design might be to split all hotplug event handling from the > > dp_display, provide a lightweight state machine for the eDP and select > > which state machine to use depending on the hardware connector type. > > The dp_display_bind/unbind would probably also be duplicated and > > receive correct code flows for calling dp_parser_get_next_bridge, etc. > > Basically that means that depending on the device data we'd use either > > dp_display_comp_ops or (new) edp_comp_ops. > > > > WDYT? > > I don't think I know the structure of the MSM DP code to make a definitive > answer here. I think I'd have to see a patch. However I'd agree in general > terms that we need some different flows for the two. > ;-) We definitely want to limit the differences but some of them will be > unavoidable... > > > -Doug Thank you, Sankeerth
Re: [PATCH] drm/amd/display: Fix indenting mistakes in dcn10_hw_sequencer.c
On 2022-04-07 02:00, Haowen Bai wrote:
> Smatch reports the following:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2174
> dcn10_enable_vblanks_synchronization() warn: if statement not indented
>
> Signed-off-by: Haowen Bai
Reviewed-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index ee22f4422d26..3c338b85040c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2172,13 +2172,13 @@ void dcn10_enable_vblanks_synchronization(
> if (master >= 0) {
> for (i = 0; i < group_size; i++) {
> if (i != master &&
> !grouped_pipes[i]->stream->has_non_synchronizable_pclk)
> - grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> - grouped_pipes[master]->stream_res.tg,
> - grouped_pipes[i]->stream_res.tg,
> -
> grouped_pipes[master]->stream->timing.pix_clk_100hz,
> - grouped_pipes[i]->stream->timing.pix_clk_100hz,
> - get_clock_divider(grouped_pipes[master], false),
> - get_clock_divider(grouped_pipes[i], false));
> +
> grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
> + grouped_pipes[master]->stream_res.tg,
> + grouped_pipes[i]->stream_res.tg,
> +
> grouped_pipes[master]->stream->timing.pix_clk_100hz,
> +
> grouped_pipes[i]->stream->timing.pix_clk_100hz,
> +
> get_clock_divider(grouped_pipes[master], false),
> + get_clock_divider(grouped_pipes[i],
> false));
> grouped_pipes[i]->stream->vblank_synchronized =
> true;
> }
> grouped_pipes[master]->stream->vblank_synchronized = true;
Re: [PATCH] drm/amd/display: Fix pointer dereferenced before checking
On 2022-04-07 01:52, Haowen Bai wrote: > The pointer dc is dereferencing pointer plane_state before plane_state > is being null checked. Fix this by assigning plane_state->ctx->dc to > dc only if plane_state is not NULL, otherwise just NULL. > > Signed-off-by: Haowen Bai > --- > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > index 50820e79d3c4..ee22f4422d26 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > @@ -3211,7 +3211,7 @@ void dcn10_update_pending_status(struct pipe_ctx > *pipe_ctx) > struct dc_plane_state *plane_state = pipe_ctx->plane_state; > struct timing_generator *tg = pipe_ctx->stream_res.tg; > bool flip_pending; > - struct dc *dc = plane_state->ctx->dc; This has worked for years now, meaning plane_state is never NULL here. It might be better to drop the NULL check below. Harry > + struct dc *dc = plane_state ? plane_state->ctx->dc : NULL; > > if (plane_state == NULL) > return;
Re: [PATCH 1/5] drm/ttm: Add common debugfs code for resource managers
On Thu, 2022-04-07 at 08:01 +0200, Christian König wrote:
> Am 07.04.22 um 04:56 schrieb Zack Rusin:
> > From: Zack Rusin
> >
> > Drivers duplicate the code required to add debugfs entries for
> > various
> > ttm resource managers. To fix it add common TTM resource manager
> > code that each driver can reuse.
> >
> > Because TTM resource managers can be initialized and set a lot
> > later
> > than TTM device initialization a seperate init function is
> > required.
> > Specific resource managers can overwrite
> > ttm_resource_manager_func::debug to get more information from those
> > debugfs entries.
> >
> > Signed-off-by: Zack Rusin
> > Cc: Christian Koenig
> > Cc: Huang Rui
> > Cc: David Airlie
> > Cc: Daniel Vetter
>
> Ah, yes that was on my TODO list for quite a while as well.
>
> > ---
> > drivers/gpu/drm/ttm/ttm_resource.c | 65
> > ++
> > include/drm/ttm/ttm_resource.h | 4 ++
> > 2 files changed, 69 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > b/drivers/gpu/drm/ttm/ttm_resource.c
> > index 492ba3157e75..6392ad3e9a88 100644
> > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > @@ -29,6 +29,8 @@
> > #include
> > #include
> >
> > +#include "ttm_module.h"
> > +
> > /**
> > * ttm_lru_bulk_move_init - initialize a bulk move structure
> > * @bulk: the structure to init
> > @@ -644,3 +646,66 @@ ttm_kmap_iter_linear_io_fini(struct
> > ttm_kmap_iter_linear_io *iter_io,
> >
> > ttm_mem_io_free(bdev, mem);
> > }
> > +
> > +#if defined(CONFIG_DEBUG_FS)
> > +
> > +#define TTM_RES_MAN_SHOW(i) \
> > + static int ttm_resource_manager##i##_show(struct seq_file *m,
> > void *unused) \
> > + { \
> > + struct ttm_device *bdev = (struct ttm_device *)m-
> > >private; \
> > + struct ttm_resource_manager *man =
> > ttm_manager_type(bdev, i); \
> > + struct drm_printer p = drm_seq_file_printer(m); \
> > + ttm_resource_manager_debug(man, &p); \
> > + return 0; \
> > + }\
> > + DEFINE_SHOW_ATTRIBUTE(ttm_resource_manager##i)
> > +
> > +TTM_RES_MAN_SHOW(0);
> > +TTM_RES_MAN_SHOW(1);
> > +TTM_RES_MAN_SHOW(2);
> > +TTM_RES_MAN_SHOW(3);
> > +TTM_RES_MAN_SHOW(4);
> > +TTM_RES_MAN_SHOW(5);
> > +TTM_RES_MAN_SHOW(6);
>
> Uff, please not a static array.
>
> > +
> > +#endif
> > +
> > +/**
> > + * ttm_resource_manager_debugfs_init - Setup debugfs entries for
> > specified
> > + * resource managers.
> > + * @bdev: The TTM device
> > + * @file_names: A mapping between TTM_TT placements and the
> > debugfs file
> > + * names
> > + * @num_file_names: The array size of @file_names.
> > + *
> > + * This function setups up debugfs files that can be used to look
> > + * at debug statistics of the specified ttm_resource_managers.
> > + * @file_names array is used to figure out which ttm placements
> > + * will get debugfs files created for them.
> > + */
> > +void
> > +ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
> > + const char * const file_names[],
> > + uint32_t num_file_names)
> > +{
> > +#if defined(CONFIG_DEBUG_FS)
> > + uint32_t i;
> > + const struct file_operations *fops[] = {
> > + &ttm_resource_manager0_fops,
> > + &ttm_resource_manager1_fops,
> > + &ttm_resource_manager2_fops,
> > + &ttm_resource_manager3_fops,
> > + &ttm_resource_manager4_fops,
> > + &ttm_resource_manager5_fops,
> > + &ttm_resource_manager6_fops,
> > + };
> > +
> > + WARN_ON(num_file_names > ARRAY_SIZE(fops));
> > +
> > + for (i = 0; i < num_file_names; ++i)
> > + if (file_names[i] && fops[i])
> > + debugfs_create_file(file_names[i], 0444,
> > + ttm_debugfs_root, bdev,
> > fops[i]);
>
> You can give the ttm_resource_manager directly as parameter here
> instead
> of the bdev and avoid the whole handling with the macro and global
> arrays.
We could but that does change the behaviour. I was trying to preserve
the old semantics. Because the lifetimes of driver specific managers
are not handled by ttm, there's nothing preventing the drivers from,
e.g. during reset, deleting the old and setting up new resource
managers. By always using ttm_manager_type() we make sure that we're
always using the current one. Passing ttm_resource_manager directly
makes it impossible to validate that at _show() time
ttm_resource_manager is still valid. It's not a problem for vmwgfx
because we never reset the resource managers but I didn't feel
comfortable changing the semantics like that for all drivers. It could
lead to weird crashes, but if you prefer that approach I'm happy to
change it.
z
RE: [PATCH v6 8/8] drm/msm/dp: Handle eDP mode_valid differently from dp
Hi Dmitry, > > > > > On Wed, 30 Mar 2022 at 19:04, Sankeerth Billakanti > > > > > wrote: > > > > > > > > > > > > The panel-edp driver modes needs to be validated differently > > > > > > from DP because the link capabilities are not available for EDP by > that time. > > > > > > > > > > > > Signed-off-by: Sankeerth Billakanti > > > > > > > > > > > > > > > > This should not be necessary after > > > > > > > > > https://patchwork.freedesktop.org/patch/479261/?series=101682&rev=1. > > > > > Could you please check? > > > > > > > > > > > > > The check for DP_MAX_PIXEL_CLK_KHZ is not necessary anymore but > we > > > > need to return early for eDP because unlike DP, eDP context will > > > > not have the information about the number of lanes and link clock. > > > > > > > > So, I will modify the patch to return after the > > > > DP_MAX_PIXEL_CLK_KHZ > > > check if is_eDP is set. > > > > > > I haven't walked through all the relevant code but something you > > > said above sounds strange. You say that for eDP we don't have info > > > about the number of lanes? We _should_. > > > > > > It's certainly possible to have a panel that supports _either_ 1 or > > > 2 lanes but then only physically connect 1 lane to it. ...or you > > > could have a panel that supports 2 or 4 lanes and you only connect 1 lane. > > > See, for instance, ti_sn_bridge_parse_lanes. There we assume 4 lanes > > > but if a "data-lanes" property is present then we can use that to > > > know that fewer lanes are physically connected. > > > > > > It's also possible to connect more lanes to a panel than it supports. > > > You could connect 2 lanes to it but then it only supports 1. This > > > case needs to be handled as well... > > > > > > > I was referring to the checks we do for DP in dp_bridge_mode_valid. We > > check if the Link bandwidth can support the pixel bandwidth. For an > > external DP connection, the Initial DPCD/EDID read after cable > > connection will return the sink capabilities like link rate, lane > > count and bpp information that are used to we filter out the unsupported > modes from the list of modes from EDID. > > > > For eDP case, the dp driver performs the first dpcd read during > > bridge_enable. The dp_bridge_mode_valid function is executed before > > bridge_enable and hence does not have the full link or the sink > > capabilities information like external DP connection, by then. > > It sounds to me like we should emulate the HPD event for eDP to be handled > earlier than the get_modes()/prepare() calls are attempted. > However this might open another can of worms. > For DP, the HPD handler mainly initiates link training and gets the EDID. Before adding support for a separate eDP panel, we had discussed about this internally and decided to emulate eDP HPD during enable(). Main reason being, eDP power is guaranteed to be on only after bridge_enable(). So, eDP link training can happen and sustain only after bridge_enable(). Emulating HPD before/during get_modes will not have any effect because: 1. get_modes() will go to panel's get_modes() function to power on read EDID 2. panel power will be turned off after get_modes(). Panel power off will reset every write transaction in DPCD. anyway invalidating link training 3. mode_valid will land in dp driver but panel will not be powered on at that time and we cannot do aux transfers or DPCD read writes. > > So, we need to proceed with the reported mode for eDP. > > Well... Even if during the first call to get_modes() the DPCD is not read, > during subsequent calls the driver has necessary information, so it can > proceed with all the checks, can't it? > get_modes() currently does not land in DP driver. It gets executed in panel bridge. But the mode_valid() comes to DP driver to check the controller compatibility. > -- > With best wishes > Dmitry Thank you, Sankeerth
Re: [PATCH v2 3/3] ARM: dts: Use new media bus type macros
On 4/7/22 15:31, Laurent Pinchart wrote:
Hi Alexandre,
On Thu, Apr 07, 2022 at 02:41:58PM +0200, Alexandre TORGUE wrote:
On 3/6/22 18:39, Laurent Pinchart wrote:
Now that a header exists with macros for the media interface bus-type
values, replace hardcoding numerical constants with the corresponding
macros in the DT sources.
Signed-off-by: Laurent Pinchart
---
arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 4 +++-
arch/arm/boot/dts/omap3-n900.dts | 5 +++--
arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts | 11 +++
.../dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi | 4 +++-
.../dts/r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi | 4 +++-
arch/arm/boot/dts/stm32429i-eval.dts | 3 ++-
arch/arm/boot/dts/stm32mp157c-ev1.dts | 3 ++-
7 files changed, 23 insertions(+), 11 deletions(-)
sorry for this late answer. Is it possible to split ARM DT patches by
vendor ?
Sure. Is that only to ease backporting, or do you want the ST part to be
merged through a different tree ?
I usually take all STM32 DT patches in my stm32-next branch. It's to
avoid merge issue at arm-soc maintainer level.
thanks
alex
diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
index a3fde3316c73..89234bbd02f4 100644
--- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
+++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
@@ -2,6 +2,8 @@
//
// Copyright (C) 2015 Freescale Semiconductor, Inc.
+#include
+
/ {
chosen {
stdout-path = &uart1;
@@ -170,7 +172,7 @@ &csi {
port {
parallel_from_ov5640: endpoint {
remote-endpoint = <&ov5640_to_parallel>;
- bus-type = <5>; /* Parallel bus */
+ bus-type = ;
};
};
};
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index d40c3d2c4914..9cad9d6a83e2 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -9,6 +9,7 @@
#include "omap34xx.dtsi"
#include
#include
+#include
/*
* Default secure signed bootloader (Nokia X-Loader) does not enable L3
firewall
@@ -194,7 +195,7 @@ port@1 {
csi_isp: endpoint {
remote-endpoint = <&csi_cam1>;
- bus-type = <3>; /* CCP2 */
+ bus-type = ;
clock-lanes = <1>;
data-lanes = <0>;
lane-polarity = <0 0>;
@@ -835,7 +836,7 @@ cam1: camera@3e {
port {
csi_cam1: endpoint {
- bus-type = <3>; /* CCP2 */
+ bus-type = ;
strobe = <1>;
clock-inv = <0>;
crc = <1>;
diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
index 3c8a7c8b1fdd..1043603fc4a5 100644
--- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
+++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
@@ -7,6 +7,9 @@
*/
/dts-v1/;
+
+#include
+
#include "r8a7742-iwg21d-q7.dts"
/ {
@@ -242,7 +245,7 @@ port {
vin0ep: endpoint {
remote-endpoint = <&cam0ep>;
bus-width = <8>;
- bus-type = <6>;
+ bus-type = ;
};
};
};
@@ -273,7 +276,7 @@ port {
vin1ep: endpoint {
remote-endpoint = <&cam1ep>;
bus-width = <8>;
- bus-type = <6>;
+ bus-type = ;
};
};
};
@@ -305,7 +308,7 @@ vin2ep: endpoint {
remote-endpoint = <&cam2ep>;
bus-width = <8>;
data-shift = <8>;
- bus-type = <6>;
+ bus-type = ;
};
};
};
@@ -335,7 +338,7 @@ port {
vin3ep: endpoint {
remote-endpoint = <&cam3ep>;
bus-width = <8>;
- bus-type = <6>;
+ bus-type = ;
};
};
};
diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
index 40cef0b1d1e6..c73160df619d 100644
--- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
+++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
@@ -7,6 +7,8 @@
* Copyright (C) 2020 Renesas Electronics Corp.
*/
+#include
+
#define CAM_ENABLED 1
&CAM_PARENT_I2C {
@@ -26,7 +28,7 @@ port {
CAM_EP: endpoint {
Re: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu
Dear Arunpravin,
Thank you for your patch.
Am 07.04.22 um 07:46 schrieb Arunpravin Paneer Selvam:
- Switch to drm buddy allocator
- Add resource cursor support for drm buddy
I though after the last long discussion, you would actually act on the
review comments. Daniel wrote a good summary, you could more or less
copy and past. So why didn’t you?
So, I really wish to not have the patch commit as is.
The summary should also say something about using mutex over spinlocks.
For me the version change summaries below are just for reviewers of
earlier iterations to see what changed, and not something to be read easily.
Kind regards,
Paul
v2(Matthew Auld):
- replace spinlock with mutex as we call kmem_cache_zalloc
(..., GFP_KERNEL) in drm_buddy_alloc() function
- lock drm_buddy_block_trim() function as it calls
mark_free/mark_split are all globally visible
v3(Matthew Auld):
- remove trim method error handling as we address the failure case
at drm_buddy_block_trim() function
v4:
- fix warnings reported by kernel test robot
v5:
- fix merge conflict issue
v6:
- fix warnings reported by kernel test robot
v7:
- remove DRM_BUDDY_RANGE_ALLOCATION flag usage
v8:
- keep DRM_BUDDY_RANGE_ALLOCATION flag usage
- resolve conflicts created by drm/amdgpu: remove VRAM accounting v2
v9(Christian):
- merged the below patch
- drm/amdgpu: move vram inline functions into a header
- rename label name as fallback
- move struct amdgpu_vram_mgr to amdgpu_vram_mgr.h
- remove unnecessary flags from struct amdgpu_vram_reservation
- rewrite block NULL check condition
- change else style as per coding standard
- rewrite the node max size
- add a helper function to fetch the first entry from the list
v10(Christian):
- rename amdgpu_get_node() function name as amdgpu_vram_mgr_first_block
v11:
- if size is not aligned with min_page_size, enable is_contiguous flag,
therefore, the size round up to the power of two and trimmed to the
original size.
v12:
- rename the function names having prefix as amdgpu_vram_mgr_*()
- modify the round_up() logic conforming to contiguous flag enablement
or if size is not aligned to min_block_size
- modify the trim logic
- rename node as block wherever applicable
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/Kconfig | 1 +
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 97 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 10 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 359 ++
4 files changed, 291 insertions(+), 176 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f1422bee3dcc..5133c3f028ab 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -280,6 +280,7 @@ config DRM_AMDGPU
select HWMON
select BACKLIGHT_CLASS_DEVICE
select INTERVAL_TREE
+ select DRM_BUDDY
help
Choose this option if you have a recent AMD Radeon graphics card.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index acfa207cf970..6546552e596c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -30,12 +30,15 @@
#include
#include
+#include "amdgpu_vram_mgr.h"
+
/* state back for walking over vram_mgr and gtt_mgr allocations */
struct amdgpu_res_cursor {
uint64_tstart;
uint64_tsize;
uint64_tremaining;
- struct drm_mm_node *node;
+ void*node;
+ uint32_tmem_type;
};
/**
@@ -52,27 +55,63 @@ static inline void amdgpu_res_first(struct ttm_resource
*res,
uint64_t start, uint64_t size,
struct amdgpu_res_cursor *cur)
{
+ struct drm_buddy_block *block;
+ struct list_head *head, *next;
struct drm_mm_node *node;
- if (!res || res->mem_type == TTM_PL_SYSTEM) {
- cur->start = start;
- cur->size = size;
- cur->remaining = size;
- cur->node = NULL;
- WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
- return;
- }
+ if (!res)
+ goto fallback;
BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
- node = to_ttm_range_mgr_node(res)->mm_nodes;
- while (start >= node->size << PAGE_SHIFT)
- start -= node++->size << PAGE_SHIFT;
+ cur->mem_type = res->mem_type;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ head = &to_amdgpu_vram_mgr_resource(res)->blocks;
+
+ block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+
Re: [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g
On 2022.04.07 03:19:43 -0400, Zhi Wang wrote: > From: Zhi Wang > > To support the new mdev interfaces and the re-factor patches from > Christoph, which moves the GVT-g code into a dedicated module, the GVT-g > MMIO tracking table needs to be separated from GVT-g. > Looks fine to me. Thanks! Reviewed-by: Zhenyu Wang signature.asc Description: PGP signature
Re: [PATCH 1/4] drm/fourcc: Introduce format modifiers for DG2 render and media compression
Reviewed-by: Juha-Pekka Heikkila
On 4.4.2022 16.38, Imre Deak wrote:
From: Matt Roper
The render/media engines on DG2 unify render compression and media
compression into a single format for the first time, using the Tile 4
layout for main surfaces. The compression algorithm is different from
any previous platform and the display engine must still be configured to
decompress either a render or media compressed surface; as such, we
need new RC and MC framebuffer modifiers to represent buffers in this
format.
v2: Clarify modifier layout description.
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Matt Roper
Signed-off-by: Imre Deak
Acked-by: Nanley Chery
---
include/uapi/drm/drm_fourcc.h | 22 ++
1 file changed, 22 insertions(+)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index b73fe6797fc37..4a5117715db3c 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -583,6 +583,28 @@ extern "C" {
*/
#define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 9)
+/*
+ * Intel color control surfaces (CCS) for DG2 render compression.
+ *
+ * The main surface is Tile 4 and at plane index 0. The CCS data is stored
+ * outside of the GEM object in a reserved memory area dedicated for the
+ * storage of the CCS data for all RC/RC_CC/MC compressible GEM objects. The
+ * main surface pitch is required to be a multiple of four Tile 4 widths.
+ */
+#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10)
+
+/*
+ * Intel color control surfaces (CCS) for DG2 media compression.
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all RC/RC_CC/MC compressible GEM objects. The main surface
+ * pitch is required to be a multiple of four Tile 4 widths.
+ */
+#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 11)
+
/*
* Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
*
Re: [PATCH v2 0/4] drm/ssd130x: Add support for SINO WEALTH SH1106
On 4/6/22 19:29, Chen-Yu Tsai wrote: Pushed this series to drm-misc (drm-misc-next), thanks again for your patches! -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
