[PATCH 1/5] drm: report dp downstream port type as a subconnector property
From: Oleg Vasilev
Currently, downstream port type is only reported in debugfs. This
information should be considered important since it reflects the actual
physical connector type. Some userspace (e.g. window compositors)
may want to show this info to a user.
The 'subconnector' property is already utilized for DVI-I and TV-out for
reporting connector subtype.
The initial motivation for this feature came from i2c test [1].
It is supposed to be skipped on VGA connectors, but it cannot
detect VGA over DP and fails instead.
v2:
- Ville: utilized drm_dp_is_branch()
- Ville: implement DP 1.0 downstream type info
- Replaced create_dp_properties with add_dp_subconnector_property
- Added dp_set_subconnector_property helper
v4:
- Ville: add DP1.0 best assumption about subconnector
- Ville: assume DVI is DVI-D
- Ville: reuse Writeback enum value for Virtual subconnector
- Renamed #defines: HDMI -> HDMIA, DP -> DisplayPort
v5: rebase
[1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
Cc: Ville Syrjälä
Cc: intel-...@lists.freedesktop.org
Signed-off-by: Jeevan B
Signed-off-by: Oleg Vasilev
Reviewed-by: Emil Velikov
Link:
https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-3-oleg.vasi...@intel.com
---
drivers/gpu/drm/drm_connector.c | 49 --
drivers/gpu/drm/drm_dp_helper.c | 77 +
include/drm/drm_connector.h | 3 ++
include/drm/drm_dp_helper.h | 8 +
include/drm/drm_mode_config.h | 6
include/uapi/drm/drm_mode.h | 21 ++-
6 files changed, 154 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b1099e1..b6972d1 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -844,7 +844,7 @@ static const struct drm_prop_enum_list
drm_dvi_i_select_enum_list[] = {
DRM_ENUM_NAME_FN(drm_get_dvi_i_select_name, drm_dvi_i_select_enum_list)
static const struct drm_prop_enum_list drm_dvi_i_subconnector_enum_list[] = {
- { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I and TV-out */
+ { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I, TV-out and
DP */
{ DRM_MODE_SUBCONNECTOR_DVID, "DVI-D" }, /* DVI-I */
{ DRM_MODE_SUBCONNECTOR_DVIA, "DVI-A" }, /* DVI-I */
};
@@ -861,7 +861,7 @@ static const struct drm_prop_enum_list
drm_tv_select_enum_list[] = {
DRM_ENUM_NAME_FN(drm_get_tv_select_name, drm_tv_select_enum_list)
static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
- { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I and TV-out */
+ { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I, TV-out and
DP */
{ DRM_MODE_SUBCONNECTOR_Composite, "Composite" }, /* TV-out */
{ DRM_MODE_SUBCONNECTOR_SVIDEO,"SVIDEO"}, /* TV-out */
{ DRM_MODE_SUBCONNECTOR_Component, "Component" }, /* TV-out */
@@ -870,6 +870,19 @@ static const struct drm_prop_enum_list
drm_tv_subconnector_enum_list[] = {
DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
drm_tv_subconnector_enum_list)
+static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = {
+ { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I, TV-out
and DP */
+ { DRM_MODE_SUBCONNECTOR_VGA, "VGA" }, /* DP */
+ { DRM_MODE_SUBCONNECTOR_DVID,"DVI-D" }, /* DP */
+ { DRM_MODE_SUBCONNECTOR_HDMIA, "HDMI" }, /* DP */
+ { DRM_MODE_SUBCONNECTOR_DisplayPort, "DP"}, /* DP */
+ { DRM_MODE_SUBCONNECTOR_Wireless,"Wireless" }, /* DP */
+ { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
+};
+
+DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
+drm_dp_subconnector_enum_list)
+
static const struct drm_prop_enum_list hdmi_colorspaces[] = {
/* For Default case, driver will set the colorspace */
{ DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
@@ -1186,6 +1199,14 @@ static const struct drm_prop_enum_list dp_colorspaces[]
= {
* can also expose this property to external outputs, in which case they
* must support "None", which should be the default (since external screens
* have a built-in scaler).
+ *
+ * subconnector:
+ * This property is used by DVI-I, TVout and DisplayPort to indicate
different
+ * connector subtypes. Enum values more or less match with those from main
+ * connector types.
+ * For DVI-I and TVout there is also a matching property "select
subconnector"
+ * allowing to switch between signal types.
+ * DP subconnector corresponds to a downstream port.
*/
int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1275,6 +1296,30 @@ int drm_mode_create_dvi_i_properties(struct drm_device
*dev)
EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
/**
+ * drm_mode_add_dp_subconnector_property - create subconn
[PATCH 2/5] drm/i915: utilize subconnector property for DP
From: Oleg Vasilev Since DP-specific information is stored in driver's structures, every driver needs to implement subconnector property by itself. v2: updates to match previous commit changes v3: rebase Cc: Ville Syrjälä Cc: intel-...@lists.freedesktop.org Signed-off-by: Jeevan B Signed-off-by: Oleg Vasilev Reviewed-by: Emil Velikov Tested-by: Oleg Vasilev Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-4-oleg.vasi...@intel.com --- drivers/gpu/drm/i915/display/intel_dp.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index db6ae8e..ba443e1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -6155,6 +6155,11 @@ intel_dp_detect(struct drm_connector *connector, */ intel_display_power_flush_work(dev_priv); + if (!intel_dp_is_edp(intel_dp)) + drm_dp_set_subconnector_property(connector, +status, +intel_dp->dpcd, +intel_dp->downstream_ports); return status; } @@ -7211,6 +7216,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect struct drm_i915_private *dev_priv = to_i915(connector->dev); enum port port = dp_to_dig_port(intel_dp)->base.port; + if (!intel_dp_is_edp(intel_dp)) + drm_mode_add_dp_subconnector_property(connector); + if (!IS_G4X(dev_priv) && port != PORT_A) intel_attach_force_audio_property(connector); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/nouveau: utilize subconnector property for DP
From: Oleg Vasilev
Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself.
v2: rebase
Cc: Ben Skeggs
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Jeevan B
Signed-off-by: Oleg Vasilev
Reviewed-by: Emil Velikov
Link:
https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-5-oleg.vasi...@intel.com
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 13 +
drivers/gpu/drm/nouveau/nouveau_dp.c| 9 +
drivers/gpu/drm/nouveau/nouveau_encoder.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 9a9a7f5..6464e48 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -648,6 +648,17 @@ nouveau_connector_detect(struct drm_connector *connector,
bool force)
pm_runtime_mark_last_busy(dev->dev);
pm_runtime_put_autosuspend(dev->dev);
+ if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+ enum drm_mode_subconnector subconnector =
DRM_MODE_SUBCONNECTOR_Unknown;
+
+ if (conn_status == connector_status_connected && nv_encoder)
+ subconnector = nv_encoder->dp.subconnector;
+ drm_object_property_set_value(&connector->base,
+ connector->dev->mode_config.dp_subconnector_property,
+ subconnector);
+ }
+
return conn_status;
}
@@ -1373,6 +1384,8 @@ nouveau_connector_create(struct drm_device *dev,
kfree(nv_connector);
return ERR_PTR(ret);
}
+
+ drm_mode_add_dp_subconnector_property(connector);
funcs = &nouveau_connector_funcs;
break;
default:
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 2674f15..85eac85 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -62,6 +62,7 @@ nouveau_dp_detect(struct nouveau_encoder *nv_encoder)
struct nouveau_drm *drm = nouveau_drm(dev);
struct nvkm_i2c_aux *aux;
u8 dpcd[8];
+ u8 port_cap[DP_MAX_DOWNSTREAM_PORTS] = {};
int ret;
aux = nv_encoder->aux;
@@ -72,6 +73,14 @@ nouveau_dp_detect(struct nouveau_encoder *nv_encoder)
if (ret)
return ret;
+ if (dpcd[DP_DPCD_REV] > 0x10) {
+ ret = nvkm_rdaux(aux, DP_DOWNSTREAM_PORT_0,
+port_cap, DP_MAX_DOWNSTREAM_PORTS);
+ if (ret)
+ memset(port_cap, 0, DP_MAX_DOWNSTREAM_PORTS);
+ }
+ nv_encoder->dp.subconnector = drm_dp_subconnector_type(dpcd, port_cap);
+
nv_encoder->dp.link_bw = 27000 * dpcd[1];
nv_encoder->dp.link_nr = dpcd[2] & DP_MAX_LANE_COUNT_MASK;
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h
b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 3517f92..e17971a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -63,6 +63,7 @@ struct nouveau_encoder {
struct nv50_mstm *mstm;
int link_nr;
int link_bw;
+ enum drm_mode_subconnector subconnector;
} dp;
};
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/amdgpu: utilize subconnector property for DP through DisplayManager
From: Oleg Vasilev
Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself. Display
Core already has the subconnector information, we only need to
expose it through DRM property.
v2:rebase
Cc: Alex Deucher
Cc: Christian König
Cc: David (ChunMing) Zhou
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Jeevan B
Signed-off-by: Oleg Vasilev
Tested-by: Oleg Vasilev
Link:
https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-7-oleg.vasi...@intel.com
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 +-
.../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 3 ++
2 files changed, 43 insertions(+), 1 deletion(-)
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 d3674d8..91c0ef2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -121,6 +121,42 @@ MODULE_FIRMWARE(FIRMWARE_NAVI12_DMCU);
static int amdgpu_dm_init(struct amdgpu_device *adev);
static void amdgpu_dm_fini(struct amdgpu_device *adev);
+static enum drm_mode_subconnector get_subconnector_type(struct dc_link *link)
+{
+ switch (link->dpcd_caps.dongle_type) {
+ case DISPLAY_DONGLE_NONE:
+ return DRM_MODE_SUBCONNECTOR_Native;
+ case DISPLAY_DONGLE_DP_VGA_CONVERTER:
+ return DRM_MODE_SUBCONNECTOR_VGA;
+ case DISPLAY_DONGLE_DP_DVI_CONVERTER:
+ case DISPLAY_DONGLE_DP_DVI_DONGLE:
+ return DRM_MODE_SUBCONNECTOR_DVID;
+ case DISPLAY_DONGLE_DP_HDMI_CONVERTER:
+ case DISPLAY_DONGLE_DP_HDMI_DONGLE:
+ return DRM_MODE_SUBCONNECTOR_HDMIA;
+ case DISPLAY_DONGLE_DP_HDMI_MISMATCHED_DONGLE:
+ default:
+ return DRM_MODE_SUBCONNECTOR_Unknown;
+ }
+}
+
+static void update_subconnector_property(struct amdgpu_dm_connector
*aconnector)
+{
+ struct dc_link *link = aconnector->dc_link;
+ struct drm_connector *connector = &aconnector->base;
+ enum drm_mode_subconnector subconnector = DRM_MODE_SUBCONNECTOR_Unknown;
+
+ if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
+ return;
+
+ if (aconnector->dc_sink)
+ subconnector = get_subconnector_type(link);
+
+ drm_object_property_set_value(&connector->base,
+ connector->dev->mode_config.dp_subconnector_property,
+ subconnector);
+}
+
/*
* initializes drm_device display related structures, based on the information
* provided by DAL. The drm strcutures are: drm_crtc, drm_connector,
@@ -1917,7 +1953,6 @@ void amdgpu_dm_update_connector_after_detect(
if (aconnector->mst_mgr.mst_state == true)
return;
-
sink = aconnector->dc_link->local_sink;
if (sink)
dc_sink_retain(sink);
@@ -2038,6 +2073,8 @@ void amdgpu_dm_update_connector_after_detect(
mutex_unlock(&dev->mode_config.mutex);
+ update_subconnector_property(aconnector);
+
if (sink)
dc_sink_release(sink);
}
@@ -4518,6 +4555,8 @@ amdgpu_dm_connector_detect(struct drm_connector
*connector, bool force)
else
connected = (aconnector->base.force == DRM_FORCE_ON);
+ update_subconnector_property(aconnector);
+
return (connected ? connector_status_connected :
connector_status_disconnected);
}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index d56b758..267c82c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include "dm_services.h"
#include "amdgpu.h"
#include "amdgpu_dm.h"
@@ -464,6 +465,8 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
16,
4,
aconnector->connector_id);
+
+ drm_mode_add_dp_subconnector_property(&aconnector->base);
}
int dm_mst_get_pbn_divider(struct dc_link *link)
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/amdgpu: utilize subconnector property for DP through atombios
From: Oleg Vasilev
Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself.
v2: rebase
Cc: Alex Deucher
Cc: Christian König
Cc: David (ChunMing) Zhou
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Jeevan B
Signed-off-by: Oleg Vasilev
Reviewed-by: Emil Velikov
Link:
https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-6-oleg.vasi...@intel.com
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 10 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 +
drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 18 +-
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index f355d9a..71aade0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -26,6 +26,7 @@
#include
#include
+#include
#include
#include
#include "amdgpu.h"
@@ -1405,6 +1406,10 @@ amdgpu_connector_dp_detect(struct drm_connector
*connector, bool force)
pm_runtime_put_autosuspend(connector->dev->dev);
}
+ drm_dp_set_subconnector_property(&amdgpu_connector->base,
+ret,
+amdgpu_dig_connector->dpcd,
+
amdgpu_dig_connector->downstream_ports);
return ret;
}
@@ -1951,6 +1956,11 @@ amdgpu_connector_add(struct amdgpu_device *adev,
if (has_aux)
amdgpu_atombios_dp_aux_init(amdgpu_connector);
+ if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector_type == DRM_MODE_CONNECTOR_eDP) {
+ drm_mode_add_dp_subconnector_property(&amdgpu_connector->base);
+ }
+
return;
failed:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 37ba07e..04a430e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -469,6 +469,7 @@ struct amdgpu_encoder {
struct amdgpu_connector_atom_dig {
/* displayport */
u8 dpcd[DP_RECEIVER_CAP_SIZE];
+ u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
u8 dp_sink_type;
int dp_clock;
int dp_lane_count;
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 9b74cfd..900b272 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -328,6 +328,22 @@ static void amdgpu_atombios_dp_probe_oui(struct
amdgpu_connector *amdgpu_connect
buf[0], buf[1], buf[2]);
}
+static void amdgpu_atombios_dp_ds_ports(struct amdgpu_connector
*amdgpu_connector)
+{
+ struct amdgpu_connector_atom_dig *dig_connector =
amdgpu_connector->con_priv;
+ int ret;
+
+ if (dig_connector->dpcd[DP_DPCD_REV] > 0x10) {
+ ret = drm_dp_dpcd_read(&amdgpu_connector->ddc_bus->aux,
+ DP_DOWNSTREAM_PORT_0,
+ dig_connector->downstream_ports,
+ DP_MAX_DOWNSTREAM_PORTS);
+ if (ret)
+ memset(dig_connector->downstream_ports, 0,
+ DP_MAX_DOWNSTREAM_PORTS);
+ }
+}
+
int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
{
struct amdgpu_connector_atom_dig *dig_connector =
amdgpu_connector->con_priv;
@@ -343,7 +359,7 @@ int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector
*amdgpu_connector)
dig_connector->dpcd);
amdgpu_atombios_dp_probe_oui(amdgpu_connector);
-
+ amdgpu_atombios_dp_ds_ports(amdgpu_connector);
return 0;
}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/1] drm/i915: Fix a deadlock that only affects 5.4
On Mon, Apr 06, 2020 at 11:26:21PM -0700, Sultan Alsawaf wrote: > From: Sultan Alsawaf > > Hi, > > There's a mutex lock deadlock in i915 that only affects 5.4, but was fixed in > 5.5. Normally, I would send a backport of the fix from 5.5, but the patch set > that fixes the deadlock involves massive changes that are neither feasible nor > desirable for backporting [1][2][3]. Therefore, I've made a small patch that > only addresses the deadlock specifically for 5.4. This paragraph needs to go into the patch itself, otherwise just looking at that doesn't make any sense. And you do not need a cover letter for a single patch. Please fix up and resend. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/ttm: clean up ttm_trace_dma_map/ttm_trace_dma_unmap
ttm_trace_dma_map/ttm_trace_dma_unmap is never used anymore. Move the pr_fmt prefix into this header. Signed-off-by: Huang Rui --- include/drm/ttm/ttm_debug.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/drm/ttm/ttm_debug.h b/include/drm/ttm/ttm_debug.h index b5e460f..bd7cf37 100644 --- a/include/drm/ttm/ttm_debug.h +++ b/include/drm/ttm/ttm_debug.h @@ -27,5 +27,5 @@ /* * Authors: Tom St Denis */ -extern void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt); -extern void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt); + +#define pr_fmt(fmt) "[TTM] " fmt -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/ttm: abstract all ttm prefix into ttm_debug header
Using the debug header instead of macro at the start of the files. Signed-off-by: Huang Rui --- drivers/gpu/drm/ttm/ttm_agp_backend.c| 3 +-- drivers/gpu/drm/ttm/ttm_bo.c | 3 +-- drivers/gpu/drm/ttm/ttm_bo_vm.c | 3 +-- drivers/gpu/drm/ttm/ttm_memory.c | 3 +-- drivers/gpu/drm/ttm/ttm_page_alloc.c | 3 +-- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 +-- drivers/gpu/drm/ttm/ttm_tt.c | 3 +-- drivers/gpu/drm/vmwgfx/ttm_object.c | 3 +-- 8 files changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c b/drivers/gpu/drm/ttm/ttm_agp_backend.c index 6050dc8..53fa96f 100644 --- a/drivers/gpu/drm/ttm/ttm_agp_backend.c +++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c @@ -30,8 +30,7 @@ * Keith Packard. */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index ca5a8d0..469a3f1 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -29,8 +29,7 @@ * Authors: Thomas Hellstrom */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index eebb4c0..fa5e237 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -29,8 +29,7 @@ * Authors: Thomas Hellstrom */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index acd63b7..f51d70f 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -26,8 +26,7 @@ * **/ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c index b40a467..4363420 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c @@ -31,8 +31,7 @@ * - doesn't track currently in use pages */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index bf876fa..0017d6d 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -33,8 +33,7 @@ * when freed). */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include /* for seq_printf */ diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 2ec448e..4fa8a51 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -29,8 +29,7 @@ * Authors: Thomas Hellstrom */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c b/drivers/gpu/drm/vmwgfx/ttm_object.c index 1607778..cd77370 100644 --- a/drivers/gpu/drm/vmwgfx/ttm_object.c +++ b/drivers/gpu/drm/vmwgfx/ttm_object.c @@ -57,8 +57,7 @@ * for fast lookup of ref objects given a base object. */ -#define pr_fmt(fmt) "[TTM] " fmt - +#include #include #include #include -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm/i915/dp_mst: Cast intel_connector->port as drm_dp_mst_port
On Mon, 06 Apr 2020, Lyude Paul wrote:
> The only reason for having this cast as void * before was because we
> originally needed to use drm_dp_mst_get_port_validated() and friends in
> order to (attempt to) safely access MST ports. However, we've since
> improved how reference counting works with ports and mstbs such that we
> can now rely on drm_dp_mst_port structs remaining in memory for as long
> as the driver needs. This means we don't really need to cast this as
> void* anymore, and can just access the struct directly.
To/from void* does not need a cast in C anyway. ;)
Acked-by: Jani Nikula
>
> We'll also need this for the next commit, so that we can remove
> drm_dp_mst_port_has_audio().
>
> Signed-off-by: Lyude Paul
> Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/i915/display/intel_display_types.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 5a0adf14ebef..0ddc98afe252 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -438,7 +438,7 @@ struct intel_connector {
> state of connector->polled in case hotplug storm detection changes
> it */
> u8 polled;
>
> - void *port; /* store this opaque as its illegal to dereference it */
> + struct drm_dp_mst_port *port;
>
> struct intel_dp *mst_port;
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/17] drm/vram-helper: make drm_vram_mm_debugfs_init() return 0
Hi
Am 19.03.20 um 13:27 schrieb Wambui Karuga:
>
>
> On Thu, 19 Mar 2020, Daniel Vetter wrote:
>
>> On Thu, Mar 19, 2020 at 08:55:24AM +0100, Greg KH wrote:
>>> On Wed, Mar 18, 2020 at 08:10:43PM +0100, Daniel Vetter wrote:
On Wed, Mar 18, 2020 at 5:58 PM Greg KH
wrote:
>
> On Wed, Mar 18, 2020 at 05:31:47PM +0100, Daniel Vetter wrote:
>> On Wed, Mar 18, 2020 at 5:03 PM Wambui Karuga
>> wrote:
>>>
>>>
>>>
>>> On Wed, 18 Mar 2020, Daniel Vetter wrote:
>>>
On Tue, Mar 10, 2020 at 04:31:14PM +0300, Wambui Karuga wrote:
> Since 987d65d01356 (drm: debugfs: make
> drm_debugfs_create_files() never fail),
> drm_debugfs_create_files() never
> fails and should return void. Therefore, remove its use as the
> return value of drm_vram_mm_debugfs_init(), and have the function
> return 0 directly.
>
> v2: have drm_vram_mm_debugfs_init() return 0 instead of void to
> avoid
> introducing build issues and build breakage.
>
> References:
> https://lists.freedesktop.org/archives/dri-devel/2020-February/257183.html
>
> Signed-off-by: Wambui Karuga
> Acked-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 92a11bb42365..c8bcc8609650 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1048,14 +1048,12 @@ static const struct drm_info_list
> drm_vram_mm_debugfs_list[] = {
> */
> int drm_vram_mm_debugfs_init(struct drm_minor *minor)
> {
> - int ret = 0;
> -
> #if defined(CONFIG_DEBUG_FS)
Just noticed that this #if here is not needed, we already have a
dummy
function for that case. Care to write a quick patch to remove
it? On top
of this patch series here ofc, I'm in the processing of merging
the entire
pile.
Thanks, Daniel
>>> Hi Daniel,
>>> Without this check here, and compiling without CONFIG_DEBUG_FS, this
>>> function is run and the drm_debugfs_create_files() does not have
>>> access to
>>> the parameters also protected by an #if above this function. So
>>> the change
>>> throws an error for me. Is that correct?
>>
>> Hm right. Other drivers don't #ifdef out their debugfs file functions
>> ... kinda a bit disappointing that we can't do this in the neatest
>> way
>> possible.
>>
>> Greg, has anyone ever suggested to convert the debugfs_create_file
>> function (and similar things) to macros that don't use any of the
>> arguments, and then also annotating all the static
>> functions/tables as
>> __maybe_unused and let the compiler garbage collect everything?
>> Instead of explicit #ifdef in all the drivers ...
>
> No, no one has suggested that, having the functions be static inline
> should make it all "just work" properly if debugfs is not enabled.
> The
> variables will not be used, so the compiler should just optimize them
> away properly.
>
> No checks for CONFIG_DEBUG_FS should be needed anywhere in .c code.
So the trouble with this one is that the static inline functions for
the debugfs file are wrapped in a #if too, and hence if we drop the
#if around the function call stuff won't compile. Should we drop all
the #if in the .c file and assume the compiler will remove all the
dead code and dead functions?
>>>
>>> Yes you should :)
>>>
>>> there should not be any need for #if in a .c file for debugfs stuff.
>>
>> Wambui, can you pls try that out? I.e. removing all the #if for
>> CONFIG_DEBUG_FS from that file.
>
> Removing them works with CONFIG_DEBUG_FS enabled or disabled.
> I can send a patch for that.
Please do. Removing explicit checks for CONFIG_ is usually a good thing.
Best regards
Thomas
>
> wambui karuga
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/10] Set up generic fbdev after registering device
Hi Sam Am 06.04.20 um 22:00 schrieb Sam Ravnborg: > Hi Thomas. > > On Mon, Apr 06, 2020 at 03:43:55PM +0200, Thomas Zimmermann wrote: >> Generic fbdev emulation is a DRM client. If possible, it should behave >> like userspace clients. Therefore it should not run before the driver >> registered the new DRM device. If the setup function fails, the driver >> should not report an error. > > Thanks for taking the time to refactor all the relevant drivers. > > I have received some push-back in the past when suggesting this, > but cannot remember from who. > Let's see what review comments you get. > > As the rule is that the fbdev setup shall be setup after registering > the DRM device - it would be nice to have this included in the > documentation of drm_fbdev_generic_setup > > Could you try to to update the documentation to cover this? Good idea. I'll add this to patchset's next iteration. Best regards Thomas > > I will get back to the patches later this week. > > Sam > >> >> This is a follow-up patchset to the discussion at [1]. I went >> through all calls to drm_fbdev_generic_setup(), moved them to the >> final operation of their driver's probe function, and removed the >> return value. >> >> Built-tested on x86-64, aarch64 and arm. >> >> [1] >> https://lore.kernel.org/dri-devel/20200403135828.2542770-1-daniel.vet...@ffwll.ch/T/#m216b5b37aeeb7b28d55ad73b7a702b3d1d7bf867 >> >> Thomas Zimmermann (10): >> drm/ast: Set up fbdev after registering device; remove error checks >> drm/hibmc: Remove error check from fbdev setup >> drm/kirin: Set up fbdev after fully registering device >> drm/ingenic: Remove error check from fbdev setup >> drm/mediathek: Remove error check from fbdev setup >> drm/mgag200: Set up fbdev after registering device; remove error >> checks >> drm/tilcdc: Set up fbdev after fully registering device >> drm/udl: Remove error check from fbdev setup >> drm/vboxvideo: Set up fbdev after registering device; remove error >> checks >> drm/fb-helper: Remove return value from drm_fbdev_generic_setup() >> >> drivers/gpu/drm/ast/ast_drv.c | 3 +++ >> drivers/gpu/drm/ast/ast_main.c | 5 - >> drivers/gpu/drm/drm_fb_helper.c| 18 -- >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c| 6 +- >> .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c| 4 ++-- >> drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +--- >> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +--- >> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ >> drivers/gpu/drm/mgag200/mgag200_main.c | 4 >> drivers/gpu/drm/tilcdc/tilcdc_drv.c| 3 +-- >> drivers/gpu/drm/udl/udl_drv.c | 6 +- >> drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++ >> include/drm/drm_fb_helper.h| 5 +++-- >> 13 files changed, 25 insertions(+), 45 deletions(-) >> >> -- >> 2.26.0 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/vblank: Add intro to documentation
Am 06.04.20 um 21:47 schrieb Sam Ravnborg:
> Lyude Paul wrote a very good intro to vblank here:
> https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.ca...@redhat.com/T/#mce6480be738160e9d07c5d023e88fd78d7a06d27
>
> Add this to the intro chapter in drm_vblank.c so others
> can benefit from it too.
>
> v2:
> - Reworded to improve readability (Thomas)
>
> v3:
> - Added nice ascii drawing from Lyude (Lyude)
> - Added referende to high-precision timestamp (Daniel)
> - Improved grammar (Thomas)
> - Combined it all and made kernel-doc happy
> - Dropped any a-b, r-b do to the amount of changes
>
> Signed-off-by: Sam Ravnborg
> Co-developed-by: Lyude Paul
> Cc: Lyude Paul
> Cc: Thomas Zimmermann
> Cc: Daniel Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
Acked-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_vblank.c | 53
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..9633092c9ad5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,59 @@
> /**
> * DOC: vblank handling
> *
> + * From the computer's perspective, every time the monitor displays
> + * a new frame the scanout engine have "scanned out" the display image
> + * from top to bottom, one row of pixels at a time.
> + * The current row of pixels is referred to as the current scanline.
> + *
> + * In addition to the display's visible area, there's usually a couple of
> + * extra scanlines which aren't actually displayed on the screen.
> + * These extra scanlines don't contain image data and are occasionally used
> + * for features like audio and infoframes. The region made up of these
> + * scanlines is referred to as the vertical blanking region, or vblank for
> + * short.
> + *
> + * ::
> + *
> + *
> + *physical →
> + *top of ||
> + *display ||
> + *| New frame|
> + *||
> + *||
> + *|| ← Scanline,
> updates
> + *|| the frame as
> it
> + *|| travels down
> + *|| ("scan out")
> + *||
> + *| Old frame|
> + *||
> + *||
> + *||
> + *|| physical
> + *|| bottom of
> + *vertical|| ← display
> + *blanking┆┆
> + *region → ┆┆
> + *┆┆
> + *start of →
> + *new frame
> + *
> + * "Physical top of display" is the reference point for the high-precision/
> + * corrected timestamp.
> + *
> + * On a lot of display hardware, programming needs to take effect during the
> + * vertical blanking period so that settings like gamma, the image buffer
> + * buffer to be scanned out, etc. can safely be changed without showing
> + * any visual artifacts on the screen. In some unforgiving hardware, some of
> + * this programming has to both start and end in the same vblank.
> + *
> + * The vblank interrupt may be fired at different points depending on the
> + * hardware. Some hardware implementations will fire the interrupt when the
> + * new frame start, other implementations will fire the interrupt at
> different
> + * points in time.
> + *
> * Vertical blanking plays a major role in graphics rendering. To achieve
> * tear-free display, users must synchronize page flips and/or rendering to
> * vertical blanking. The DRM API offers ioctls to perform page flips
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] nouveau-fixes 5.7
Hey Dave, A couple of misc fixes/workarounds for some issues that are causing a lot of pain for people. Of most interest are the PCI power management and GR init WARs, which effect a rather significant number of laptop systems that are in use today. Ben. The following changes since commit 0e7e6198af28c1573267aba1be33dd0b7fb35691: Merge branch 'ttm-transhuge' of git://people.freedesktop.org/~thomash/linux into drm-next (2020-04-03 09:07:49 +1000) are available in the Git repository at: git://github.com/skeggsb/linux linux-5.7 for you to fetch changes up to 374b55802cd567e9f340b7f27d2c5e20b069ac3c: drm/nouveau/kms/nv50-: wait for FIFO space on PIO channels (2020-04-07 14:37:50 +1000) Ben Skeggs (5): drm/nouveau/ttm: evict other IO mappings when running out of BAR1 space drm/nouveau/gr/gp107,gp108: implement workaround for HW hanging during init drm/nouveau/nvif: access PTIMER through usermode class, if available drm/nouveau/nvif: protect waits against GPU falling off the bus drm/nouveau/kms/nv50-: wait for FIFO space on PIO channels Karol Herbst (1): drm/nouveau: workaround runpm fail by disabling PCI power management on certain intel bridges Ralph Campbell (3): drm/nouveau/svm: fix vma range check for migration drm/nouveau/svm: check for SVM initialized before migrating drm/nouveau/svm: remove useless SVM range check Wambui Karuga (1): drm/nouveau: remove checks for return value of debugfs functions drivers/gpu/drm/nouveau/dispnv04/dac.c | 3 ++- drivers/gpu/drm/nouveau/dispnv04/hw.c | 1 + drivers/gpu/drm/nouveau/dispnv50/base507c.c| 1 + drivers/gpu/drm/nouveau/dispnv50/core507d.c| 1 + drivers/gpu/drm/nouveau/dispnv50/corec37d.c| 2 ++ drivers/gpu/drm/nouveau/dispnv50/curs507a.c| 21 ++--- drivers/gpu/drm/nouveau/dispnv50/cursc37a.c| 9 ++--- drivers/gpu/drm/nouveau/dispnv50/disp.c| 1 + drivers/gpu/drm/nouveau/dispnv50/ovly827e.c| 2 ++ drivers/gpu/drm/nouveau/dispnv50/wndw.h| 1 + drivers/gpu/drm/nouveau/include/nvif/device.h | 21 - drivers/gpu/drm/nouveau/include/nvif/timer.h | 35 +++ drivers/gpu/drm/nouveau/include/nvif/user.h| 1 + drivers/gpu/drm/nouveau/nouveau_bo.c | 9 +++-- drivers/gpu/drm/nouveau/nouveau_debugfs.c | 20 drivers/gpu/drm/nouveau/nouveau_drm.c | 63 +++ drivers/gpu/drm/nouveau/nouveau_drv.h | 2 ++ drivers/gpu/drm/nouveau/nouveau_svm.c | 9 ++--- drivers/gpu/drm/nouveau/nvif/Kbuild| 1 + drivers/gpu/drm/nouveau/nvif/device.c | 14 +- drivers/gpu/drm/nouveau/nvif/timer.c | 56 drivers/gpu/drm/nouveau/nvif/userc361.c| 14 ++ drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c | 26 ++ 23 files changed, 263 insertions(+), 50 deletions(-) create mode 100644 drivers/gpu/drm/nouveau/include/nvif/timer.h create mode 100644 drivers/gpu/drm/nouveau/nvif/timer.c ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [DPU PATCH v5 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
Hi Sam, Thanks for reviews. The changes were posted by Vara Reddy. Due to some configuration errors, changes were posted with my E-mail ID. Vara will be addressing comments, and we will take care of this error with next patchset. Thanks, Tanmay -Original Message- From: Sam Ravnborg Sent: Tuesday, March 31, 2020 10:50 PM To: Tanmay Shah Cc: freedr...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; devicet...@vger.kernel.org; seanp...@chromium.org; swb...@chromium.org; abhin...@codeaurora.org; hoegsb...@google.com; dri-devel@lists.freedesktop.org; Vara Reddy ; aravi...@codeaurora.org; linux-...@vger.kernel.org; Chandan Uddaraju Subject: Re: [DPU PATCH v5 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Hi Tanmay Reviewing the yaml bindings triggered a few comments. See below. Sam On Tue, Mar 31, 2020 at 05:30:27PM -0700, Tanmay Shah wrote: > From: Chandan Uddaraju > > Add bindings for Snapdragon DisplayPort and display-port PLL driver. > > Changes in V2: > Provide details about sel-gpio > > Changes in V4: > Provide details about max dp lanes > Change the commit text > > Changes in V5: > Moved dp.txt to yaml file. > > Signed-off-by: Chandan Uddaraju > Signed-off-by: Vara Reddy As you handle the patch, thus the patch passed throgh you, you are supposed to sign-off the patch. The changes to dpu.txt is not explained in the changelog. > --- > .../devicetree/bindings/display/msm/dp-sc7180.yaml | 325 + > .../devicetree/bindings/display/msm/dpu.txt| 16 +- > 2 files changed, 337 insertions(+), 4 deletions(-) create mode > 100644 Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > > diff --git > a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > new file mode 100644 > index 000..761a01d > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > @@ -0,0 +1,325 @@ > +# SPDX-License-Identifier: GPL-2.0-only For new bindings please use: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Description of Qualcomm Display Port dt properties. > + > +maintainers: > + - Chandan Uddaraju > + - Vara Reddy > + > +description: | > + Device tree bindings for MSM Display Port which supports DP host > +controllers > + that are compatible with VESA Display Port interface specification. > + > +properties: > + "msm_dp": The quotes seems not necessary. This describes the name of the node. The typical way to identify a node is using a compatible. So I think that the right solution here is to drop "msm_dp". > +type: object > +description: | > + Node containing Display port register address bases, clocks, power supplies. > + And start here. > +properties: > + compatible: > + items: > + - const: qcom,dp-display > + > + cell-index: > + description: Specifies the controller instance. > + > + reg: > + description: Physical base address and length of controller's registers. This description is generic and can be omitted. But it would be good with a descrition of the individual registers like this: reg: items: - description: AHB bla bla - description: aux bla bla > + > + reg-names: > + description: | > + Names for different register regions defined above. The required region > + is mentioned below. > + items: > + - const: dp_ahb > + - const: dp_aux > + - const: dp_link > + - const: dp_p0 > + - const: dp_phy > + - const: dp_ln_tx0 > + - const: dp_ln_tx1 > + - const: afprom_physical > + - const: dp_pll > + - const: usb3_dp_com > + - const: hdcp_physical > + > + interrupts: > + description: The interrupt signal from the DP block. > + > + clocks: > + description: List of clock specifiers for clocks needed by the device. items: - description: aux clock bla bla - description: ref clock bla bla > + > + clock-names: > + description: | > + Device clock names in the same order as mentioned in clocks property. > + The required clocks are mentioned below. > + items: > + - const: core_aux_clk > + - const: core_ref_clk_src > + - const: core_usb_ref_clk > + - const: core_usb_cfg_ahb_clk > + - const: core_usb_pipe_clk > + - const: ctrl_link_clk > + - const: ctrl_link_iface_clk > + - const: ctrl_pixel_clk > + - const: crypto_clk > + - const: pixel_clk_rcg > + > + pll-node: > + description: phandle to DP PLL node. Add type (phandle) > + > + vdda-1p2-supply: > + description: phandle to vdda 1.2V regulator n
Re: [PATCH v2 16/17] drm: Nuke mode->private_flags
Hi Jani On 2020-04-06 02:11, Jani Nikula wrote: On Fri, 03 Apr 2020, abhin...@codeaurora.org wrote: Hi Ville Thanks for the patch. Our understanding of private_flags was that we can use it within our drivers to handle vendor specific features. Hence we do have several features in our downstream drivers as well as some planned work based on this. This was the only method to pass around and consume the driver only information which we have been using. In the current qualcomm upstream display drivers, the only usage of the mode->private_flags is what you have removed in https://patchwork.kernel.org/patch/11473497/. However, for other projects which do not use upstream drivers yet, we have several features already which are using the mode->private_flags. We do have a plan to remove the usage of mode->private_flags for those drivers as well but its not ready yet. These downstream drivers still use the upstream drm files for compilation. So how it works is we use all the headers under include/drm and also the files under drivers/gpu/drm as-it-is from upstream but maintain our drivers on top of this. Removing this will result in compilation failures for us in the near term. Can we keep this one as-it-is and when our changes are ready to post it upstream we shall remove private_flags from the drm_modes.h If your driver were upstream, Ville would have fixed it in the process of removing private_flags. It would be part of this patch series. That is the only guarantee you get for kernel internal APIs, and you only get that guarantee for drivers in the upstream kernel. Otherwise, all bets are off. Taking all the upstream considerations into account is complicated enough. It is simply not reasonable to hold back internal kernel changes due to out-of-tree or downstream drivers. I know it is painful, but that's the cost of maintaining a driver out-of-tree. Sorry, but no. Further reading [1]. BR, Jani. [1] https://www.kernel.org/doc/html/latest/process/stable-api-nonsense.html Thanks for the response. We will plan to remove mode->private_flags in our downstream driver as well. Abhinav ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/17] drm: Nuke mode->vrefresh
Hi Jani
On 2020-04-06 01:32, Jani Nikula wrote:
On Fri, 03 Apr 2020, abhin...@codeaurora.org wrote:
On 2020-04-03 13:39, Ville Syrjala wrote:
diff --git a/drivers/gpu/drm/drm_modes.c
b/drivers/gpu/drm/drm_modes.c
index fec1c33b3045..e3d5f011f7bd 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -759,9 +759,7 @@ int drm_mode_vrefresh(const struct
drm_display_mode
*mode)
{
int refresh = 0;
- if (mode->vrefresh > 0)
- refresh = mode->vrefresh;
The mode->vrefresh has been replaced with calling this API in all its
usages.
However in this API, the above if statement was returning the vrefresh
if it was already
set. mode->clock is holding the pixel clock . So this will not cause
any
issues in non-compressed cases.
In case of compression like DSC, the pixel
clock will be different based on the compression ratio hence the
mode->clock will change but fps will not.
So we did have usages in our downstream driver where we would use this
API and the refresh rate
returned will be the mode->vrefresh which did not change but after
this
change for those cases it will end up returning the refresh rate
calculated using mode->clock which will result in a different value
now.
So is the recommendation that even in the case of compression
mode->clock should always hold
uncompressed pixel clock value because with this part of the change we
will now get a different value when we call this API.
Yes. The mode remains the same regardless of compression, and
compression is just an implementation detail of the transport.
You may need to maintain separate "physical port clock" and "logical
port clock" for DSC, where the latter is a function of the former and
the DSC parameters. And then you can see if your logical port clock
provides enough bandwidth for your mode. But this is up to your driver
and encoder implementation.
BR,
Jani.
Thanks for the information. We will make changes to our driver to
accommodate the changes in the
drm_mode_vrefresh API.
Thanks
Abhinav
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 05/19] virtgpu: pull in uaccess.h
In preparation to virtio header changes, include uaccess.h directly as this file is using copy to/from user. Signed-off-by: Michael S. Tsirkin --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 205ec4abae2b..289dabbce477 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -27,6 +27,7 @@ #include #include +#include #include #include -- MST ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 05/19] virtgpu: pull in uaccess.h
In preparation to virtio header changes, include uaccess.h directly as this file is using copy to/from user. Signed-off-by: Michael S. Tsirkin --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 205ec4abae2b..2342a8baa5f8 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -27,6 +27,7 @@ #include #include +#include #include #include -- MST ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.4 28/32] drm/scheduler: fix rare NULL ptr race
From: Yintian Tao [ Upstream commit 77bb2f204f1f0a53a602a8fd15816d6826212077 ] There is one one corner case at dma_fence_signal_locked which will raise the NULL pointer problem just like below. ->dma_fence_signal ->dma_fence_signal_locked ->test_and_set_bit here trigger dma_fence_release happen due to the zero of fence refcount. ->dma_fence_put ->dma_fence_release ->drm_sched_fence_release_scheduled ->call_rcu here make the union fled “cb_list” at finished fence to NULL because struct rcu_head contains two pointer which is same as struct list_head cb_list Therefore, to hold the reference of finished fence at drm_sched_process_job to prevent the null pointer during finished fence dma_fence_signal [ 732.912867] BUG: kernel NULL pointer dereference, address: 0008 [ 732.914815] #PF: supervisor write access in kernel mode [ 732.915731] #PF: error_code(0x0002) - not-present page [ 732.916621] PGD 0 P4D 0 [ 732.917072] Oops: 0002 [#1] SMP PTI [ 732.917682] CPU: 7 PID: 0 Comm: swapper/7 Tainted: G OE 5.4.0-rc7 #1 [ 732.918980] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 [ 732.920906] RIP: 0010:dma_fence_signal_locked+0x3e/0x100 [ 732.938569] Call Trace: [ 732.939003] [ 732.939364] dma_fence_signal+0x29/0x50 [ 732.940036] drm_sched_fence_finished+0x12/0x20 [gpu_sched] [ 732.940996] drm_sched_process_job+0x34/0xa0 [gpu_sched] [ 732.941910] dma_fence_signal_locked+0x85/0x100 [ 732.942692] dma_fence_signal+0x29/0x50 [ 732.943457] amdgpu_fence_process+0x99/0x120 [amdgpu] [ 732.944393] sdma_v4_0_process_trap_irq+0x81/0xa0 [amdgpu] v2: hold the finished fence at drm_sched_process_job instead of amdgpu_fence_process v3: resume the blank line Signed-off-by: Yintian Tao Reviewed-by: Christian König Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/scheduler/sched_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 2af64459b3d77..dfb29e6eeff1e 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -627,7 +627,9 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb) trace_drm_sched_process_job(s_fence); + dma_fence_get(&s_fence->finished); drm_sched_fence_finished(s_fence); + dma_fence_put(&s_fence->finished); wake_up_interruptible(&sched->wake_up_worker); } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.5 31/35] drm/scheduler: fix rare NULL ptr race
From: Yintian Tao [ Upstream commit 3c0fdf3302cb4f186c871684eac5c407a107e480 ] There is one one corner case at dma_fence_signal_locked which will raise the NULL pointer problem just like below. ->dma_fence_signal ->dma_fence_signal_locked ->test_and_set_bit here trigger dma_fence_release happen due to the zero of fence refcount. ->dma_fence_put ->dma_fence_release ->drm_sched_fence_release_scheduled ->call_rcu here make the union fled “cb_list” at finished fence to NULL because struct rcu_head contains two pointer which is same as struct list_head cb_list Therefore, to hold the reference of finished fence at drm_sched_process_job to prevent the null pointer during finished fence dma_fence_signal [ 732.912867] BUG: kernel NULL pointer dereference, address: 0008 [ 732.914815] #PF: supervisor write access in kernel mode [ 732.915731] #PF: error_code(0x0002) - not-present page [ 732.916621] PGD 0 P4D 0 [ 732.917072] Oops: 0002 [#1] SMP PTI [ 732.917682] CPU: 7 PID: 0 Comm: swapper/7 Tainted: G OE 5.4.0-rc7 #1 [ 732.918980] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 [ 732.920906] RIP: 0010:dma_fence_signal_locked+0x3e/0x100 [ 732.938569] Call Trace: [ 732.939003] [ 732.939364] dma_fence_signal+0x29/0x50 [ 732.940036] drm_sched_fence_finished+0x12/0x20 [gpu_sched] [ 732.940996] drm_sched_process_job+0x34/0xa0 [gpu_sched] [ 732.941910] dma_fence_signal_locked+0x85/0x100 [ 732.942692] dma_fence_signal+0x29/0x50 [ 732.943457] amdgpu_fence_process+0x99/0x120 [amdgpu] [ 732.944393] sdma_v4_0_process_trap_irq+0x81/0xa0 [amdgpu] v2: hold the finished fence at drm_sched_process_job instead of amdgpu_fence_process v3: resume the blank line Signed-off-by: Yintian Tao Reviewed-by: Christian König Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/scheduler/sched_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 3c57e84222ca9..5bb9feddbfd6b 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -632,7 +632,9 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb) trace_drm_sched_process_job(s_fence); + dma_fence_get(&s_fence->finished); drm_sched_fence_finished(s_fence); + dma_fence_put(&s_fence->finished); wake_up_interruptible(&sched->wake_up_worker); } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/vblank: Add intro to documentation
On Mon, 2020-04-06 at 21:47 +0200, Sam Ravnborg wrote:
> Lyude Paul wrote a very good intro to vblank here:
> https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.ca...@redhat.com/T/#mce6480be738160e9d07c5d023e88fd78d7a06d27
>
> Add this to the intro chapter in drm_vblank.c so others
> can benefit from it too.
>
> v2:
> - Reworded to improve readability (Thomas)
>
> v3:
> - Added nice ascii drawing from Lyude (Lyude)
> - Added referende to high-precision timestamp (Daniel)
> - Improved grammar (Thomas)
> - Combined it all and made kernel-doc happy
> - Dropped any a-b, r-b do to the amount of changes
>
> Signed-off-by: Sam Ravnborg
> Co-developed-by: Lyude Paul
> Cc: Lyude Paul
> Cc: Thomas Zimmermann
> Cc: Daniel Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> ---
> drivers/gpu/drm/drm_vblank.c | 53
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..9633092c9ad5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,59 @@
> /**
> * DOC: vblank handling
> *
> + * From the computer's perspective, every time the monitor displays
> + * a new frame the scanout engine have "scanned out" the display image
> + * from top to bottom, one row of pixels at a time.
s/have/has/
Other then that:
Reviewed-by: Lyude Paul
> + * The current row of pixels is referred to as the current scanline.
> + *
> + * In addition to the display's visible area, there's usually a couple of
> + * extra scanlines which aren't actually displayed on the screen.
> + * These extra scanlines don't contain image data and are occasionally used
> + * for features like audio and infoframes. The region made up of these
> + * scanlines is referred to as the vertical blanking region, or vblank for
> + * short.
> + *
> + * ::
> + *
> + *
> + *physical →
> + *top of ||
> + *display ||
> + *| New frame|
> + *||
> + *||
> + *|| ← Scanline,
> updates
> + *|| the frame as
> it
> + *|| travels down
> + *|| ("scan out")
> + *||
> + *| Old frame|
> + *||
> + *||
> + *||
> + *|| physical
> + *|| bottom of
> + *vertical|| ← display
> + *blanking┆┆
> + *region → ┆┆
> + *┆┆
> + *start of →
> + *new frame
> + *
> + * "Physical top of display" is the reference point for the high-precision/
> + * corrected timestamp.
> + *
> + * On a lot of display hardware, programming needs to take effect during
> the
> + * vertical blanking period so that settings like gamma, the image buffer
> + * buffer to be scanned out, etc. can safely be changed without showing
> + * any visual artifacts on the screen. In some unforgiving hardware, some
> of
> + * this programming has to both start and end in the same vblank.
> + *
> + * The vblank interrupt may be fired at different points depending on the
> + * hardware. Some hardware implementations will fire the interrupt when the
> + * new frame start, other implementations will fire the interrupt at
> different
> + * points in time.
> + *
> * Vertical blanking plays a major role in graphics rendering. To achieve
> * tear-free display, users must synchronize page flips and/or rendering to
> * vertical blanking. The DRM API offers ioctls to perform page flips
--
Cheers,
Lyude Paul (she/her)
Associate Software Engineer at Red Hat
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/4] drm/dp_mst: Improve kdocs for drm_dp_check_act_status()
No functional changes.
Signed-off-by: Lyude Paul
Cc: Sean Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5b205aea58d4..828ca63cc576 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4462,10 +4462,14 @@ static int drm_dp_dpcd_write_payload(struct
drm_dp_mst_topology_mgr *mgr,
/**
- * drm_dp_check_act_status() - Check ACT handled status.
+ * drm_dp_check_act_status() - Polls for ACT handled status.
* @mgr: manager to use
*
- * Check the payload status bits in the DPCD for ACT handled completion.
+ * Tries waiting for the MST hub to finish updating it's payload table by
+ * polling for the ACT handled bit.
+ *
+ * Returns:
+ * 0 if the ACT was handled in time, negative error code on failure.
*/
int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
{
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] drm/dp_mst: Increase ACT retry timeout to 3s
Currently we only poll for an ACT up to 30 times, with a busy-wait delay
of 100µs between each attempt - giving us a timeout of 2900µs. While
this might seem sensible, it would appear that in certain scenarios it
can take dramatically longer then that for us to receive an ACT. On one
of the EVGA MST hubs that I have available, I observed said hub
sometimes taking longer then a second before signalling the ACT. These
delays mostly seem to occur when previous sideband messages we've sent
are NAKd by the hub, however it wouldn't be particularly surprising if
it's possible to reproduce times like this simply by introducing branch
devices with large LCTs since payload allocations have to take effect on
every downstream device up to the payload's target.
So, instead of just retrying 30 times we poll for the ACT for up to 3ms,
and additionally use usleep_range() to avoid a very long and rude
busy-wait. Note that the previous retry count of 30 appears to have been
arbitrarily chosen, as I can't find any mention of a recommended timeout
or retry count for ACTs in the DisplayPort 2.0 specification. This also
goes for the range we were previously using for udelay(), although I
suspect that was just copied from the recommended delay for link
training on SST devices.
Changes since v1:
* Use readx_poll_timeout() instead of open-coding timeout loop - Sean
Paul
Signed-off-by: Lyude Paul
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: Sean Paul
Cc: # v3.17+
---
drivers/gpu/drm/drm_dp_mst_topology.c | 57 ---
1 file changed, 34 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index c83adbdfc1cd..ce61964baa7c 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS)
#include
@@ -4460,43 +4461,53 @@ static int drm_dp_dpcd_write_payload(struct
drm_dp_mst_topology_mgr *mgr,
return ret;
}
+static int do_get_act_status(struct drm_dp_aux *aux)
+{
+ int ret;
+ u8 status;
+
+ ret = drm_dp_dpcd_readb(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
+ if (ret < 0)
+ return ret;
+
+ return status;
+}
/**
* drm_dp_check_act_status() - Polls for ACT handled status.
* @mgr: manager to use
*
* Tries waiting for the MST hub to finish updating it's payload table by
- * polling for the ACT handled bit.
+ * polling for the ACT handled bit for up to 3 seconds (yes-some hubs really
+ * take that long).
*
* Returns:
* 0 if the ACT was handled in time, negative error code on failure.
*/
int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
{
- int count = 0, ret;
- u8 status;
-
- do {
- ret = drm_dp_dpcd_readb(mgr->aux,
- DP_PAYLOAD_TABLE_UPDATE_STATUS,
- &status);
- if (ret < 0) {
- DRM_DEBUG_KMS("failed to read payload table status
%d\n",
- ret);
- return ret;
- }
-
- if (status & DP_PAYLOAD_ACT_HANDLED)
- break;
- count++;
- udelay(100);
- } while (count < 30);
-
- if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
- DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
- status, count);
+ /*
+* There doesn't seem to be any recommended retry count or timeout in
+* the MST specification. Since some hubs have been observed to take
+* over 1 second to update their payload allocations under certain
+* conditions, we use a rather large timeout value.
+*/
+ const int timeout_ms = 3000;
+ int ret, status;
+
+ ret = readx_poll_timeout(do_get_act_status, mgr->aux, status,
+status & DP_PAYLOAD_ACT_HANDLED || status < 0,
+100, timeout_ms * USEC_PER_MSEC);
+ if (ret < 0 && status >= 0) {
+ DRM_DEBUG_KMS("Failed to get ACT bit %d after %dms\n",
+ status, timeout_ms);
return -EINVAL;
+ } else if (status < 0) {
+ DRM_DEBUG_KMS("Failed to read payload table status: %d\n",
+ status);
+ return status;
}
+
return 0;
}
EXPORT_SYMBOL(drm_dp_check_act_status);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/4] drm/dp_mst: drm_dp_check_act_status() fixes
Noticed this while fixing some unrelated issues with NAKs being dropped - we don't wait nearly long enough to receive ACTs from MST hubs in some situations. Also, we take the time to refactor this function a bit. This fixes some ACT timeouts I observed on an EVGA MST hub with i915. Lyude Paul (4): drm/dp_mst: Improve kdocs for drm_dp_check_act_status() drm/dp_mst: Reformat drm_dp_check_act_status() a bit drm/dp_mst: Increase ACT retry timeout to 3s drm/dp_mst: Print errors on ACT timeouts drivers/gpu/drm/drm_dp_mst_topology.c | 71 +-- 1 file changed, 44 insertions(+), 27 deletions(-) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/4] drm/dp_mst: Reformat drm_dp_check_act_status() a bit
Just add a bit more line wrapping, get rid of some extraneous
whitespace, remove an unneeded goto label, and move around some variable
declarations. No functional changes here.
Signed-off-by: Lyude Paul
[this isn't a fix, but it's needed for the fix that comes after this]
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: Sean Paul
Cc: # v3.17+
---
drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 828ca63cc576..c83adbdfc1cd 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4473,33 +4473,31 @@ static int drm_dp_dpcd_write_payload(struct
drm_dp_mst_topology_mgr *mgr,
*/
int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
{
+ int count = 0, ret;
u8 status;
- int ret;
- int count = 0;
do {
- ret = drm_dp_dpcd_readb(mgr->aux,
DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
-
+ ret = drm_dp_dpcd_readb(mgr->aux,
+ DP_PAYLOAD_TABLE_UPDATE_STATUS,
+ &status);
if (ret < 0) {
- DRM_DEBUG_KMS("failed to read payload table status
%d\n", ret);
- goto fail;
+ DRM_DEBUG_KMS("failed to read payload table status
%d\n",
+ ret);
+ return ret;
}
if (status & DP_PAYLOAD_ACT_HANDLED)
break;
count++;
udelay(100);
-
} while (count < 30);
if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
- DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
status, count);
- ret = -EINVAL;
- goto fail;
+ DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
+ status, count);
+ return -EINVAL;
}
return 0;
-fail:
- return ret;
}
EXPORT_SYMBOL(drm_dp_check_act_status);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] drm/dp_mst: Print errors on ACT timeouts
Although it's not unexpected for drm_dp_check_act_status() to fail due
to DPCD read failures (as the hub may have just been unplugged
suddenly), timeouts are a bit more worrying as they either mean we need
a longer timeout value, or we aren't setting up payload allocations
properly. So, let's start printing errors on timeouts.
Signed-off-by: Lyude Paul
Cc: Sean Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index ce61964baa7c..0cbeb0f5c834 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4499,10 +4499,14 @@ int drm_dp_check_act_status(struct
drm_dp_mst_topology_mgr *mgr)
status & DP_PAYLOAD_ACT_HANDLED || status < 0,
100, timeout_ms * USEC_PER_MSEC);
if (ret < 0 && status >= 0) {
- DRM_DEBUG_KMS("Failed to get ACT bit %d after %dms\n",
- status, timeout_ms);
+ DRM_ERROR("Failed to get ACT after %dms, last status: %02x\n",
+ timeout_ms, status);
return -EINVAL;
} else if (status < 0) {
+ /*
+* Failure here isn't unexpected - the hub may have
+* just been unplugged
+*/
DRM_DEBUG_KMS("Failed to read payload table status: %d\n",
status);
return status;
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/dp_mst: Reformat drm_dp_check_act_status() a bit
On Mon, 2020-04-06 at 15:23 -0400, Sean Paul wrote:
> On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
> > Just add a bit more line wrapping, get rid of some extraneous
> > whitespace, remove an unneeded goto label, and move around some variable
> > declarations. No functional changes here.
> >
> > Signed-off-by: Lyude Paul
> > [this isn't a fix, but it's needed for the fix that comes after this]
> > Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper
> > (v0.6)")
> > Cc: Sean Paul
> > Cc: # v3.17+
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++
> > 1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 2b9ce965f044..7aaf184a2e5f 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4473,33 +4473,31 @@ static int drm_dp_dpcd_write_payload(struct
> > drm_dp_mst_topology_mgr *mgr,
> > */
> > int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> > {
> > + int count = 0, ret;
> > u8 status;
> > - int ret;
> > - int count = 0;
> >
> > do {
> > - ret = drm_dp_dpcd_readb(mgr->aux,
> > DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
> > -
> > + ret = drm_dp_dpcd_readb(mgr->aux,
> > + DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > + &status);
> > if (ret < 0) {
> > - DRM_DEBUG_KMS("failed to read payload table status
> > %d\n", ret);
> > - goto fail;
> > + DRM_DEBUG_KMS("failed to read payload table status
> > %d\n",
> > + ret);
> > + return ret;
> > }
> >
> > if (status & DP_PAYLOAD_ACT_HANDLED)
> > break;
> > count++;
> > udelay(100);
> > -
> > } while (count < 30);
> >
> > if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> > - DRM_DEBUG_KMS("failed to get ACT bit %d after %d
> > retries\n", status, count);
> > - ret = -EINVAL;
> > - goto fail;
> > + DRM_DEBUG_KMS("failed to get ACT bit %d after %d
> > retries\n",
>
> Should we print status in base16 here?
jfyi - I realized we don't actually need to do this, because we do this in the
next patch whoops. Just figured I'd point that out
>
> Otherwise:
>
> Reviewed-by: Sean Paul
>
> > + status, count);
> > + return -EINVAL;
> > }
> > return 0;
> > -fail:
> > - return ret;
> > }
> > EXPORT_SYMBOL(drm_dp_check_act_status);
> >
> > --
> > 2.25.1
> >
--
Cheers,
Lyude Paul (she/her)
Associate Software Engineer at Red Hat
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Bad rss-counter state from drm/ttm, drm/vmwgfx: Support huge TTM pagefaults
Hi,
On 4/6/20 9:51 PM, Alex Xu (Hello71) wrote:
Using 314b658 with amdgpu, starting sway and firefox causes "BUG: Bad
rss-counter state" and "BUG: non-zero pgtables_bytes on freeing mm" to
start filling dmesg, and then closing programs causes more BUGs and
hangs, and then everything grinds to a halt (can't start more programs,
can't even reboot through systemd).
Using master and reverting that branch up to that point fixes the
problem.
I'm using a Ryzen 1600 and AMD Radeon RX 480 on an ASRock B450 Pro4
board with IOMMU enabled.
If you could try the attached patch, that'd be great!
Thanks,
Thomas
>From b630b9b4dcc1d01514d97a84cbb7f0cb85333154 Mon Sep 17 00:00:00 2001
From: "Thomas Hellstrom (VMware)"
Date: Mon, 6 Apr 2020 22:55:13 +0200
Subject: [PATCH] drm/ttm: Temporarily disable the huge_fault() callback
Signed-off-by: Thomas Hellstrom (VMware)
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 63 -
1 file changed, 63 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 6ee3b96f0d13..0ad30b112982 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -442,66 +442,6 @@ vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
}
EXPORT_SYMBOL(ttm_bo_vm_fault);
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-/**
- * ttm_pgprot_is_wrprotecting - Is a page protection value write-protecting?
- * @prot: The page protection value
- *
- * Return: true if @prot is write-protecting. false otherwise.
- */
-static bool ttm_pgprot_is_wrprotecting(pgprot_t prot)
-{
- /*
- * This is meant to say "pgprot_wrprotect(prot) == prot" in a generic
- * way. Unfortunately there is no generic pgprot_wrprotect.
- */
- return pte_val(pte_wrprotect(__pte(pgprot_val(prot ==
- pgprot_val(prot);
-}
-
-static vm_fault_t ttm_bo_vm_huge_fault(struct vm_fault *vmf,
- enum page_entry_size pe_size)
-{
- struct vm_area_struct *vma = vmf->vma;
- pgprot_t prot;
- struct ttm_buffer_object *bo = vma->vm_private_data;
- vm_fault_t ret;
- pgoff_t fault_page_size = 0;
- bool write = vmf->flags & FAULT_FLAG_WRITE;
-
- switch (pe_size) {
- case PE_SIZE_PMD:
- fault_page_size = HPAGE_PMD_SIZE >> PAGE_SHIFT;
- break;
-#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
- case PE_SIZE_PUD:
- fault_page_size = HPAGE_PUD_SIZE >> PAGE_SHIFT;
- break;
-#endif
- default:
- WARN_ON_ONCE(1);
- return VM_FAULT_FALLBACK;
- }
-
- /* Fallback on write dirty-tracking or COW */
- if (write && ttm_pgprot_is_wrprotecting(vma->vm_page_prot))
- return VM_FAULT_FALLBACK;
-
- ret = ttm_bo_vm_reserve(bo, vmf);
- if (ret)
- return ret;
-
- prot = vm_get_page_prot(vma->vm_flags);
- ret = ttm_bo_vm_fault_reserved(vmf, prot, 1, fault_page_size);
- if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
- return ret;
-
- dma_resv_unlock(bo->base.resv);
-
- return ret;
-}
-#endif
-
void ttm_bo_vm_open(struct vm_area_struct *vma)
{
struct ttm_buffer_object *bo = vma->vm_private_data;
@@ -604,9 +544,6 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.open = ttm_bo_vm_open,
.close = ttm_bo_vm_close,
.access = ttm_bo_vm_access,
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
- .huge_fault = ttm_bo_vm_huge_fault,
-#endif
};
static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
--
2.21.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Bad rss-counter state from drm/ttm, drm/vmwgfx: Support huge TTM pagefaults
On 4/6/20 9:51 PM, Alex Xu (Hello71) wrote: Using 314b658 with amdgpu, starting sway and firefox causes "BUG: Bad rss-counter state" and "BUG: non-zero pgtables_bytes on freeing mm" to start filling dmesg, and then closing programs causes more BUGs and hangs, and then everything grinds to a halt (can't start more programs, can't even reboot through systemd). Using master and reverting that branch up to that point fixes the problem. I'm using a Ryzen 1600 and AMD Radeon RX 480 on an ASRock B450 Pro4 board with IOMMU enabled. Hmm. That sounds bad. Could you send a copy of your config? Meanwhile, I'll prepare a small patch that disables the non-vmwgfx huge_fault() until we've figured out what's happening. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 03/18] drm/i915/display/ddi: Prefer drm_WARN* over WARN*
Hi Pankaj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20200406]
[cannot apply to v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Pankaj-Bharadiya/Prefer-drm_WARN-over-WARN/20200406-210932
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-f002-20200406 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project
a43e23360657e3df82af6b96b403d1a5a3174744)
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_ddi.c:3624:14: error: use of undeclared
>> identifier 'state'
drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
^
>> drivers/gpu/drm/i915/display/intel_ddi.c:3624:14: error: use of undeclared
>> identifier 'state'
2 errors generated.
vim +/state +3624 drivers/gpu/drm/i915/display/intel_ddi.c
3619
3620 static void intel_enable_ddi(struct intel_encoder *encoder,
3621 const struct intel_crtc_state *crtc_state,
3622 const struct drm_connector_state
*conn_state)
3623 {
> 3624 drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
3625
3626 intel_enable_pipe(crtc_state);
3627
3628 intel_crtc_vblank_on(crtc_state);
3629
3630 if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
3631 intel_enable_ddi_hdmi(encoder, crtc_state, conn_state);
3632 else
3633 intel_enable_ddi_dp(encoder, crtc_state, conn_state);
3634
3635 /* Enable hdcp if it's desired */
3636 if (conn_state->content_protection ==
3637 DRM_MODE_CONTENT_PROTECTION_DESIRED)
3638
intel_hdcp_enable(to_intel_connector(conn_state->connector),
3639crtc_state->cpu_transcoder,
3640(u8)conn_state->hdcp_content_type);
3641 }
3642
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/dp_mst: Remove drm_dp_mst_has_audio()
Drive-by fix I noticed the other day - drm_dp_mst_has_audio() only ever
made sense back when we still had to validate ports before accessing
them in order to (attempt to) avoid NULL dereferences. Since we have
proper reference counting that guarantees we always can safely access
the MST port, there's no use in keeping this function around as all it
does is validate the port pointer before checking the audio status.
Note - drm_dp_mst_port->has_audio is technically protected by
drm_device->mode_config.connection_mutex, since it's only ever updated
from drm_dp_mst_get_edid(). Additionally, we change the declaration for
port in struct intel_connector to be properly typed, so we can directly
access it.
Changes since v1:
* Change type of intel_connector->port in a separate patch - Sean Paul
Cc: "Lee, Shawn C"
Reviewed-by: Sean Paul
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 21 ---
.../drm/i915/display/intel_display_debugfs.c | 10 ++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +--
include/drm/drm_dp_mst_helper.h | 2 --
4 files changed, 3 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5b205aea58d4..8289d59b62da 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4063,27 +4063,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
}
EXPORT_SYMBOL(drm_dp_mst_detect_port);
-/**
- * drm_dp_mst_port_has_audio() - Check whether port has audio capability or not
- * @mgr: manager for this port
- * @port: unverified pointer to a port.
- *
- * This returns whether the port supports audio or not.
- */
-bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
- struct drm_dp_mst_port *port)
-{
- bool ret = false;
-
- port = drm_dp_mst_topology_get_port_validated(mgr, port);
- if (!port)
- return ret;
- ret = port->has_audio;
- drm_dp_mst_topology_put_port(port);
- return ret;
-}
-EXPORT_SYMBOL(drm_dp_mst_port_has_audio);
-
/**
* drm_dp_mst_get_edid() - get EDID for an MST port
* @connector: toplevel connector to get EDID for
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 424f4e52f783..9f736420d83f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -631,15 +631,9 @@ static void intel_dp_info(struct seq_file *m,
}
static void intel_dp_mst_info(struct seq_file *m,
- struct intel_connector *intel_connector)
+ struct intel_connector *intel_connector)
{
- struct intel_encoder *intel_encoder =
intel_attached_encoder(intel_connector);
- struct intel_dp_mst_encoder *intel_mst =
- enc_to_mst(intel_encoder);
- struct intel_digital_port *intel_dig_port = intel_mst->primary;
- struct intel_dp *intel_dp = &intel_dig_port->dp;
- bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
- intel_connector->port);
+ bool has_audio = intel_connector->port->has_audio;
seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 61605eb8c2af..c35efc9e628d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -114,8 +114,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder
*encoder,
if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
pipe_config->has_audio =
- drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
- connector->port);
+ connector->port->has_audio;
else
pipe_config->has_audio =
intel_conn_state->force_audio == HDMI_AUDIO_ON;
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 7af51c947b81..2d7c26592c05 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -732,8 +732,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port);
-bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
- struct drm_dp_mst_port *port);
struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/i915/dp_mst: Cast intel_connector->port as drm_dp_mst_port
The only reason for having this cast as void * before was because we
originally needed to use drm_dp_mst_get_port_validated() and friends in
order to (attempt to) safely access MST ports. However, we've since
improved how reference counting works with ports and mstbs such that we
can now rely on drm_dp_mst_port structs remaining in memory for as long
as the driver needs. This means we don't really need to cast this as
void* anymore, and can just access the struct directly.
We'll also need this for the next commit, so that we can remove
drm_dp_mst_port_has_audio().
Signed-off-by: Lyude Paul
Reviewed-by: Sean Paul
---
drivers/gpu/drm/i915/display/intel_display_types.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 5a0adf14ebef..0ddc98afe252 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -438,7 +438,7 @@ struct intel_connector {
state of connector->polled in case hotplug storm detection changes
it */
u8 polled;
- void *port; /* store this opaque as its illegal to dereference it */
+ struct drm_dp_mst_port *port;
struct intel_dp *mst_port;
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/10] Set up generic fbdev after registering device
Hi Thomas. On Mon, Apr 06, 2020 at 03:43:55PM +0200, Thomas Zimmermann wrote: > Generic fbdev emulation is a DRM client. If possible, it should behave > like userspace clients. Therefore it should not run before the driver > registered the new DRM device. If the setup function fails, the driver > should not report an error. Thanks for taking the time to refactor all the relevant drivers. I have received some push-back in the past when suggesting this, but cannot remember from who. Let's see what review comments you get. As the rule is that the fbdev setup shall be setup after registering the DRM device - it would be nice to have this included in the documentation of drm_fbdev_generic_setup Could you try to to update the documentation to cover this? I will get back to the patches later this week. Sam > > This is a follow-up patchset to the discussion at [1]. I went > through all calls to drm_fbdev_generic_setup(), moved them to the > final operation of their driver's probe function, and removed the > return value. > > Built-tested on x86-64, aarch64 and arm. > > [1] > https://lore.kernel.org/dri-devel/20200403135828.2542770-1-daniel.vet...@ffwll.ch/T/#m216b5b37aeeb7b28d55ad73b7a702b3d1d7bf867 > > Thomas Zimmermann (10): > drm/ast: Set up fbdev after registering device; remove error checks > drm/hibmc: Remove error check from fbdev setup > drm/kirin: Set up fbdev after fully registering device > drm/ingenic: Remove error check from fbdev setup > drm/mediathek: Remove error check from fbdev setup > drm/mgag200: Set up fbdev after registering device; remove error > checks > drm/tilcdc: Set up fbdev after fully registering device > drm/udl: Remove error check from fbdev setup > drm/vboxvideo: Set up fbdev after registering device; remove error > checks > drm/fb-helper: Remove return value from drm_fbdev_generic_setup() > > drivers/gpu/drm/ast/ast_drv.c | 3 +++ > drivers/gpu/drm/ast/ast_main.c | 5 - > drivers/gpu/drm/drm_fb_helper.c| 18 -- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c| 6 +- > .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c| 4 ++-- > drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +--- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +--- > drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ > drivers/gpu/drm/mgag200/mgag200_main.c | 4 > drivers/gpu/drm/tilcdc/tilcdc_drv.c| 3 +-- > drivers/gpu/drm/udl/udl_drv.c | 6 +- > drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++ > include/drm/drm_fb_helper.h| 5 +++-- > 13 files changed, 25 insertions(+), 45 deletions(-) > > -- > 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp_mst: Remove drm_dp_mst_has_audio()
On Fri, Apr 3, 2020 at 6:23 PM Lyude Paul wrote:
>
> Drive-by fix I noticed the other day - drm_dp_mst_has_audio() only ever
> made sense back when we still had to validate ports before accessing
> them in order to (attempt to) avoid NULL dereferences. Since we have
> proper reference counting that guarantees we always can safely access
> the MST port, there's no use in keeping this function around as all it
> does is validate the port pointer before checking the audio status.
>
> Note - drm_dp_mst_port->has_audio is technically protected by
> drm_device->mode_config.connection_mutex, since it's only ever updated
> from drm_dp_mst_get_edid(). Additionally, we change the declaration for
> port in struct intel_connector to be properly typed, so we can directly
> access it.
>
This is kind of burying the lede. I'd almost prefer a 2 patch series:
drm/i915: Allow connectors to directly access drm_dp_mst_port
drm/dp_mst: Remove unused drm_dp_mst_port_has_audio()
That way if anyone objects to the idea of accessing mst_port directly
from i915 driver, it's more obvious from the patch subject.
Nitpicks aside, the code looks good to me, it's a nice cleanup!
Reviewed-by: Sean Paul
> Cc: "Lee, Shawn C"
> Cc: Sean Paul
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 21 ---
> .../drm/i915/display/intel_display_debugfs.c | 10 ++---
> .../drm/i915/display/intel_display_types.h| 2 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +--
> include/drm/drm_dp_mst_helper.h | 2 --
> 5 files changed, 4 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1ff49547b2e8..129126091e90 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4063,27 +4063,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_dp_mst_detect_port);
>
> -/**
> - * drm_dp_mst_port_has_audio() - Check whether port has audio capability or
> not
> - * @mgr: manager for this port
> - * @port: unverified pointer to a port.
> - *
> - * This returns whether the port supports audio or not.
> - */
> -bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
> - struct drm_dp_mst_port *port)
> -{
> - bool ret = false;
> -
> - port = drm_dp_mst_topology_get_port_validated(mgr, port);
> - if (!port)
> - return ret;
> - ret = port->has_audio;
> - drm_dp_mst_topology_put_port(port);
> - return ret;
> -}
> -EXPORT_SYMBOL(drm_dp_mst_port_has_audio);
> -
> /**
> * drm_dp_mst_get_edid() - get EDID for an MST port
> * @connector: toplevel connector to get EDID for
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 424f4e52f783..9f736420d83f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -631,15 +631,9 @@ static void intel_dp_info(struct seq_file *m,
> }
>
> static void intel_dp_mst_info(struct seq_file *m,
> - struct intel_connector *intel_connector)
> + struct intel_connector *intel_connector)
> {
> - struct intel_encoder *intel_encoder =
> intel_attached_encoder(intel_connector);
> - struct intel_dp_mst_encoder *intel_mst =
> - enc_to_mst(intel_encoder);
> - struct intel_digital_port *intel_dig_port = intel_mst->primary;
> - struct intel_dp *intel_dp = &intel_dig_port->dp;
> - bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
> - intel_connector->port);
> + bool has_audio = intel_connector->port->has_audio;
>
> seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 2bedd626c686..1de7bef0a49b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -436,7 +436,7 @@ struct intel_connector {
>state of connector->polled in case hotplug storm detection changes
> it */
> u8 polled;
>
> - void *port; /* store this opaque as its illegal to dereference it */
> + struct drm_dp_mst_port *port;
>
> struct intel_dp *mst_port;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 61605eb8c2af..c35efc9e628d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -114,8 +114,7 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
>
> if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
>
Re: [PATCH] drm/dp_mst: Don't drop NAKs for down responses
On Fri, Apr 3, 2020 at 4:03 PM Lyude Paul wrote:
>
> It looks like that when we introduced the ability to handle multiple
> down requests at once, we accidentally started dropping NAK replies -
> causing sideband messages which got NAK'd to seemingly timeout and cause
> all sorts of weirdness.
>
> So, fix this by making sure we don't return from
> drm_dp_mst_handle_down_rep() early, but instead treat NAKs like any
> other message.
>
> Signed-off-by: Lyude Paul
> Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies")
> Cc: Wayne Lin
> Cc: Wayne Lin
> Cc: Sean Paul
Thank you for fixing this
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 10d0315af513..5449ada3e019 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3813,7 +3813,6 @@ static int drm_dp_mst_handle_down_rep(struct
> drm_dp_mst_topology_mgr *mgr)
> txmsg->reply.u.nak.reason,
>
> drm_dp_mst_nak_reason_str(txmsg->reply.u.nak.reason),
> txmsg->reply.u.nak.nak_data);
> - goto out_clear_reply;
> }
>
> memset(msg, 0, sizeof(struct drm_dp_sideband_msg_rx));
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/dp_mst: Increase ACT retry timeout to 3s
On Mon, Apr 6, 2020 at 3:43 PM Lyude Paul wrote:
>
> On Mon, 2020-04-06 at 15:41 -0400, Sean Paul wrote:
> > On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
> > > Currently we only poll for an ACT up to 30 times, with a busy-wait delay
> > > of 100µs between each attempt - giving us a timeout of 2900µs. While
> > > this might seem sensible, it would appear that in certain scenarios it
> > > can take dramatically longer then that for us to receive an ACT. On one
> > > of the EVGA MST hubs that I have available, I observed said hub
> > > sometimes taking longer then a second before signalling the ACT. These
> > > delays mostly seem to occur when previous sideband messages we've sent
> > > are NAKd by the hub, however it wouldn't be particularly surprising if
> > > it's possible to reproduce times like this simply by introducing branch
> > > devices with large LCTs since payload allocations have to take effect on
> > > every downstream device up to the payload's target.
> > >
> > > So, instead of just retrying 30 times we poll for the ACT for up to 3ms,
> > > and additionally use usleep_range() to avoid a very long and rude
> > > busy-wait. Note that the previous retry count of 30 appears to have been
> > > arbitrarily chosen, as I can't find any mention of a recommended timeout
> > > or retry count for ACTs in the DisplayPort 2.0 specification. This also
> > > goes for the range we were previously using for udelay(), although I
> > > suspect that was just copied from the recommended delay for link
> > > training on SST devices.
> > >
> > > Signed-off-by: Lyude Paul
> > > Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper
> > > (v0.6)")
> > > Cc: Sean Paul
> > > Cc: # v3.17+
> > > ---
> > > drivers/gpu/drm/drm_dp_mst_topology.c | 26 +++---
> > > 1 file changed, 19 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 7aaf184a2e5f..f313407374ed 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -4466,17 +4466,30 @@ static int drm_dp_dpcd_write_payload(struct
> > > drm_dp_mst_topology_mgr *mgr,
> > > * @mgr: manager to use
> > > *
> > > * Tries waiting for the MST hub to finish updating it's payload table by
> > > - * polling for the ACT handled bit.
> > > + * polling for the ACT handled bit for up to 3 seconds (yes-some hubs
> > > really
> > > + * take that long).
> > > *
> > > * Returns:
> > > * 0 if the ACT was handled in time, negative error code on failure.
> > > */
> > > int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> > > {
> > > - int count = 0, ret;
> > > + /*
> > > +* There doesn't seem to be any recommended retry count or timeout
> > > in
> > > +* the MST specification. Since some hubs have been observed to
> > > take
> > > +* over 1 second to update their payload allocations under certain
> > > +* conditions, we use a rather large timeout value.
> > > +*/
> > > + const int timeout_ms = 3000;
> > > + unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> > > + int ret;
> > > + bool retrying = false;
> > > u8 status;
> > >
> > > do {
> > > + if (retrying)
> > > + usleep_range(100, 1000);
> > > +
> > > ret = drm_dp_dpcd_readb(mgr->aux,
> > > DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > &status);
> > > @@ -4488,13 +4501,12 @@ int drm_dp_check_act_status(struct
> > > drm_dp_mst_topology_mgr *mgr)
> > >
> > > if (status & DP_PAYLOAD_ACT_HANDLED)
> > > break;
> > > - count++;
> > > - udelay(100);
> > > - } while (count < 30);
> > > + retrying = true;
> > > + } while (jiffies < timeout);
> >
> > Somewhat academic, but I think there's an overflow possibility here if
> > timeout is near ulong_max and jiffies overflows during the usleep. In
> > that case we'll be retrying for a very loong time.
> >
> > I wish we had i915's wait_for() macro available to all drm...
>
> Maybe we could add it to the kernel library somewhere? I don't see why we'd
> need to stop at DRM
So You Want To Build A Bikeshed...
Seriously though, I'd be very happy with that. Alternatively you could
shoehorn this into readx_poll_timeout as well.
Sean
>
> >
> > Sean
> >
> > > if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> > > - DRM_DEBUG_KMS("failed to get ACT bit %d after %d
> > > retries\n",
> > > - status, count);
> > > + DRM_DEBUG_KMS("failed to get ACT bit %d after %dms\n",
> > > + status, timeout_ms);
> > > return -EINVAL;
> > > }
> > > return 0;
> > > --
> > >
[PATCH v2 3/3] drm/writeback: wire drm_writeback.h to kernel-doc
drm_writeback.h included a lot of nice kernel-doc comments.
Wire it up so the header file is included in the kernel-doc
generated documentation.
Added a few simple comments to the two structs so they
get picked up by kernel-doc.
Signed-off-by: Sam Ravnborg
Cc: Laurent Pinchart
Cc: Brian Starkey
Cc: Liviu Dudau
Cc: Daniel Vetter
Cc: Thomas Zimmermann
Cc: Maxime Ripard
Cc: Maxime Ripard
---
Documentation/gpu/drm-kms.rst | 3 +++
include/drm/drm_writeback.h | 9 +
2 files changed, 12 insertions(+)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index e1f685015807..397314d08f77 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -397,6 +397,9 @@ Connector Functions Reference
Writeback Connectors
+.. kernel-doc:: include/drm/drm_writeback.h
+ :internal:
+
.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
:doc: overview
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 777c14c847f0..9697d2714d2a 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -15,7 +15,13 @@
#include
#include
+/**
+ * struct drm_writeback_connector - DRM writeback connector
+ */
struct drm_writeback_connector {
+ /**
+* @base: base drm_connector object
+*/
struct drm_connector base;
/**
@@ -78,6 +84,9 @@ struct drm_writeback_connector {
char timeline_name[32];
};
+/**
+ * struct drm_writeback_job - DRM writeback job
+ */
struct drm_writeback_job {
/**
* @connector:
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] drm: kernel-doc stuff
Lyude - I added a "Co-developed-by: ..." Can I get your s-o-b on the patch, to document that you are OK with this to go in. Needed on top of an r-b. v2: - committed acked/reviewed patches - significantly updated the vbalnk patch based on a lot of good inputs - updated the writeback patch - wired drm_writeback.h to kernel-doc Sam Sam Ravnborg (3): drm/vblank: Add intro to documentation drm: writeback: document callbacks drm/writeback: wire drm_writeback.h to kernel-doc Documentation/gpu/drm-kms.rst| 3 ++ drivers/gpu/drm/drm_vblank.c | 53 include/drm/drm_modeset_helper_vtables.h | 27 include/drm/drm_writeback.h | 9 ++ 4 files changed, 92 insertions(+) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/vblank: Add intro to documentation
Lyude Paul wrote a very good intro to vblank here:
https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.ca...@redhat.com/T/#mce6480be738160e9d07c5d023e88fd78d7a06d27
Add this to the intro chapter in drm_vblank.c so others
can benefit from it too.
v2:
- Reworded to improve readability (Thomas)
v3:
- Added nice ascii drawing from Lyude (Lyude)
- Added referende to high-precision timestamp (Daniel)
- Improved grammar (Thomas)
- Combined it all and made kernel-doc happy
- Dropped any a-b, r-b do to the amount of changes
Signed-off-by: Sam Ravnborg
Co-developed-by: Lyude Paul
Cc: Lyude Paul
Cc: Thomas Zimmermann
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
---
drivers/gpu/drm/drm_vblank.c | 53
1 file changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index bcf346b3e486..9633092c9ad5 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -41,6 +41,59 @@
/**
* DOC: vblank handling
*
+ * From the computer's perspective, every time the monitor displays
+ * a new frame the scanout engine have "scanned out" the display image
+ * from top to bottom, one row of pixels at a time.
+ * The current row of pixels is referred to as the current scanline.
+ *
+ * In addition to the display's visible area, there's usually a couple of
+ * extra scanlines which aren't actually displayed on the screen.
+ * These extra scanlines don't contain image data and are occasionally used
+ * for features like audio and infoframes. The region made up of these
+ * scanlines is referred to as the vertical blanking region, or vblank for
+ * short.
+ *
+ * ::
+ *
+ *
+ *physical →
+ *top of ||
+ *display ||
+ *| New frame|
+ *||
+ *||
+ *|| ← Scanline,
updates
+ *|| the frame as it
+ *|| travels down
+ *|| ("scan out")
+ *||
+ *| Old frame|
+ *||
+ *||
+ *||
+ *|| physical
+ *|| bottom of
+ *vertical|| ← display
+ *blanking┆┆
+ *region → ┆┆
+ *┆┆
+ *start of →
+ *new frame
+ *
+ * "Physical top of display" is the reference point for the high-precision/
+ * corrected timestamp.
+ *
+ * On a lot of display hardware, programming needs to take effect during the
+ * vertical blanking period so that settings like gamma, the image buffer
+ * buffer to be scanned out, etc. can safely be changed without showing
+ * any visual artifacts on the screen. In some unforgiving hardware, some of
+ * this programming has to both start and end in the same vblank.
+ *
+ * The vblank interrupt may be fired at different points depending on the
+ * hardware. Some hardware implementations will fire the interrupt when the
+ * new frame start, other implementations will fire the interrupt at different
+ * points in time.
+ *
* Vertical blanking plays a major role in graphics rendering. To achieve
* tear-free display, users must synchronize page flips and/or rendering to
* vertical blanking. The DRM API offers ioctls to perform page flips
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm: writeback: document callbacks
Document the callbacks:
drm_connector_helper_funcs.prepare_writeback_job
drm_connector_helper_funcs.cleanup_writeback_job
The documentation was pulled from the changelong introducing the
callbacks, originally written by Laurent.
Adding the missing documentation fixes the following warnings:
drm_modeset_helper_vtables.h:1052: warning: Function parameter or member
'prepare_writeback_job' not described in 'drm_connector_helper_funcs'
drm_modeset_helper_vtables.h:1052: warning: Function parameter or member
'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
v2:
- Fix formatting (Daniel)
- Drop changelog text and add reference (Daniel)
- Improve grammar. and use "operation" (Laurent)
Signed-off-by: Sam Ravnborg
Reviewed-by: Daniel Vetter
Reviewed-by: Laurent Pinchart
Cc: Laurent Pinchart
Cc: Liviu Dudau
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
---
include/drm/drm_modeset_helper_vtables.h | 27
1 file changed, 27 insertions(+)
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index 7c20b1c8b6a7..421a30f08463 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -1075,8 +1075,35 @@ struct drm_connector_helper_funcs {
void (*atomic_commit)(struct drm_connector *connector,
struct drm_connector_state *state);
+ /**
+* @prepare_writeback_job:
+*
+* As writeback jobs contain a framebuffer, drivers may need to
+* prepare and clean them up the same way they can prepare and
+* clean up framebuffers for planes. This optional connector operation
+* is used to support the preparation of writeback jobs. The job
+* prepare operation is called from drm_atomic_helper_prepare_planes()
+* for struct &drm_writeback_connector connectors only.
+*
+* This operation is optional.
+*
+* This callback is used by the atomic modeset helpers.
+*/
int (*prepare_writeback_job)(struct drm_writeback_connector *connector,
struct drm_writeback_job *job);
+ /**
+* @cleanup_writeback_job:
+*
+* This optional connector operation is used to support the
+* cleanup of writeback jobs. The job cleanup operation is called
+* from the existing drm_writeback_cleanup_job() function, invoked
+* both when destroying the job as part of an aborted commit, or when
+* the job completes.
+*
+* This operation is optional.
+*
+* This callback is used by the atomic modeset helpers.
+*/
void (*cleanup_writeback_job)(struct drm_writeback_connector *connector,
struct drm_writeback_job *job);
};
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp_mst: Fix NULL deref in drm_dp_get_one_sb_msg()
On Mon, Apr 6, 2020 at 3:34 PM Lyude Paul wrote:
>
> While we don't need this function to store an mstb anywhere for UP
> requests since we process them asynchronously, we do need to make sure
> that we don't try to write to **mstb for UP requests otherwise we'll
> cause a NULL pointer deref:
>
> RIP: 0010:drm_dp_get_one_sb_msg+0x4b/0x460 [drm_kms_helper]
> Call Trace:
> ? vprintk_emit+0x16a/0x230
> ? drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
> drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
> ? __drm_dbg+0x87/0x90 [drm]
> ? intel_dp_hpd_pulse+0x24b/0x400 [i915]
> intel_dp_hpd_pulse+0x24b/0x400 [i915]
> i915_digport_work_func+0xd6/0x160 [i915]
> process_one_work+0x1a9/0x370
> worker_thread+0x4d/0x3a0
> kthread+0xf9/0x130
> ? process_one_work+0x370/0x370
> ? kthread_park+0x90/0x90
> ret_from_fork+0x35/0x40
>
> So, fix this.
Ugggh, what a fail! I found this in Feb and posted the patch in
20200218171522.GF253734@art_vandelay. I had to migrate my workstation
due to WFH order and didn't apply the patch before pushing. Messy
messy messy.
Thanks for fixing!
Reviewed-by: Sean Paul
>
> Signed-off-by: Lyude Paul
> Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies")
> Cc: Wayne Lin
> Cc: Lyude Paul
> Cc: Wayne Lin
> Cc: Sean Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1ff49547b2e8..8751278b3941 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3703,7 +3703,8 @@ static bool drm_dp_get_one_sb_msg(struct
> drm_dp_mst_topology_mgr *mgr, bool up,
> int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
>DP_SIDEBAND_MSG_DOWN_REP_BASE;
>
> - *mstb = NULL;
> + if (!up)
> + *mstb = NULL;
> *seqno = -1;
>
> len = min(mgr->max_dpcd_transaction_bytes, 16);
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/dp_mst: Print errors on ACT timeouts
On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
>
> Although it's not unexpected for drm_dp_check_act_status() to fail due
> to DPCD read failures (as the hub may have just been unplugged
> suddenly), timeouts are a bit more worrying as they either mean we need
> a longer timeout value, or we aren't setting up payload allocations
> properly. So, let's start printing errors on timeouts.
>
> Signed-off-by: Lyude Paul
> Cc: Sean Paul
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index f313407374ed..3d0d373f6f91 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4494,6 +4494,10 @@ int drm_dp_check_act_status(struct
> drm_dp_mst_topology_mgr *mgr)
> DP_PAYLOAD_TABLE_UPDATE_STATUS,
> &status);
> if (ret < 0) {
> + /*
> +* Failure here isn't unexpected - the hub may have
> +* just been unplugged
> +*/
> DRM_DEBUG_KMS("failed to read payload table status
> %d\n",
> ret);
> return ret;
> @@ -4505,8 +4509,8 @@ int drm_dp_check_act_status(struct
> drm_dp_mst_topology_mgr *mgr)
> } while (jiffies < timeout);
>
> if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> - DRM_DEBUG_KMS("failed to get ACT bit %d after %dms\n",
> - status, timeout_ms);
> + DRM_ERROR("Failed to get ACT after %dms, last status: %02x\n",
> + timeout_ms, status);
> return -EINVAL;
> }
> return 0;
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/dp_mst: Increase ACT retry timeout to 3s
On Mon, 2020-04-06 at 15:41 -0400, Sean Paul wrote:
> On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
> > Currently we only poll for an ACT up to 30 times, with a busy-wait delay
> > of 100µs between each attempt - giving us a timeout of 2900µs. While
> > this might seem sensible, it would appear that in certain scenarios it
> > can take dramatically longer then that for us to receive an ACT. On one
> > of the EVGA MST hubs that I have available, I observed said hub
> > sometimes taking longer then a second before signalling the ACT. These
> > delays mostly seem to occur when previous sideband messages we've sent
> > are NAKd by the hub, however it wouldn't be particularly surprising if
> > it's possible to reproduce times like this simply by introducing branch
> > devices with large LCTs since payload allocations have to take effect on
> > every downstream device up to the payload's target.
> >
> > So, instead of just retrying 30 times we poll for the ACT for up to 3ms,
> > and additionally use usleep_range() to avoid a very long and rude
> > busy-wait. Note that the previous retry count of 30 appears to have been
> > arbitrarily chosen, as I can't find any mention of a recommended timeout
> > or retry count for ACTs in the DisplayPort 2.0 specification. This also
> > goes for the range we were previously using for udelay(), although I
> > suspect that was just copied from the recommended delay for link
> > training on SST devices.
> >
> > Signed-off-by: Lyude Paul
> > Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper
> > (v0.6)")
> > Cc: Sean Paul
> > Cc: # v3.17+
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 26 +++---
> > 1 file changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 7aaf184a2e5f..f313407374ed 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4466,17 +4466,30 @@ static int drm_dp_dpcd_write_payload(struct
> > drm_dp_mst_topology_mgr *mgr,
> > * @mgr: manager to use
> > *
> > * Tries waiting for the MST hub to finish updating it's payload table by
> > - * polling for the ACT handled bit.
> > + * polling for the ACT handled bit for up to 3 seconds (yes-some hubs
> > really
> > + * take that long).
> > *
> > * Returns:
> > * 0 if the ACT was handled in time, negative error code on failure.
> > */
> > int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> > {
> > - int count = 0, ret;
> > + /*
> > +* There doesn't seem to be any recommended retry count or timeout
> > in
> > +* the MST specification. Since some hubs have been observed to
> > take
> > +* over 1 second to update their payload allocations under certain
> > +* conditions, we use a rather large timeout value.
> > +*/
> > + const int timeout_ms = 3000;
> > + unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> > + int ret;
> > + bool retrying = false;
> > u8 status;
> >
> > do {
> > + if (retrying)
> > + usleep_range(100, 1000);
> > +
> > ret = drm_dp_dpcd_readb(mgr->aux,
> > DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > &status);
> > @@ -4488,13 +4501,12 @@ int drm_dp_check_act_status(struct
> > drm_dp_mst_topology_mgr *mgr)
> >
> > if (status & DP_PAYLOAD_ACT_HANDLED)
> > break;
> > - count++;
> > - udelay(100);
> > - } while (count < 30);
> > + retrying = true;
> > + } while (jiffies < timeout);
>
> Somewhat academic, but I think there's an overflow possibility here if
> timeout is near ulong_max and jiffies overflows during the usleep. In
> that case we'll be retrying for a very loong time.
>
> I wish we had i915's wait_for() macro available to all drm...
Maybe we could add it to the kernel library somewhere? I don't see why we'd
need to stop at DRM
>
> Sean
>
> > if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> > - DRM_DEBUG_KMS("failed to get ACT bit %d after %d
> > retries\n",
> > - status, count);
> > + DRM_DEBUG_KMS("failed to get ACT bit %d after %dms\n",
> > + status, timeout_ms);
> > return -EINVAL;
> > }
> > return 0;
> > --
> > 2.25.1
> >
--
Cheers,
Lyude Paul (she/her)
Associate Software Engineer at Red Hat
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/dp_mst: Increase ACT retry timeout to 3s
On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
>
> Currently we only poll for an ACT up to 30 times, with a busy-wait delay
> of 100µs between each attempt - giving us a timeout of 2900µs. While
> this might seem sensible, it would appear that in certain scenarios it
> can take dramatically longer then that for us to receive an ACT. On one
> of the EVGA MST hubs that I have available, I observed said hub
> sometimes taking longer then a second before signalling the ACT. These
> delays mostly seem to occur when previous sideband messages we've sent
> are NAKd by the hub, however it wouldn't be particularly surprising if
> it's possible to reproduce times like this simply by introducing branch
> devices with large LCTs since payload allocations have to take effect on
> every downstream device up to the payload's target.
>
> So, instead of just retrying 30 times we poll for the ACT for up to 3ms,
> and additionally use usleep_range() to avoid a very long and rude
> busy-wait. Note that the previous retry count of 30 appears to have been
> arbitrarily chosen, as I can't find any mention of a recommended timeout
> or retry count for ACTs in the DisplayPort 2.0 specification. This also
> goes for the range we were previously using for udelay(), although I
> suspect that was just copied from the recommended delay for link
> training on SST devices.
>
> Signed-off-by: Lyude Paul
> Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
> Cc: Sean Paul
> Cc: # v3.17+
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 7aaf184a2e5f..f313407374ed 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4466,17 +4466,30 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
> * @mgr: manager to use
> *
> * Tries waiting for the MST hub to finish updating it's payload table by
> - * polling for the ACT handled bit.
> + * polling for the ACT handled bit for up to 3 seconds (yes-some hubs really
> + * take that long).
> *
> * Returns:
> * 0 if the ACT was handled in time, negative error code on failure.
> */
> int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> {
> - int count = 0, ret;
> + /*
> +* There doesn't seem to be any recommended retry count or timeout in
> +* the MST specification. Since some hubs have been observed to take
> +* over 1 second to update their payload allocations under certain
> +* conditions, we use a rather large timeout value.
> +*/
> + const int timeout_ms = 3000;
> + unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> + int ret;
> + bool retrying = false;
> u8 status;
>
> do {
> + if (retrying)
> + usleep_range(100, 1000);
> +
> ret = drm_dp_dpcd_readb(mgr->aux,
> DP_PAYLOAD_TABLE_UPDATE_STATUS,
> &status);
> @@ -4488,13 +4501,12 @@ int drm_dp_check_act_status(struct
> drm_dp_mst_topology_mgr *mgr)
>
> if (status & DP_PAYLOAD_ACT_HANDLED)
> break;
> - count++;
> - udelay(100);
> - } while (count < 30);
> + retrying = true;
> + } while (jiffies < timeout);
Somewhat academic, but I think there's an overflow possibility here if
timeout is near ulong_max and jiffies overflows during the usleep. In
that case we'll be retrying for a very loong time.
I wish we had i915's wait_for() macro available to all drm...
Sean
>
> if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> - DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
> - status, count);
> + DRM_DEBUG_KMS("failed to get ACT bit %d after %dms\n",
> + status, timeout_ms);
> return -EINVAL;
> }
> return 0;
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: Fix NULL deref in drm_dp_get_one_sb_msg()
While we don't need this function to store an mstb anywhere for UP
requests since we process them asynchronously, we do need to make sure
that we don't try to write to **mstb for UP requests otherwise we'll
cause a NULL pointer deref:
RIP: 0010:drm_dp_get_one_sb_msg+0x4b/0x460 [drm_kms_helper]
Call Trace:
? vprintk_emit+0x16a/0x230
? drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
? __drm_dbg+0x87/0x90 [drm]
? intel_dp_hpd_pulse+0x24b/0x400 [i915]
intel_dp_hpd_pulse+0x24b/0x400 [i915]
i915_digport_work_func+0xd6/0x160 [i915]
process_one_work+0x1a9/0x370
worker_thread+0x4d/0x3a0
kthread+0xf9/0x130
? process_one_work+0x370/0x370
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
So, fix this.
Signed-off-by: Lyude Paul
Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies")
Cc: Wayne Lin
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Sean Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1ff49547b2e8..8751278b3941 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3703,7 +3703,8 @@ static bool drm_dp_get_one_sb_msg(struct
drm_dp_mst_topology_mgr *mgr, bool up,
int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
DP_SIDEBAND_MSG_DOWN_REP_BASE;
- *mstb = NULL;
+ if (!up)
+ *mstb = NULL;
*seqno = -1;
len = min(mgr->max_dpcd_transaction_bytes, 16);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] dt-bindings: display: bridge: renesas,lvds: Convert binding to YAML
On Mon, Apr 6, 2020 at 5:40 AM Geert Uytterhoeven wrote:
>
> Hi Laurent,
>
> On Mon, Apr 6, 2020 at 1:09 PM Laurent Pinchart
> wrote:
> > On Mon, Apr 06, 2020 at 10:47:37AM +0200, Geert Uytterhoeven wrote:
> > > On Mon, Apr 6, 2020 at 1:24 AM Laurent Pinchart wrote:
> > > > Convert the Renesas R-Car LVDS encoder text binding to YAML.
> > > >
> > > > Signed-off-by: Laurent Pinchart
> > > >
>
> > > > +if:
> > > > + properties:
> > > > +compatible:
> > > > + enum:
> > > > +- renesas,r8a774c0-lvds
> > > > +- renesas,r8a77990-lvds
> > > > +- renesas,r8a77995-lvds
> > > > +then:
> > > > + properties:
> > > > +clocks:
> > > > + minItems: 1
> > > > + maxItems: 4
> > > > + items:
> > > > +- description: Functional clock
> > > > +- description: EXTAL input clock
> > > > +- description: DU_DOTCLKIN0 input clock
> > > > +- description: DU_DOTCLKIN1 input clock
> > > > +
> > > > +clock-names:
> > > > + minItems: 1
> > > > + maxItems: 4
> > > > + items:
> > > > +- const: fck
> > > > +# The LVDS encoder can use the EXTAL or DU_DOTCLKINx clocks.
> > > > +# These clocks are optional.
> > > > +- enum:
> > > > + - extal
> > > > + - dclkin.0
> > > > + - dclkin.1
> > > > +- enum:
> > > > + - extal
> > > > + - dclkin.0
> > > > + - dclkin.1
> > > > +- enum:
> > > > + - extal
> > > > + - dclkin.0
> > > > + - dclkin.1
> > >
> > > Can the duplication of the last 3 entries be avoided?
> > > Perhaps like in
> > > Documentation/devicetree/bindings/serial/renesas,scif.yaml?
> >
> > I'd love to, if you can tell me how to make sure the fck entry is
> > mandatory. The following
> >
> > minItems: 1
> > maxItems: 4
> > items:
> > enum:
> > - fck
> > - extal
> > - dclkin.0
> > - dclkin.1
> >
> > passes the checks, but would accept
> >
> > clock-names = "extal";
> >
> > which is not valid. Your
> > Documentation/devicetree/bindings/serial/renesas,scif.yaml bindings
> > suffer from the same problem :-)
>
> Hmm
>
> > > > +examples:
> > > > + - |
> > > > +#include
> > > > +#include
> > > > +
> > > > +lvds@feb9 {
> > > > +compatible = "renesas,r8a7795-lvds";
> > > > +reg = <0 0xfeb9 0 0x14>;
> > >
> > > Examples are built with #{address,size}-cells = <1>.
> >
> > Are they ? I don't get any failure from make dt_binding_check.
>
> Hmm... And you do have "reg: maxItems: 1"...
At first glance I was expecting an error too, but there isn't. As far
as the schema is concerned, it's valid because it's a single entry
(i.e. one entry in <>). And then dtc can only check that reg is a
multiple of 2. The size check does work where we have more constraints
like I2C.
If we enforce bracketing, then we should be able to check these.
Otherwise, knowing both the cell sizes and number of entries is a
problem. With bracketing, we can split those checks. I'd been thinking
checking cell sizes would be easier in dtc (we're already doing that
in lots of cases), but thinking about it some more there is a way to
do this with schema:
if:
properties:
'#address-cells':
const: 2
'#size-cells':
const: 2
required:
- '#address-cells'
- '#size-cells'
then:
patternProperties:
'@':
properties:
reg:
items:
minItems: 4
maxItems: 4
required:
- reg
...and copy-n-paste for each size combination.
I imagine implementing this will result in another set of fixes.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/dp_mst: Reformat drm_dp_check_act_status() a bit
On Mon, 2020-04-06 at 15:23 -0400, Sean Paul wrote:
> On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
> > Just add a bit more line wrapping, get rid of some extraneous
> > whitespace, remove an unneeded goto label, and move around some variable
> > declarations. No functional changes here.
> >
> > Signed-off-by: Lyude Paul
> > [this isn't a fix, but it's needed for the fix that comes after this]
> > Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper
> > (v0.6)")
> > Cc: Sean Paul
> > Cc: # v3.17+
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++
> > 1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 2b9ce965f044..7aaf184a2e5f 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4473,33 +4473,31 @@ static int drm_dp_dpcd_write_payload(struct
> > drm_dp_mst_topology_mgr *mgr,
> > */
> > int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> > {
> > + int count = 0, ret;
> > u8 status;
> > - int ret;
> > - int count = 0;
> >
> > do {
> > - ret = drm_dp_dpcd_readb(mgr->aux,
> > DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
> > -
> > + ret = drm_dp_dpcd_readb(mgr->aux,
> > + DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > + &status);
> > if (ret < 0) {
> > - DRM_DEBUG_KMS("failed to read payload table status
> > %d\n", ret);
> > - goto fail;
> > + DRM_DEBUG_KMS("failed to read payload table status
> > %d\n",
> > + ret);
> > + return ret;
> > }
> >
> > if (status & DP_PAYLOAD_ACT_HANDLED)
> > break;
> > count++;
> > udelay(100);
> > -
> > } while (count < 30);
> >
> > if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> > - DRM_DEBUG_KMS("failed to get ACT bit %d after %d
> > retries\n", status, count);
> > - ret = -EINVAL;
> > - goto fail;
> > + DRM_DEBUG_KMS("failed to get ACT bit %d after %d
> > retries\n",
>
> Should we print status in base16 here?
>
> Otherwise:
>
> Reviewed-by: Sean Paul
Good point - I'll make sure to fix that before I push the series
>
> > + status, count);
> > + return -EINVAL;
> > }
> > return 0;
> > -fail:
> > - return ret;
> > }
> > EXPORT_SYMBOL(drm_dp_check_act_status);
> >
> > --
> > 2.25.1
> >
--
Cheers,
Lyude Paul (she/her)
Associate Software Engineer at Red Hat
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/dp_mst: Reformat drm_dp_check_act_status() a bit
On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
>
> Just add a bit more line wrapping, get rid of some extraneous
> whitespace, remove an unneeded goto label, and move around some variable
> declarations. No functional changes here.
>
> Signed-off-by: Lyude Paul
> [this isn't a fix, but it's needed for the fix that comes after this]
> Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
> Cc: Sean Paul
> Cc: # v3.17+
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2b9ce965f044..7aaf184a2e5f 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4473,33 +4473,31 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
> */
> int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> {
> + int count = 0, ret;
> u8 status;
> - int ret;
> - int count = 0;
>
> do {
> - ret = drm_dp_dpcd_readb(mgr->aux,
> DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
> -
> + ret = drm_dp_dpcd_readb(mgr->aux,
> + DP_PAYLOAD_TABLE_UPDATE_STATUS,
> + &status);
> if (ret < 0) {
> - DRM_DEBUG_KMS("failed to read payload table status
> %d\n", ret);
> - goto fail;
> + DRM_DEBUG_KMS("failed to read payload table status
> %d\n",
> + ret);
> + return ret;
> }
>
> if (status & DP_PAYLOAD_ACT_HANDLED)
> break;
> count++;
> udelay(100);
> -
> } while (count < 30);
>
> if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
> - DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
> status, count);
> - ret = -EINVAL;
> - goto fail;
> + DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
Should we print status in base16 here?
Otherwise:
Reviewed-by: Sean Paul
> + status, count);
> + return -EINVAL;
> }
> return 0;
> -fail:
> - return ret;
> }
> EXPORT_SYMBOL(drm_dp_check_act_status);
>
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/dp_mst: Improve kdocs for drm_dp_check_act_status()
On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul wrote:
>
> No functional changes.
>
> Signed-off-by: Lyude Paul
> Cc: Sean Paul
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 10d0315af513..2b9ce965f044 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4462,10 +4462,14 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
>
>
> /**
> - * drm_dp_check_act_status() - Check ACT handled status.
> + * drm_dp_check_act_status() - Polls for ACT handled status.
> * @mgr: manager to use
> *
> - * Check the payload status bits in the DPCD for ACT handled completion.
> + * Tries waiting for the MST hub to finish updating it's payload table by
> + * polling for the ACT handled bit.
> + *
> + * Returns:
> + * 0 if the ACT was handled in time, negative error code on failure.
> */
> int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
> {
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
Am 03.04.20 um 15:58 schrieb Daniel Vetter:
> Upcasting using a container_of macro is more typesafe, faster and
> easier for the compiler to optimize.
>
> Signed-off-by: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: Daniel Vetter
> Cc: "Noralf Trønnes"
> Cc: Sam Ravnborg
> Cc: Eric Anholt
> Cc: Thomas Zimmermann
> Cc: virtualizat...@lists.linux-foundation.org
> ---
> drivers/gpu/drm/cirrus/cirrus.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> index 4b65637147ba..744a8e337e41 100644
> --- a/drivers/gpu/drm/cirrus/cirrus.c
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -59,6 +59,8 @@ struct cirrus_device {
> void __iomem *mmio;
> };
>
> +#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev)
> +
Maybe to_cirrus_device() ? I had the same comment for vbox and I think
it applies to all patches.
Best regards
Thomas
> /* -- */
> /*
> * The meat of this driver. The core passes us a mode and we have to program
> @@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
> static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
> struct drm_rect *rect)
> {
> - struct cirrus_device *cirrus = fb->dev->dev_private;
> + struct cirrus_device *cirrus = to_cirrus(fb->dev);
> void *vmap;
> int idx, ret;
>
> @@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct
> drm_simple_display_pipe *pipe,
> struct drm_crtc_state *crtc_state,
> struct drm_plane_state *plane_state)
> {
> - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
> + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
>
> cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
> cirrus_fb_blit_fullscreen(plane_state->fb);
> @@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct
> drm_simple_display_pipe *pipe,
> static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
> struct drm_plane_state *old_state)
> {
> - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
> + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
> struct drm_plane_state *state = pipe->plane.state;
> struct drm_crtc *crtc = &pipe->crtc;
> struct drm_rect rect;
> @@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
> return PTR_ERR(cirrus);
>
> dev = &cirrus->dev;
> - dev->dev_private = cirrus;
>
> cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 01/18] drm/i915/display/icl_dsi: Prefer drm_WARN_ON over WARN_ON
Hi Pankaj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20200406]
[cannot apply to v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Pankaj-Bharadiya/Prefer-drm_WARN-over-WARN/20200406-210932
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-f002-20200406 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project
a43e23360657e3df82af6b96b403d1a5a3174744)
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/icl_dsi.c:1127:14: error: use of undeclared
>> identifier 'state'
drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
^
>> drivers/gpu/drm/i915/display/icl_dsi.c:1127:14: error: use of undeclared
>> identifier 'state'
2 errors generated.
vim +/state +1127 drivers/gpu/drm/i915/display/icl_dsi.c
1120
1121 static void gen11_dsi_enable(struct intel_encoder *encoder,
1122 const struct intel_crtc_state *crtc_state,
1123 const struct drm_connector_state
*conn_state)
1124 {
1125 struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
1126
> 1127 drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
1128
1129 /* step6d: enable dsi transcoder */
1130 gen11_dsi_enable_transcoder(encoder);
1131
1132 /* step7: enable backlight */
1133 intel_panel_enable_backlight(crtc_state, conn_state);
1134 intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
1135
1136 intel_crtc_vblank_on(crtc_state);
1137 }
1138
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gitlab.fd.o financial situation and impact on services
On Mon, Apr 6, 2020 at 10:04 AM Michel Dänzer wrote: > > On 2020-04-06 6:34 p.m., Rob Clark wrote: > > > > The ideal thing would be to be able to click any jobs that we want to > > run, say "arm64_a630_gles31", and for gitlab to realize that it needs > > to automatically trigger dependencies of that job (meson-arm64, and > > arm_build+arm_test). But not sure if that is a thing gitlab can do. > > Not that I know of. The dependency handling is still pretty rudimentary > in general. > > > > Triggering just the first container jobs and having everything from > > there run automatically would be ok if the current rules to filter out > > unneeded jobs still apply, ie. a panfrost change isn't triggering > > freedreno CI jobs and visa versa. But tbh I don't understand enough > > of what that MR is doing to understand if that is what it does. (It > > was suggested on IRC that this is probably what it does.) > > It is. Filtered jobs don't exist at all in the pipeline, so they can't > be triggered by any means. :) > ahh, ok, I didn't realize that.. thx for the explaination BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/10] drm/fb-helper: Remove return value from drm_fbdev_generic_setup()
Generic fbdev emulation is a DRM client. Drivers should invoke the
setup function, but not depend on its success. Hence remove the return
value.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_fb_helper.c | 18 --
include/drm/drm_fb_helper.h | 5 +++--
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 165c8dab50797..24db97eee53d4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2186,11 +2186,9 @@ static const struct drm_client_funcs
drm_fbdev_client_funcs = {
* Setup will be retried on the next hotplug event.
*
* The fbdev is destroyed by drm_dev_unregister().
- *
- * Returns:
- * Zero on success or negative error code on failure.
*/
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp)
+void drm_fbdev_generic_setup(struct drm_device *dev,
+unsigned int preferred_bpp)
{
struct drm_fb_helper *fb_helper;
int ret;
@@ -2198,17 +2196,19 @@ int drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp)
WARN(dev->fb_helper, "fb_helper is already set!\n");
if (!drm_fbdev_emulation)
- return 0;
+ return;
fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
- if (!fb_helper)
- return -ENOMEM;
+ if (!fb_helper) {
+ drm_err(dev, "Failed to allocate fb_helper\n");
+ return;
+ }
ret = drm_client_init(dev, &fb_helper->client, "fbdev",
&drm_fbdev_client_funcs);
if (ret) {
kfree(fb_helper);
drm_err(dev, "Failed to register client: %d\n", ret);
- return ret;
+ return;
}
if (!preferred_bpp)
@@ -,8 +,6 @@ int drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp)
drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
drm_client_register(&fb_helper->client);
-
- return 0;
}
EXPORT_SYMBOL(drm_fbdev_generic_setup);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 208dbf87afa3e..fb037be83997d 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -269,7 +269,8 @@ int drm_fb_helper_debug_leave(struct fb_info *info);
void drm_fb_helper_lastclose(struct drm_device *dev);
void drm_fb_helper_output_poll_changed(struct drm_device *dev);
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int
preferred_bpp);
+void drm_fbdev_generic_setup(struct drm_device *dev,
+unsigned int preferred_bpp);
#else
static inline void drm_fb_helper_prepare(struct drm_device *dev,
struct drm_fb_helper *helper,
@@ -443,7 +444,7 @@ static inline void drm_fb_helper_output_poll_changed(struct
drm_device *dev)
{
}
-static inline int
+static inline void
drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp)
{
return 0;
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH/RFC 4/6] dt-bindings: display: rockchip: dw-hdmi: Convert binding to YAML
Hi Maxime,
On Mon, Apr 06, 2020 at 07:09:15PM +0200, Maxime Ripard wrote:
> On Mon, Apr 06, 2020 at 02:19:27PM +0300, Laurent Pinchart wrote:
> > On Mon, Apr 06, 2020 at 10:00:32AM +0200, Maxime Ripard wrote:
> > > On Mon, Apr 06, 2020 at 02:39:33AM +0300, Laurent Pinchart wrote:
> > > > Convert the Rockchip HDMI TX text binding to YAML.
> > > >
> > > > Signed-off-by: Laurent Pinchart
> > > >
> > > > ---
> > > > .../display/rockchip/dw_hdmi-rockchip.txt | 74
> > > > .../display/rockchip/rockchip,dw-hdmi.yaml| 178 ++
> > > > 2 files changed, 178 insertions(+), 74 deletions(-)
> > > > delete mode 100644
> > > > Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> > > >
> > > > b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> > > > deleted file mode 100644
> > > > index 3d32ce137e7f..
> > > > ---
> > > > a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> > > > +++ /dev/null
> > > > @@ -1,74 +0,0 @@
> > > > -Rockchip DWC HDMI TX Encoder
> > > > -
> > > > -
> > > > -The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
> > > > -with a companion PHY IP.
> > > > -
> > > > -These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> > > > -Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
> > > > -following device-specific properties.
> > > > -
> > > > -
> > > > -Required properties:
> > > > -
> > > > -- compatible: should be one of the following:
> > > > - "rockchip,rk3228-dw-hdmi"
> > > > - "rockchip,rk3288-dw-hdmi"
> > > > - "rockchip,rk3328-dw-hdmi"
> > > > - "rockchip,rk3399-dw-hdmi"
> > > > -- reg: See dw_hdmi.txt.
> > > > -- reg-io-width: See dw_hdmi.txt. Shall be 4.
> > > > -- interrupts: HDMI interrupt number
> > > > -- clocks: See dw_hdmi.txt.
> > > > -- clock-names: Shall contain "iahb" and "isfr" as defined in
> > > > dw_hdmi.txt.
> > > > -- ports: See dw_hdmi.txt. The DWC HDMI shall have a single port
> > > > numbered 0
> > > > - corresponding to the video input of the controller. The port shall
> > > > have two
> > > > - endpoints, numbered 0 and 1, connected respectively to the vopb and
> > > > vopl.
> > > > -- rockchip,grf: Shall reference the GRF to mux vopl/vopb.
> > > > -
> > > > -Optional properties
> > > > -
> > > > -- ddc-i2c-bus: The HDMI DDC bus can be connected to either a system
> > > > I2C master
> > > > - or the functionally-reduced I2C master contained in the DWC HDMI.
> > > > When
> > > > - connected to a system I2C master this property contains a phandle to
> > > > that
> > > > - I2C master controller.
> > > > -- clock-names: See dw_hdmi.txt. The "cec" clock is optional.
> > > > -- clock-names: May contain "cec" as defined in dw_hdmi.txt.
> > > > -- clock-names: May contain "grf", power for grf io.
> > > > -- clock-names: May contain "vpll", external clock for some hdmi phy.
> > > > -- phys: from general PHY binding: the phandle for the PHY device.
> > > > -- phy-names: Should be "hdmi" if phys references an external phy.
> > > > -
> > > > -Optional pinctrl entry:
> > > > -- If you have both a "unwedge" and "default" pinctrl entry, dw_hdmi
> > > > - will switch to the unwedge pinctrl state for 10ms if it ever gets an
> > > > - i2c timeout. It's intended that this unwedge pinctrl entry will
> > > > - cause the SDA line to be driven low to work around a hardware
> > > > - errata.
> > > > -
> > > > -Example:
> > > > -
> > > > -hdmi: hdmi@ff98 {
> > > > - compatible = "rockchip,rk3288-dw-hdmi";
> > > > - reg = <0xff98 0x2>;
> > > > - reg-io-width = <4>;
> > > > - ddc-i2c-bus = <&i2c5>;
> > > > - rockchip,grf = <&grf>;
> > > > - interrupts = ;
> > > > - clocks = <&cru PCLK_HDMI_CTRL>, <&cru SCLK_HDMI_HDCP>;
> > > > - clock-names = "iahb", "isfr";
> > > > - ports {
> > > > - hdmi_in: port {
> > > > - #address-cells = <1>;
> > > > - #size-cells = <0>;
> > > > - hdmi_in_vopb: endpoint@0 {
> > > > - reg = <0>;
> > > > - remote-endpoint = <&vopb_out_hdmi>;
> > > > - };
> > > > - hdmi_in_vopl: endpoint@1 {
> > > > - reg = <1>;
> > > > - remote-endpoint = <&vopl_out_hdmi>;
> > > > - };
> > > > - };
> > > > - };
> > > > -};
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > > >
> > > > b/Documentation/
[PATCH 07/10] drm/tilcdc: Set up fbdev after fully registering device
Generic fbdev support is a DRM client. Set it up after fully registering the new DRM device. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 78c1877d13a83..a5e9ee4c7fbf4 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -390,10 +390,9 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ret = drm_dev_register(ddev, 0); if (ret) goto init_failed; + priv->is_registered = true; drm_fbdev_generic_setup(ddev, bpp); - - priv->is_registered = true; return 0; init_failed: -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/10] drm/mgag200: Set up fbdev after registering device; remove error checks
Generic fbdev support is a DRM client. Set it up after registering the new DRM device. Remove the error checks as the driver's probe function should not depend on a DRM client's state. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ drivers/gpu/drm/mgag200/mgag200_main.c | 4 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 7a5bad2f57d70..3298b7ef18b03 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -77,6 +77,8 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mgag200_driver_unload; + drm_fbdev_generic_setup(dev, 0); + return 0; err_mgag200_driver_unload: diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index e278b6a547bde..b680cf47cbb94 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -181,10 +181,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) dev_warn(&dev->pdev->dev, "Could not initialize cursors. Not doing hardware cursors.\n"); - r = drm_fbdev_generic_setup(mdev->dev, 0); - if (r) - goto err_modeset; - return 0; err_modeset: -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/10] drm/vboxvideo: Set up fbdev after registering device; remove error checks
Generic fbdev support is a DRM client. Set it up after registering the new DRM device. Remove the error checks as the driver's probe function should not depend on a DRM client's state. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index d685ec197fa05..282348e071fe3 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -82,14 +82,12 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini; - ret = drm_fbdev_generic_setup(&vbox->ddev, 32); - if (ret) - goto err_irq_fini; - ret = drm_dev_register(&vbox->ddev, 0); if (ret) goto err_irq_fini; + drm_fbdev_generic_setup(&vbox->ddev, 32); + return 0; err_irq_fini: -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/10] drm/kirin: Set up fbdev after fully registering device
Generic fbdev support is a DRM client. Set it up after fully registering
the new DRM device.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index d3145ae877d74..981858cc8d2b5 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -277,8 +277,6 @@ static int kirin_drm_bind(struct device *dev)
if (ret)
goto err_kms_cleanup;
- drm_fbdev_generic_setup(drm_dev, 32);
-
/* connectors should be registered after drm device register */
if (driver_data->register_connects) {
ret = kirin_drm_connectors_register(drm_dev);
@@ -286,6 +284,8 @@ static int kirin_drm_bind(struct device *dev)
goto err_drm_dev_unregister;
}
+ drm_fbdev_generic_setup(drm_dev, 32);
+
return 0;
err_drm_dev_unregister:
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/10] drm/ingenic: Remove error check from fbdev setup
Remove the error check from the fbdev setup function. The function will print a warning. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 7f3f869f57b3f..d938f2b1a96f1 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -783,9 +783,7 @@ static int ingenic_drm_probe(struct platform_device *pdev) goto err_devclk_disable; } - ret = drm_fbdev_generic_setup(drm, 32); - if (ret) - dev_warn(dev, "Unable to start fbdev emulation: %i", ret); + drm_fbdev_generic_setup(drm, 32); return 0; -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 30/44] drm/qxl: Use devm_drm_dev_alloc
Am 03.04.20 um 15:58 schrieb Daniel Vetter:
> Also need to remove the drm_dev_put from the remove hook.
>
> Signed-off-by: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: spice-de...@lists.freedesktop.org
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
> drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
> drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
> 3 files changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 09102e2efabc..6b4ae4c5fb76 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> return -EINVAL; /* TODO: ENODEV ? */
> }
>
> - qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> - if (!qdev)
> + qdev = devm_drm_dev_alloc(&pdev->dev, &qxl_driver,
> + struct qxl_device, ddev);
> + if (IS_ERR(qdev)) {
> + pr_err("Unable to init drm dev");
> return -ENOMEM;
> + }
My feeling is that it is too early to allocate. Wouldn't it be better to
first do the pdev and conflicting-fb stuff and allocate right before
qxl_device_init() ?
Best regards
Thomas
>
> ret = pci_enable_device(pdev);
> if (ret)
> - goto free_dev;
> + return ret;
>
> ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> if (ret)
> @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> }
> }
>
> - ret = qxl_device_init(qdev, &qxl_driver, pdev);
> + ret = qxl_device_init(qdev, pdev);
> if (ret)
> goto put_vga;
>
> @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> disable_pci:
> pci_disable_device(pdev);
> -free_dev:
> - kfree(qdev);
> +
> return ret;
> }
>
> @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> drm_atomic_helper_shutdown(dev);
> if (is_vga(pdev))
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> - drm_dev_put(dev);
> }
>
> DEFINE_DRM_GEM_FOPS(qxl_fops);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 435126facc9b..86ac191d9205 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -276,8 +276,7 @@ struct qxl_device {
> extern const struct drm_ioctl_desc qxl_ioctls[];
> extern int qxl_max_ioctl;
>
> -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> - struct pci_dev *pdev);
> +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> void qxl_device_fini(struct qxl_device *qdev);
>
> int qxl_modeset_init(struct qxl_device *qdev);
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 9eed1a375f24..91a34dd835d7 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> }
>
> int qxl_device_init(struct qxl_device *qdev,
> - struct drm_driver *drv,
> struct pci_dev *pdev)
> {
> int r, sb;
>
> - r = drm_dev_init(&qdev->ddev, drv, &pdev->dev);
> - if (r) {
> - pr_err("Unable to init drm dev");
> - goto error;
> - }
> -
> qdev->ddev.pdev = pdev;
> pci_set_drvdata(pdev, &qdev->ddev);
> qdev->ddev.dev_private = qdev;
> - drmm_add_final_kfree(&qdev->ddev, qdev);
>
> mutex_init(&qdev->gem.mutex);
> mutex_init(&qdev->update_area_mutex);
> @@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
> qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base,
> pci_resource_len(pdev, 0));
> if (!qdev->vram_mapping) {
> pr_err("Unable to create vram_mapping");
> - r = -ENOMEM;
> - goto error;
> + return -ENOMEM;
> }
>
> if (pci_resource_len(pdev, 4) > 0) {
> @@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
> io_mapping_free(qdev->surface_mapping);
> vram_mapping_free:
> io_mapping_free(qdev->vram_mapping);
> -error:
> return r;
> }
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/10] drm/ast: Set up fbdev after registering device; remove error checks
Generic fbdev support is a DRM client. Set it up after registering the new DRM device. Remove the error checks as the driver's probe function should not depend on a DRM client's state. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_drv.c | 3 +++ drivers/gpu/drm/ast/ast_main.c | 5 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 30aa73a5d9b72..b7ba22dddcad9 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -32,6 +32,7 @@ #include #include +#include #include #include @@ -111,6 +112,8 @@ static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_ast_driver_unload; + drm_fbdev_generic_setup(dev, 32); + return 0; err_ast_driver_unload: diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 18a0a4ce00f6e..e5398e3dabe70 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -30,7 +30,6 @@ #include #include -#include #include #include #include @@ -512,10 +511,6 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags) drm_mode_config_reset(dev); - ret = drm_fbdev_generic_setup(dev, 32); - if (ret) - goto out_free; - return 0; out_free: kfree(ast); -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/10] drm/hibmc: Remove error check from fbdev setup
Generic fbdev support is a DRM client. Remove the error check as the
driver's probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 79a180ae4509f..a6fd0c29e5b89 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -307,11 +307,7 @@ static int hibmc_load(struct drm_device *dev)
/* reset all the states of crtc/plane/encoder/connector */
drm_mode_config_reset(dev);
- ret = drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth);
- if (ret) {
- DRM_ERROR("failed to initialize fbdev: %d\n", ret);
- goto err;
- }
+ drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth);
return 0;
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/10] Set up generic fbdev after registering device
Generic fbdev emulation is a DRM client. If possible, it should behave like userspace clients. Therefore it should not run before the driver registered the new DRM device. If the setup function fails, the driver should not report an error. This is a follow-up patchset to the discussion at [1]. I went through all calls to drm_fbdev_generic_setup(), moved them to the final operation of their driver's probe function, and removed the return value. Built-tested on x86-64, aarch64 and arm. [1] https://lore.kernel.org/dri-devel/20200403135828.2542770-1-daniel.vet...@ffwll.ch/T/#m216b5b37aeeb7b28d55ad73b7a702b3d1d7bf867 Thomas Zimmermann (10): drm/ast: Set up fbdev after registering device; remove error checks drm/hibmc: Remove error check from fbdev setup drm/kirin: Set up fbdev after fully registering device drm/ingenic: Remove error check from fbdev setup drm/mediathek: Remove error check from fbdev setup drm/mgag200: Set up fbdev after registering device; remove error checks drm/tilcdc: Set up fbdev after fully registering device drm/udl: Remove error check from fbdev setup drm/vboxvideo: Set up fbdev after registering device; remove error checks drm/fb-helper: Remove return value from drm_fbdev_generic_setup() drivers/gpu/drm/ast/ast_drv.c | 3 +++ drivers/gpu/drm/ast/ast_main.c | 5 - drivers/gpu/drm/drm_fb_helper.c| 18 -- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c| 6 +- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c| 4 ++-- drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +--- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ drivers/gpu/drm/mgag200/mgag200_main.c | 4 drivers/gpu/drm/tilcdc/tilcdc_drv.c| 3 +-- drivers/gpu/drm/udl/udl_drv.c | 6 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++ include/drm/drm_fb_helper.h| 5 +++-- 13 files changed, 25 insertions(+), 45 deletions(-) -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/44] drm/udl: Use demv_drm_dev_alloc
Am 05.04.20 um 12:18 schrieb Noralf Trønnes:
>
>
> Den 03.04.2020 15.57, skrev Daniel Vetter:
>> Also init the fbdev emulation before we register the device, that way
>> we can rely on the auto-cleanup and simplify the probe error code even
>> more.
>>
>> Signed-off-by: Daniel Vetter
>> Cc: Dave Airlie
>> Cc: Sean Paul
>> Cc: Thomas Zimmermann
>> Cc: Daniel Vetter
>> Cc: Emil Velikov
>> Cc: Sam Ravnborg
>> Cc: Thomas Gleixner
>> ---
>> drivers/gpu/drm/udl/udl_drv.c | 36 +++
>> 1 file changed, 11 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
>> index 1ce2d865c36d..4ba5149fdd57 100644
>> --- a/drivers/gpu/drm/udl/udl_drv.c
>> +++ b/drivers/gpu/drm/udl/udl_drv.c
>> @@ -57,27 +57,20 @@ static struct udl_device *udl_driver_create(struct
>> usb_interface *interface)
>> struct udl_device *udl;
>> int r;
>>
>> -udl = kzalloc(sizeof(*udl), GFP_KERNEL);
>> -if (!udl)
>> -return ERR_PTR(-ENOMEM);
>> -
>> -r = drm_dev_init(&udl->drm, &driver, &interface->dev);
>> -if (r) {
>> -kfree(udl);
>> -return ERR_PTR(r);
>> -}
>> +udl = devm_drm_dev_alloc(&interface->dev, &driver,
>> + struct udl_device, drm);
>> +if (IS_ERR(udl))
>> +return udl;
>>
>> udl->udev = udev;
>> udl->drm.dev_private = udl;
>> -drmm_add_final_kfree(&udl->drm, udl);
>>
>> r = udl_init(udl);
>> -if (r) {
>> -drm_dev_put(&udl->drm);
>> +if (r)
>> return ERR_PTR(r);
>> -}
>>
>> usb_set_intfdata(interface, udl);
>> +
>> return udl;
>> }
>>
>> @@ -91,23 +84,17 @@ static int udl_usb_probe(struct usb_interface *interface,
>> if (IS_ERR(udl))
>> return PTR_ERR(udl);
>>
>> +r = drm_fbdev_generic_setup(&udl->drm, 0);
>
> It doesn't feel right to have a client open the device before the DRM
> device itself is registered. I would prefer to keep it where it is but
Agreed. IMHO we should also go through drivers and make the fbdev setup
the final step everywhere.
Best regards
Thomas
> ignore any errors. A failing client shouldn't prevent the driver from
> probing. drm_fbdev_generic_setup() do print errors if it fails. So yeah,
> in hindsight I should have made drm_fbdev_generic_setup() return void.
>
> Noralf.
>
>> +if (r)
>> +return r;
>> +
>> r = drm_dev_register(&udl->drm, 0);
>> if (r)
>> -goto err_free;
>> +return r;
>>
>> DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
>>
>> -r = drm_fbdev_generic_setup(&udl->drm, 0);
>> -if (r)
>> -goto err_drm_dev_unregister;
>> -
>> return 0;
>> -
>> -err_drm_dev_unregister:
>> -drm_dev_unregister(&udl->drm);
>> -err_free:
>> -drm_dev_put(&udl->drm);
>> -return r;
>> }
>>
>> static void udl_usb_disconnect(struct usb_interface *interface)
>> @@ -117,7 +104,6 @@ static void udl_usb_disconnect(struct usb_interface
>> *interface)
>> drm_kms_helper_poll_fini(dev);
>> udl_drop_usb(dev);
>> drm_dev_unplug(dev);
>> -drm_dev_put(dev);
>> }
>>
>> /*
>>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/10] drm/udl: Remove error check from fbdev setup
Remove the error check from the fbdev setup function. The driver's
probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/udl/udl_drv.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 1ce2d865c36dc..9cc6d075cb402 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,14 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
- r = drm_fbdev_generic_setup(&udl->drm, 0);
- if (r)
- goto err_drm_dev_unregister;
+ drm_fbdev_generic_setup(&udl->drm, 0);
return 0;
-err_drm_dev_unregister:
- drm_dev_unregister(&udl->drm);
err_free:
drm_dev_put(&udl->drm);
return r;
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/44] drm/vboxvideo: Stop using drm_device->dev_private
Hi
Am 03.04.20 um 15:57 schrieb Daniel Vetter:
> We use the baseclass pattern here, so lets to the proper (and more
> typesafe) upcasting.
>
> Signed-off-by: Daniel Vetter
> Cc: Hans de Goede
> ---
> drivers/gpu/drm/vboxvideo/vbox_drv.c | 1 -
> drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 +
> drivers/gpu/drm/vboxvideo/vbox_irq.c | 2 +-
> drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index be0600b22cf5..d34cddd809fd 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -52,7 +52,6 @@ static int vbox_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> return PTR_ERR(vbox);
>
> vbox->ddev.pdev = pdev;
> - vbox->ddev.dev_private = vbox;
> pci_set_drvdata(pdev, vbox);
> mutex_init(&vbox->hw_mutex);
>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> index 87421903816c..ac7c2effc46f 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> @@ -127,6 +127,7 @@ struct vbox_encoder {
> #define to_vbox_crtc(x) container_of(x, struct vbox_crtc, base)
> #define to_vbox_connector(x) container_of(x, struct vbox_connector, base)
> #define to_vbox_encoder(x) container_of(x, struct vbox_encoder, base)
> +#define to_vbox_dev(x) container_of(x, struct vbox_private, ddev)
I suggest ot call this macro to to_vbox_device(). At some point, we
should rename struct vbox_private to struct vbox_device for consistency
among drivers. The new macro's name would then fit.
For the overall patch:
Acked-by: Thomas Zimmermann
Best regards
Thomas
>
> bool vbox_check_supported(u16 id);
> int vbox_hw_init(struct vbox_private *vbox);
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_irq.c
> b/drivers/gpu/drm/vboxvideo/vbox_irq.c
> index 16a1e29f5292..631657fa554f 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_irq.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_irq.c
> @@ -34,7 +34,7 @@ void vbox_report_hotplug(struct vbox_private *vbox)
> irqreturn_t vbox_irq_handler(int irq, void *arg)
> {
> struct drm_device *dev = (struct drm_device *)arg;
> - struct vbox_private *vbox = (struct vbox_private *)dev->dev_private;
> + struct vbox_private *vbox = to_vbox_dev(dev);
> u32 host_flags = vbox_get_flags(vbox);
>
> if (!(host_flags & HGSMIHOSTFLAGS_IRQ))
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index 0883a435e62b..d9a5af62af89 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -36,7 +36,7 @@ static void vbox_do_modeset(struct drm_crtc *crtc)
> u16 flags;
> s32 x_offset, y_offset;
>
> - vbox = crtc->dev->dev_private;
> + vbox = to_vbox_dev(crtc->dev);
> width = vbox_crtc->width ? vbox_crtc->width : 640;
> height = vbox_crtc->height ? vbox_crtc->height : 480;
> bpp = fb ? fb->format->cpp[0] * 8 : 32;
> @@ -77,7 +77,7 @@ static void vbox_do_modeset(struct drm_crtc *crtc)
> static int vbox_set_view(struct drm_crtc *crtc)
> {
> struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> - struct vbox_private *vbox = crtc->dev->dev_private;
> + struct vbox_private *vbox = to_vbox_dev(crtc->dev);
> struct vbva_infoview *p;
>
> /*
> @@ -174,7 +174,7 @@ static void vbox_crtc_set_base_and_mode(struct drm_crtc
> *crtc,
> int x, int y)
> {
> struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]);
> - struct vbox_private *vbox = crtc->dev->dev_private;
> + struct vbox_private *vbox = to_vbox_dev(crtc->dev);
> struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> bool needs_modeset = drm_atomic_crtc_needs_modeset(crtc->state);
>
> @@ -272,7 +272,7 @@ static void vbox_primary_atomic_update(struct drm_plane
> *plane,
> {
> struct drm_crtc *crtc = plane->state->crtc;
> struct drm_framebuffer *fb = plane->state->fb;
> - struct vbox_private *vbox = fb->dev->dev_private;
> + struct vbox_private *vbox = to_vbox_dev(fb->dev);
> struct drm_mode_rect *clips;
> uint32_t num_clips, i;
>
> @@ -704,7 +704,7 @@ static int vbox_get_modes(struct drm_connector *connector)
> int preferred_width, preferred_height;
>
> vbox_connector = to_vbox_connector(connector);
> - vbox = connector->dev->dev_private;
> + vbox = to_vbox_dev(connector->dev);
>
> hgsmi_report_flags_location(vbox->guest_pool, GUEST_HEAP_OFFSET(vbox) +
> HOST_FLAGS_OFFSET);
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenP
[PATCH 05/10] drm/mediathek: Remove error check from fbdev setup
Remove the error check from the fbdev setup function. The function
will print a warning.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 2eaa9080d2505..ce570283b55f7 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -347,9 +347,7 @@ static int mtk_drm_bind(struct device *dev)
if (ret < 0)
goto err_deinit;
- ret = drm_fbdev_generic_setup(drm, 32);
- if (ret)
- DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
+ drm_fbdev_generic_setup(drm, 32);
return 0;
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207137] AMDGPU incorrectly reports vddgfx voltage for R9 390
https://bugzilla.kernel.org/show_bug.cgi?id=207137 --- Comment #3 from sandy.8...@gmail.com --- Created attachment 288235 --> https://bugzilla.kernel.org/attachment.cgi?id=288235&action=edit GPU VBIOS -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207137] AMDGPU incorrectly reports vddgfx voltage for R9 390
https://bugzilla.kernel.org/show_bug.cgi?id=207137 sandy.8...@gmail.com changed: What|Removed |Added CC||sandy.8...@gmail.com --- Comment #2 from sandy.8...@gmail.com --- Created attachment 288233 --> https://bugzilla.kernel.org/attachment.cgi?id=288233&action=edit dmesg output -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gitlab.fd.o financial situation and impact on services
On 2020-04-06 6:34 p.m., Rob Clark wrote: > > The ideal thing would be to be able to click any jobs that we want to > run, say "arm64_a630_gles31", and for gitlab to realize that it needs > to automatically trigger dependencies of that job (meson-arm64, and > arm_build+arm_test). But not sure if that is a thing gitlab can do. Not that I know of. The dependency handling is still pretty rudimentary in general. > Triggering just the first container jobs and having everything from > there run automatically would be ok if the current rules to filter out > unneeded jobs still apply, ie. a panfrost change isn't triggering > freedreno CI jobs and visa versa. But tbh I don't understand enough > of what that MR is doing to understand if that is what it does. (It > was suggested on IRC that this is probably what it does.) It is. Filtered jobs don't exist at all in the pipeline, so they can't be triggered by any means. :) -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: mediatek: fix device passed to cmdq
Hi, Hsin-Yi:
Hsin-Yi Wang 於 2020年4月6日 週一 下午1:12寫道：
>
> drm device is now probed from mmsys. We need to use mmsys device to get gce
> nodes. Fix following errors:
>
> [0.740068] mediatek-drm mediatek-drm.1.auto: error -2 can't parse
> gce-client-reg property (0)
> [0.748721] mediatek-drm mediatek-drm.1.auto: error -2 can't parse
> gce-client-reg property (0)
> ...
> [2.659645] mediatek-drm mediatek-drm.1.auto: failed to request channel
> [2.666270] mediatek-drm mediatek-drm.1.auto: failed to request channel
Reviewed-by: Chun-Kuang Hu
>
> Fixes: 1d367541aded ("soc / drm: mediatek: Fix mediatek-drm device probing")
> Signed-off-by: Hsin-Yi Wang
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 6 --
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 3 ++-
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 615a54e60fe2..8621f0289399 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -822,14 +822,16 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
>
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> mtk_crtc->cmdq_client =
> - cmdq_mbox_create(dev, drm_crtc_index(&mtk_crtc->base),
> + cmdq_mbox_create(mtk_crtc->mmsys_dev,
> +drm_crtc_index(&mtk_crtc->base),
> 2000);
> if (IS_ERR(mtk_crtc->cmdq_client)) {
> dev_dbg(dev, "mtk_crtc %d failed to create mailbox client,
> writing register by CPU now\n",
> drm_crtc_index(&mtk_crtc->base));
> mtk_crtc->cmdq_client = NULL;
> }
> - ret = of_property_read_u32_index(dev->of_node, "mediatek,gce-events",
> + ret = of_property_read_u32_index(mtk_crtc->mmsys_dev->of_node,
> +"mediatek,gce-events",
> drm_crtc_index(&mtk_crtc->base),
> &mtk_crtc->cmdq_event);
> if (ret)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index e2bb0d19ef99..dc78e86bccc0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -517,7 +517,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
> goto err_node;
> }
>
> - ret = mtk_ddp_comp_init(dev, node, comp, comp_id,
> NULL);
> + ret = mtk_ddp_comp_init(dev->parent, node, comp,
> + comp_id, NULL);
> if (ret) {
> of_node_put(node);
> goto err_node;
> --
> 2.26.0.292.g33ef6b2f38-goog
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services
On Mon, Apr 6, 2020 at 8:43 AM Adam Jackson wrote: > > On Sat, 2020-04-04 at 08:11 -0700, Rob Clark wrote: > > On Fri, Apr 3, 2020 at 7:12 AM Michel Dänzer wrote: > > > On 2020-03-01 6:46 a.m., Marek Olšák wrote: > > > > For Mesa, we could run CI only when Marge pushes, so that it's a > > > > strictly > > > > pre-merge CI. > > > > > > Thanks for the suggestion! I implemented something like this for Mesa: > > > > > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4432 > > > > I wouldn't mind manually triggering pipelines, but unless there is > > some trick I'm not realizing, it is super cumbersome. Ie. you have to > > click first the container jobs.. then wait.. then the build jobs.. > > then wait some more.. and then finally the actual runners. That would > > be a real step back in terms of usefulness of CI.. one might call it a > > regression :-( > > I think that's mostly a complaint about the conditionals we've written > so far, tbh. As I commented on the bug, when I clicked the container > job (which the rules happen to have evaluated to being "manual"), every > job (recursively) downstream of it got enqueued, which isn't what > you're describing. So I think if you can describe the UX you'd like we > can write rules to make that reality. Ok, I was fearing that we'd have to manually trigger each stage of dependencies in the pipeline. Which wouldn't be so bad except that gitlab makes you wait for the previous stage to complete before triggering the next one. The ideal thing would be to be able to click any jobs that we want to run, say "arm64_a630_gles31", and for gitlab to realize that it needs to automatically trigger dependencies of that job (meson-arm64, and arm_build+arm_test). But not sure if that is a thing gitlab can do. Triggering just the first container jobs and having everything from there run automatically would be ok if the current rules to filter out unneeded jobs still apply, ie. a panfrost change isn't triggering freedreno CI jobs and visa versa. But tbh I don't understand enough of what that MR is doing to understand if that is what it does. (It was suggested on IRC that this is probably what it does.) BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services
On Sat, 2020-04-04 at 08:11 -0700, Rob Clark wrote: > On Fri, Apr 3, 2020 at 7:12 AM Michel Dänzer wrote: > > On 2020-03-01 6:46 a.m., Marek Olšák wrote: > > > For Mesa, we could run CI only when Marge pushes, so that it's a strictly > > > pre-merge CI. > > > > Thanks for the suggestion! I implemented something like this for Mesa: > > > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4432 > > I wouldn't mind manually triggering pipelines, but unless there is > some trick I'm not realizing, it is super cumbersome. Ie. you have to > click first the container jobs.. then wait.. then the build jobs.. > then wait some more.. and then finally the actual runners. That would > be a real step back in terms of usefulness of CI.. one might call it a > regression :-( I think that's mostly a complaint about the conditionals we've written so far, tbh. As I commented on the bug, when I clicked the container job (which the rules happen to have evaluated to being "manual"), every job (recursively) downstream of it got enqueued, which isn't what you're describing. So I think if you can describe the UX you'd like we can write rules to make that reality. But I don't really know which jobs are most expensive in terms of bandwidth, or storage, or CPUs, and even if I knew those I don't know how to map those to currency. So I'm not sure if the UI we'd like would minimize the cost the way we'd like. - ajax ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/5] drm: bridge: dw_mipi_dsi: abstract register access using reg_fields
W dniu 30.03.2020 o 13:35, Adrian Ratiu pisze: > Register existence, address/offsets, field layouts, reserved bits and > so on differ between MIPI-DSI versions and between SoC vendor boards. > Despite these differences the hw IP and protocols are mostly the same > so the generic bridge can be made to compensate these differences. > > The current Rockchip and STM drivers hardcoded a lot of their common > definitions in the bridge code because they're based on DSI v1.30 and > 1.31 which are relatively close, but in order to support older/future > versions with more diverging layouts like the v1.01 present on imx6, > we abstract some of the register accesses via the regmap field APIs. > > The bridge detects the DSI core version and initializes the required > regmap register layout. Other DSI versions / register layouts can > easily be added in the future by only changing the bridge code. > > The platform drivers don't require any changes, DSI register layout > versioning will be handled transparently by the bridge, but if in > the future the regmap or layouts needs to be exposed to the drivres, > it could easily be done via plat_data or a new API in dw_mipi_dsi.h. > > Suggested-by: Boris Brezillon > Reviewed-by: Emil Velikov > Signed-off-by: Adrian Ratiu > --- > Changes since v4: >- Move regmap infrastructure logic to a separate commit (Ezequiel) >- Consolidate field infrastructure in this commit (Ezequiel) >- Move the dsi v1.01 layout logic to a separate commit (Ezequiel) > > Changes since v2: >- Added const declarations to dw_mipi_dsi structs (Emil) >- Fixed commit tags (Emil) > > Changes since v1: >- Moved register definitions & regmap initialization into bridge >module. Platform drivers get the regmap via plat_data after calling >the bridge probe (Emil). > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 510 -- > 1 file changed, 352 insertions(+), 158 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index 6d9e2f21c9cc..5b78ff925af0 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -31,6 +31,7 @@ > #include > > #define HWVER_131 0x31333100 /* IP version 1.31 */ > +#define HWVER_1300x31333000 /* IP version 1.30 */ > > #define DSI_VERSION 0x00 > #define VERSION GENMASK(31, 8) > @@ -47,7 +48,6 @@ > #define DPI_VCID(vcid) ((vcid) & 0x3) > > #define DSI_DPI_COLOR_CODING0x10 > -#define LOOSELY18_EN BIT(8) > #define DPI_COLOR_CODING_16BIT_10x0 > #define DPI_COLOR_CODING_16BIT_20x1 > #define DPI_COLOR_CODING_16BIT_30x2 > @@ -56,11 +56,6 @@ > #define DPI_COLOR_CODING_24BIT 0x5 > > #define DSI_DPI_CFG_POL 0x14 > -#define COLORM_ACTIVE_LOWBIT(4) > -#define SHUTD_ACTIVE_LOW BIT(3) > -#define HSYNC_ACTIVE_LOW BIT(2) > -#define VSYNC_ACTIVE_LOW BIT(1) > -#define DATAEN_ACTIVE_LOWBIT(0) > > #define DSI_DPI_LP_CMD_TIM 0x18 > #define OUTVACT_LPCMD_TIME(p) (((p) & 0xff) << 16) > @@ -81,27 +76,19 @@ > #define DSI_GEN_VCID0x30 > > #define DSI_MODE_CFG0x34 > -#define ENABLE_VIDEO_MODE0 > -#define ENABLE_CMD_MODE BIT(0) > > #define DSI_VID_MODE_CFG0x38 > -#define ENABLE_LOW_POWER (0x3f << 8) > -#define ENABLE_LOW_POWER_MASK(0x3f << 8) > +#define ENABLE_LOW_POWER 0x3f > + > #define VID_MODE_TYPE_NON_BURST_SYNC_PULSES 0x0 > #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS 0x1 > #define VID_MODE_TYPE_BURST 0x2 > -#define VID_MODE_TYPE_MASK 0x3 > -#define VID_MODE_VPG_ENABLE BIT(16) > -#define VID_MODE_VPG_HORIZONTAL BIT(24) > > #define DSI_VID_PKT_SIZE0x3c > -#define VID_PKT_SIZE(p) ((p) & 0x3fff) > > #define DSI_VID_NUM_CHUNKS 0x40 > -#define VID_NUM_CHUNKS(c)((c) & 0x1fff) > > #define DSI_VID_NULL_SIZE 0x44 > -#define VID_NULL_SIZE(b) ((b) & 0x1fff) > > #define DSI_VID_HSA_TIME0x48 > #define DSI_VID_HBP_TIME0x4c > @@ -125,7 +112,6 @@ > #define GEN_SW_2P_TX_LP BIT(10) > #define GEN_SW_1P_TX_LP BIT(9) > #define GEN_SW_0P_TX_LP BIT(8) > -#define ACK_RQST_EN BIT(1) > #define TEAR_FX_EN BIT(0) > > #define CMD_MODE_ALL_LP (MAX_RD_PKT_SIZE_LP | \ > @@ -154,8 +140,6 @@ > #define GEN_CMD_EMPTY BIT(0) > > #define DSI_TO_CNT_CFG 0x7
[Bug 207137] AMDGPU incorrectly reports vddgfx voltage for R9 390
https://bugzilla.kernel.org/show_bug.cgi?id=207137 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- Can you attach a copy of your vbios and dmesg output? You can dump a copy of your vbios using the following: sudo cat /sys/kernel/debug/dri/0/amdgpu_vbios > /tmp/vbios.rom -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207137] New: AMDGPU incorrectly reports vddgfx voltage for R9 390
https://bugzilla.kernel.org/show_bug.cgi?id=207137 Bug ID: 207137 Summary: AMDGPU incorrectly reports vddgfx voltage for R9 390 Product: Drivers Version: 2.5 Kernel Version: 5.6 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: low Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: sandy.8...@gmail.com Regression: No I'm using an R9 390 GPU. vddgfx voltage is always reported as 1 Volt, regardless of load. I debugged and found that in smu7_hwmgr.c , in funcion smu7_read_sensor(), the voltage is read from PLANE2_VID when a certain condition is true. This always results in a final value of 1 Volt. When I changed the code to read only from PLANE1_VID, the correct voltage seems to be reported, and it changes under load, from ~900 mv to 1.3 V. Please fix. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/5] Genericize DW MIPI DSI bridge and add i.MX 6 driver
Hi Adrian, Due to different ways of work I use different mail client, so forgive me if there are some misconfugrations. W dniu 30.03.2020 o 13:35, Adrian Ratiu pisze: > Hello everyone, > > The v5 series is a significantly cleaned up version from v4, > started by Ezequiel Garcia's suggestion of splitting out the > regmap infrastructure from the drivers (thank you!). > > Turns out no changes are required to the existing drivers and > the bridge can transparently take care of the layout logic, > so there's no need to expose the regmap via plat_data anymore. > > Starting from this version I also opted to add per-patch > changelogs. All review comments up to now have been addressed. > > Tested on IMX6DL. > > Adrian Ratiu (5): >drm: bridge: dw_mipi_dsi: add initial regmap infrastructure >drm: bridge: dw_mipi_dsi: abstract register access using reg_fields >drm: bridge: synopsis: add dsi v1.01 support >drm: imx: Add i.MX 6 MIPI DSI host platform driver >dt-bindings: display: add i.MX6 MIPI DSI host controller doc > > .../display/imx/fsl,mipi-dsi-imx6.yaml| 134 > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 683 +- So we have above 400 lines more just to add slightly different register layout of v1.01. Quite big linecount for apparently small (?) functional change - I was too lazy to check how many reg fields are really used (some are not used at all), but it does not seem to be big enough to justyfy so big change IMO. I will add more comments in specific patches. Regards Andrzej > drivers/gpu/drm/imx/Kconfig | 7 + > drivers/gpu/drm/imx/Makefile | 1 + > drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c| 399 ++ > 5 files changed, 1049 insertions(+), 175 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml > create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: KMS enums and bitfields UAPI
On Friday, April 3, 2020 4:23 PM, Adam Jackson wrote: > On Fri, 2020-04-03 at 15:24 +0300, Pekka Paalanen wrote: > > > On Fri, 03 Apr 2020 10:15:21 + Simon Ser cont...@emersion.fr wrote: > > > > > Additionally, I've heard Pekka saying that it would be nice to have > > > constants > > > for property names in the UAPI headers. Indeed, this would prevent > > > hard-to-debug typo issues. I have a very good example of such a typo > > > issue that > > > took literally hours to debug, with X11 atoms which also use free-form > > > strings > > > like KMS properties [3]. > > > If we have constants for property names in UAPI headers, it wouldn't be a > > > big > > > hurdle to also have constants for enum values alongside. > > > > To clarify, the property names would be of the form > > #define DRM_KMS_PROPERTY_KERBLAH "KerBlah" > > while enum values would be integers, i.e. the raw values. > > Daniel Stone did have a good counter-argument to defining property > > names: userspace would be full of > > #ifndef DRM_KMS_PROPERTY_KERBLAH > > #define DRM_KMS_PROPERTY_KERBLAH "KerBleh" > > #endif > > anyway as long as they cannot rely on the headers to be recent enough. > > (The typo is intentional.) > > Why not put this in some external registry and regularly sync it into > drm-next? This seems like an xorgproto kind of problem to me. How would that fix the issue of KMS clients which don't want to depend on too recent dependencies? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/44] drivers/base: Always release devres on device_del
On Mon, Apr 6, 2020 at 3:38 PM Greg Kroah-Hartman wrote: > > On Mon, Apr 06, 2020 at 02:32:51PM +0200, Daniel Vetter wrote: > > On Fri, Apr 3, 2020 at 3:58 PM Daniel Vetter wrote: > > > > > > In drm we've added nice drm_device (the main gpu driver thing, which > > > also represents the userspace interfaces and has everything else > > > dangling off it) init functions using devres, devm_drm_dev_init and > > > soon devm_drm_dev_alloc (this patch series adds that). > > > > > > A slight trouble is that drm_device itself holds a reference on the > > > struct device it's sitting on top (for sysfs links and dmesg debug and > > > lots of other things), so there's a reference loop. For real drivers > > > this is broken at remove/unplug time, where all devres resources are > > > released device_release_driver(), before the final device reference is > > > dropped. So far so good. > > > > > > There's 2 exceptions: > > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual > > > platform device to make them look more like normal devices to > > > userspace. These aren't drivers in the driver model sense, we simple > > > create a platform_device and register it. > > > > > > - drm/i915/selftests, where we create minimal mock devices, and again > > > the selftests aren't proper drivers in the driver model sense. > > > > > > For these two cases the reference loop isn't broken, because devres is > > > only cleaned up when the last device reference is dropped. But that's > > > not happening, because the drm_device holds that last struct device > > > reference. > > > > > > Thus far this wasn't a problem since the above cases simply > > > hand-rolled their cleanup code. But I want to convert all drivers over > > > to the devm_ versions, hence it would be really nice if these > > > virtual/fake/mock uses-cases could also be managed with devres > > > cleanup. > > > > > > I see three possible approaches: > > > > Restarting this at the top level, because the discussion thus far just > > ended in a long "you're doing it wrong", despite that I think we're > > doing what v4l is doing (plus/minus that we can't do an exact matching > > handling in drm because our uapi has a lot more warts, which we can't > > change because no breaking userspace). > > > > So which one of the three below is the right approach? > > > > Aside, looking at the v4l solution I think there's also a confusion > > about struct device representing a char device (which v4l directly > > uses as its userspace interface refcounted thing, and which drm does > > _not_ directly). And a struct device embedded into something like > > platform_device or a virtual device, where a driver can bind to. My > > question here is about the former, I don't care how cdev struct device > > are cleaned up one bit. Now if other subsystems relies on the devres > > cleanup behaviour we currently have because of such cdev usage, then > > yeah first approach doesn't work (and I have a big surprised that use > > case, but hey would actually learn something). > > > > End of aside, since again I want to figure out which of the tree > > approaches it the right one. Not about how wrong one of them is, > > ignoring the other three I laid out. And maybe there's even more > > options for this. > > Sorry, been swamped with other things, give me a few days to get back to > this, I need to dig into how you all are dealing with the virtual > drivers. Sure, no problem. > Doing this in the middle of the merge window is a bit rough :) Ah I always forget ... we freeze drm at -rc6, so merge window is actually my most relaxed time since everyone is busy and no one has time to report drm bugs :-) Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/44] drivers/base: Always release devres on device_del
On Mon, Apr 06, 2020 at 02:32:51PM +0200, Daniel Vetter wrote: > On Fri, Apr 3, 2020 at 3:58 PM Daniel Vetter wrote: > > > > In drm we've added nice drm_device (the main gpu driver thing, which > > also represents the userspace interfaces and has everything else > > dangling off it) init functions using devres, devm_drm_dev_init and > > soon devm_drm_dev_alloc (this patch series adds that). > > > > A slight trouble is that drm_device itself holds a reference on the > > struct device it's sitting on top (for sysfs links and dmesg debug and > > lots of other things), so there's a reference loop. For real drivers > > this is broken at remove/unplug time, where all devres resources are > > released device_release_driver(), before the final device reference is > > dropped. So far so good. > > > > There's 2 exceptions: > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual > > platform device to make them look more like normal devices to > > userspace. These aren't drivers in the driver model sense, we simple > > create a platform_device and register it. > > > > - drm/i915/selftests, where we create minimal mock devices, and again > > the selftests aren't proper drivers in the driver model sense. > > > > For these two cases the reference loop isn't broken, because devres is > > only cleaned up when the last device reference is dropped. But that's > > not happening, because the drm_device holds that last struct device > > reference. > > > > Thus far this wasn't a problem since the above cases simply > > hand-rolled their cleanup code. But I want to convert all drivers over > > to the devm_ versions, hence it would be really nice if these > > virtual/fake/mock uses-cases could also be managed with devres > > cleanup. > > > > I see three possible approaches: > > Restarting this at the top level, because the discussion thus far just > ended in a long "you're doing it wrong", despite that I think we're > doing what v4l is doing (plus/minus that we can't do an exact matching > handling in drm because our uapi has a lot more warts, which we can't > change because no breaking userspace). > > So which one of the three below is the right approach? > > Aside, looking at the v4l solution I think there's also a confusion > about struct device representing a char device (which v4l directly > uses as its userspace interface refcounted thing, and which drm does > _not_ directly). And a struct device embedded into something like > platform_device or a virtual device, where a driver can bind to. My > question here is about the former, I don't care how cdev struct device > are cleaned up one bit. Now if other subsystems relies on the devres > cleanup behaviour we currently have because of such cdev usage, then > yeah first approach doesn't work (and I have a big surprised that use > case, but hey would actually learn something). > > End of aside, since again I want to figure out which of the tree > approaches it the right one. Not about how wrong one of them is, > ignoring the other three I laid out. And maybe there's even more > options for this. Sorry, been swamped with other things, give me a few days to get back to this, I need to dig into how you all are dealing with the virtual drivers. Doing this in the middle of the merge window is a bit rough :) thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: upstream boot error: KASAN: slab-out-of-bounds Write in virtio_gpu_object_create
Hi,
> > > +drivers/gpu/drm/virtio/virtgpu_object.c maintainers
> > > Now we have both mainline and linux-next boot broken (linux-next is
> > > broken for the past 40 days).
> > > No testing of new code happens.
> > >
> > > > virtio_gpu_object_shmem_init
> > > > drivers/gpu/drm/virtio/virtgpu_object.c:151 [inline]
> > > > virtio_gpu_object_create+0x9f3/0xaa0
> > > > drivers/gpu/drm/virtio/virtgpu_object.c:230
> >
> > Ah, that one.
> >
> > broken patch: f651c8b05542 ("drm/virtio: factor out the sg_table from
> > virtio_gpu_object")
> > fixed by: 0666a8d7f6a4 ("drm/virtio: fix OOB in virtio_gpu_object_create")
> >
> > Both are in drm-misc-next. I suspect the fix was added after
> > drm-misc-next was closed for the 5.7 merge window and thus should
> > have been submitted to drm-misc-next-fixes instead.
> >
> > So, what to do now? Should I cherry-pick 0666a8d7f6a4 into
> > drm-misc-next-fixes? Or should it go into drm-misc-fixes instead?
>
> Yup cherry-pick it over, with -x, to drm-misc-next-fixes.
> -Daniel
Done. So the next linux-next build should be green again. mainline
should get the fix with the next drm pull (may take a few days).
take care,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
On Mon, 2020-04-06 at 13:44 +0300, Andy Shevchenko wrote:
> On Mon, Apr 6, 2020 at 10:46 AM Mauro Carvalho Chehab
> wrote:
> > Em Fri, 3 Apr 2020 21:32:42 +0300
> > Andy Shevchenko escreveu:
> > > On Fri, Apr 3, 2020 at 8:54 PM Joe Perches wrote:
> > > > On Fri, 2020-04-03 at 19:32 +0200, Mauro Carvalho Chehab wrote:
> > > > > Em Fri, 03 Apr 2020 09:56:42 -0700
> > > > > Joe Perches escreveu:
> > > > It _might_ be useful to use a CONFIG_MEDIA_SUPPORT guard
> > > > in lib/vsprintf for this.
> > >
> > > No need. FourCC, if Sakari makes it more generic, can be used for
> > > other purposes, e.g. printing component names from the chips (not
> > > related to media at all).
> > >
> >
> > Hmm... not 100% sure about what you're meaning with "component names".
>
> 4cc is pretty much wide standard, media is just one of (famous) users of it.
>
> As I emphasized the example I referring to has nothing to do with media.
>
> Now, I have already two examples:
> - component name inside hardware register (used by Synopsys DesignWare)
> - CSRT table in ACPI uses this code for vendor ID.
So if this is really u32_to_ascii, perhaps the "-BE" bit
should be separated and "%4pEp" could be used with some
renamed inline used like ERR_PTR so maybe something like
this might work?
static inline void * __must_check FOURCC(u32 val)
{
return (void *)(unsigned long)cpu_to_be32(val);
}
void test_4cc(void)
{
u32 val = 0x41424344;
printk("4cc like: %4pE\n", FOURCC(val));
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: upstream boot error: KASAN: slab-out-of-bounds Write in virtio_gpu_object_create
On Mon, Apr 6, 2020 at 10:06 AM Gerd Hoffmann wrote:
>
> On Mon, Apr 06, 2020 at 09:07:44AM +0200, Dmitry Vyukov wrote:
> > On Mon, Apr 6, 2020 at 8:46 AM syzbot
> > wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:ffc1c20c Merge tag 'for-5.7/dm-changes' of
> > > git://git.kerne..
> > > git tree: upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=1690471fe0
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=d6a1e2f9a9986236
> > > dashboard link:
> > > https://syzkaller.appspot.com/bug?extid=d3a7951ed361037407db
> > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+d3a7951ed36103740...@syzkaller.appspotmail.com
> >
> >
> > +drivers/gpu/drm/virtio/virtgpu_object.c maintainers
> > Now we have both mainline and linux-next boot broken (linux-next is
> > broken for the past 40 days).
> > No testing of new code happens.
> >
> > > virtio_gpu_object_shmem_init drivers/gpu/drm/virtio/virtgpu_object.c:151
> > > [inline]
> > > virtio_gpu_object_create+0x9f3/0xaa0
> > > drivers/gpu/drm/virtio/virtgpu_object.c:230
>
> Ah, that one.
>
> broken patch: f651c8b05542 ("drm/virtio: factor out the sg_table from
> virtio_gpu_object")
> fixed by: 0666a8d7f6a4 ("drm/virtio: fix OOB in virtio_gpu_object_create")
>
> Both are in drm-misc-next. I suspect the fix was added after
> drm-misc-next was closed for the 5.7 merge window and thus should
> have been submitted to drm-misc-next-fixes instead.
>
> So, what to do now? Should I cherry-pick 0666a8d7f6a4 into
> drm-misc-next-fixes? Or should it go into drm-misc-fixes instead?
Yup cherry-pick it over, with -x, to drm-misc-next-fixes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/44] drivers/base: Always release devres on device_del
On Fri, Apr 3, 2020 at 3:58 PM Daniel Vetter wrote:
>
> In drm we've added nice drm_device (the main gpu driver thing, which
> also represents the userspace interfaces and has everything else
> dangling off it) init functions using devres, devm_drm_dev_init and
> soon devm_drm_dev_alloc (this patch series adds that).
>
> A slight trouble is that drm_device itself holds a reference on the
> struct device it's sitting on top (for sysfs links and dmesg debug and
> lots of other things), so there's a reference loop. For real drivers
> this is broken at remove/unplug time, where all devres resources are
> released device_release_driver(), before the final device reference is
> dropped. So far so good.
>
> There's 2 exceptions:
> - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> platform device to make them look more like normal devices to
> userspace. These aren't drivers in the driver model sense, we simple
> create a platform_device and register it.
>
> - drm/i915/selftests, where we create minimal mock devices, and again
> the selftests aren't proper drivers in the driver model sense.
>
> For these two cases the reference loop isn't broken, because devres is
> only cleaned up when the last device reference is dropped. But that's
> not happening, because the drm_device holds that last struct device
> reference.
>
> Thus far this wasn't a problem since the above cases simply
> hand-rolled their cleanup code. But I want to convert all drivers over
> to the devm_ versions, hence it would be really nice if these
> virtual/fake/mock uses-cases could also be managed with devres
> cleanup.
>
> I see three possible approaches:
Restarting this at the top level, because the discussion thus far just
ended in a long "you're doing it wrong", despite that I think we're
doing what v4l is doing (plus/minus that we can't do an exact matching
handling in drm because our uapi has a lot more warts, which we can't
change because no breaking userspace).
So which one of the three below is the right approach?
Aside, looking at the v4l solution I think there's also a confusion
about struct device representing a char device (which v4l directly
uses as its userspace interface refcounted thing, and which drm does
_not_ directly). And a struct device embedded into something like
platform_device or a virtual device, where a driver can bind to. My
question here is about the former, I don't care how cdev struct device
are cleaned up one bit. Now if other subsystems relies on the devres
cleanup behaviour we currently have because of such cdev usage, then
yeah first approach doesn't work (and I have a big surprised that use
case, but hey would actually learn something).
End of aside, since again I want to figure out which of the tree
approaches it the right one. Not about how wrong one of them is,
ignoring the other three I laid out. And maybe there's even more
options for this.
-Daniel
> - Clean up devres from device_del (or platform_device_unregister) even
> when no driver is bound. This seems like the simplest solution, but
> also the one with the widest impact, and what this patch implements.
> We might want to include more of the cleanup than just
> devres_release_all, but this is all I need to get my use case going.
>
> - Create a devres group and release that when we unbind. The code in
> virtual drivers gets a bit ugly, since every error case has a
> different cleanup code until we've chained everything
> (platform_device, devres group and then drm_device) together and a
> devres_release_group takes care of everything. Doable, but a bit
> confusing when I typed it out.
>
> - Convert the virtual drivers to real (in the device model sense)
> drivers. Feels like too much boilerplate for not much gain. And
> especially with the mock objects minimal mocking is kinda the goal,
> to limit the amount of accidentally pulled in code into our unit
> tests as much as possible.
>
> Either way I think time to discuss this a bit and figure out what's
> the recommended approach here.
>
> Signed-off-by: Daniel Vetter
> Cc: Greg Kroah-Hartman
> Cc: "Rafael J. Wysocki"
> ---
> drivers/base/dd.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index b25bcab2a26b..1bcfb0ff5f44 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -1155,6 +1155,8 @@ static void __device_release_driver(struct device *dev,
> struct device *parent)
> dev);
>
> kobject_uevent(&dev->kobj, KOBJ_UNBIND);
> + } else {
> + devres_release_all(dev);
> }
> }
>
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/44] drm: Add devm_drm_dev_alloc macro
On Mon, Apr 6, 2020 at 2:01 PM Laurent Pinchart
wrote:
>
> Hi Daniel,
>
> Thank you for the patch.
>
> On Fri, Apr 03, 2020 at 03:57:46PM +0200, Daniel Vetter wrote:
> > The kerneldoc is only added for this new function. Existing kerneldoc
> > and examples will be udated at the very end, since once all drivers
> > are converted over to devm_drm_dev_alloc we can unexport a lot of
> > interim functions and make the documentation for driver authors a lot
> > cleaner and less confusing. There will be only one true way to
> > initialize a drm_device at the end of this, which is going to be
> > devm_drm_dev_alloc.
>
> How about drivers that expose another interface towards userspace ? If
> the other related subsystem also required allocation of the driver
> private structure through its corresponding API, we'd be stuck. As
> stated before, I want this API to be optional.
So maybe we need to import a little bit more of our epic irc
discussion here, because this is leaving out all the context. There's
a few more things:
- Since the merging of the previous patch series this is already
mandatory, because the kfree(drm_device.managed.final_kfree) is the
last thing that will run. So I already killed your use-case (and it
seems to work just fine for all the drivers in-tree, and we have a
lot).
- As part of doing all these patches here and in the previous series
and in a next one I've seen that drivers just get this wrong. I'm
extremely hesitant to give that rope back to drivers, so I really
don't want to merge anything until we're ready to merge a driver that
needs it. I've set myself a goal to fix up all 40 odd drivers still
using drm_dev_alloc(), and that's itself already pretty stupid idea.
It's definitely not going to work if new drivers keep adding bad usage
patterns.
- Now I'm not opposed to allowing this, if/when we actually need it. I
think a very clean long-term solution would be to have a struct
kref_res, which augments a kref with automatic resource cleanup. We'd
probably need to fully demidlayer that, i.e. if you supply your own
->release hook then it's your job to call kref_release_all() at the
right point in there. With that we could then do a devm_drm_dev_init()
again, which would take that kref_res structure and the drm_device,
both embedded somewhere in your overall driver struture, and use your
drivers kref_res to attach all drm related auto-cleanup. In that case
it would then also be that kref_res' responsibility to do the final
kfree, hence drm wouldn't insist on doing that either. Prototype would
be something like:
devm_drm_dev_init(struct device *dev, struct drm_device *drm,
struct drm_driver *driver, struct kref_res *kref);
The trouble with the above idea is that it assumes I have endless
amounts of time and that I can convince Greg KH that I understand
driver unload lifetime issues. The former is atm the more realistic
looking one of these two, so interim solution would be to add some
hack or another meanwhile to do this within drm only. Or as an
alternative solution, easy to convert over to an eventual kref_res
world:
#define kref_res_get() drm_dev_get()
#define kref_res_put() drm_dev_put()
With suitable amounts of casting to make this work out correctly like
the real kref_res solution.
Until we do have such drivers though I really don't want to open the
barn door again to all the bugs that'll bring, while I'm trying to get
the other barn doors closed down and fixed meanwhile.
-Daniel
>
> > Cc: Paul Kocialkowski
> > Cc: Laurent Pinchart
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_drv.c | 23 +++
> > include/drm/drm_drv.h | 33 +
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 1bb4f636b83c..9e60b784b3ac 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -739,6 +739,29 @@ int devm_drm_dev_init(struct device *parent,
> > }
> > EXPORT_SYMBOL(devm_drm_dev_init);
> >
> > +void* __devm_drm_dev_alloc(struct device *parent, struct drm_driver
> > *driver,
> > +size_t size, size_t offset)
> > +{
> > + void *container;
> > + struct drm_device *drm;
> > + int ret;
> > +
> > + container = kzalloc(size, GFP_KERNEL);
> > + if (!container)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + drm = container + offset;
> > + ret = devm_drm_dev_init(parent, drm, driver);
> > + if (ret) {
> > + kfree(container);
> > + return ERR_PTR(ret);
> > + }
> > + drmm_add_final_kfree(drm, container);
> > +
> > + return container;
> > +}
> > +EXPORT_SYMBOL(__devm_drm_dev_alloc);
> > +
> > /**
> > * drm_dev_alloc - Allocate new DRM device
> > * @driver: DRM driver to allocate device for
> > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > index e7c6ea261ed1..26776be5a21e 100644
> > --- a/include/drm/drm_drv.h
>
Re: [PATCH 02/44] drm: Add devm_drm_dev_alloc macro
Hi Daniel,
Thank you for the patch.
On Fri, Apr 03, 2020 at 03:57:46PM +0200, Daniel Vetter wrote:
> The kerneldoc is only added for this new function. Existing kerneldoc
> and examples will be udated at the very end, since once all drivers
> are converted over to devm_drm_dev_alloc we can unexport a lot of
> interim functions and make the documentation for driver authors a lot
> cleaner and less confusing. There will be only one true way to
> initialize a drm_device at the end of this, which is going to be
> devm_drm_dev_alloc.
How about drivers that expose another interface towards userspace ? If
the other related subsystem also required allocation of the driver
private structure through its corresponding API, we'd be stuck. As
stated before, I want this API to be optional.
> Cc: Paul Kocialkowski
> Cc: Laurent Pinchart
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_drv.c | 23 +++
> include/drm/drm_drv.h | 33 +
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 1bb4f636b83c..9e60b784b3ac 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -739,6 +739,29 @@ int devm_drm_dev_init(struct device *parent,
> }
> EXPORT_SYMBOL(devm_drm_dev_init);
>
> +void* __devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver,
> +size_t size, size_t offset)
> +{
> + void *container;
> + struct drm_device *drm;
> + int ret;
> +
> + container = kzalloc(size, GFP_KERNEL);
> + if (!container)
> + return ERR_PTR(-ENOMEM);
> +
> + drm = container + offset;
> + ret = devm_drm_dev_init(parent, drm, driver);
> + if (ret) {
> + kfree(container);
> + return ERR_PTR(ret);
> + }
> + drmm_add_final_kfree(drm, container);
> +
> + return container;
> +}
> +EXPORT_SYMBOL(__devm_drm_dev_alloc);
> +
> /**
> * drm_dev_alloc - Allocate new DRM device
> * @driver: DRM driver to allocate device for
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index e7c6ea261ed1..26776be5a21e 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -626,6 +626,39 @@ int devm_drm_dev_init(struct device *parent,
> struct drm_device *dev,
> struct drm_driver *driver);
>
> +void* __devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver,
> +size_t size, size_t offset);
> +
> +/**
> + * devm_drm_dev_alloc - Resource managed allocation of a &drm_device instance
> + * @parent: Parent device object
> + * @driver: DRM driver
> + * @type: the type of the struct which contains struct &drm_device
> + * @member: the name of the &drm_device within @type.
> + *
> + * This allocates and initialize a new DRM device. No device registration is
> done.
> + * Call drm_dev_register() to advertice the device to user space and
> register it
> + * with other core subsystems. This should be done last in the device
> + * initialization sequence to make sure userspace can't access an
> inconsistent
> + * state.
> + *
> + * The initial ref-count of the object is 1. Use drm_dev_get() and
> + * drm_dev_put() to take and drop further ref-counts.
> + *
> + * It is recommended that drivers embed &struct drm_device into their own
> device
> + * structure.
> + *
> + * Note that this manages the lifetime of the resulting &drm_device
> + * automatically using devres. The DRM device initialized with this function
> is
> + * automatically put on driver detach using drm_dev_put().
> + *
> + * RETURNS:
> + * Pointer to new DRM device, or ERR_PTR on failure.
> + */
> +#define devm_drm_dev_alloc(parent, driver, type, member) \
> + ((type *) __devm_drm_dev_alloc(parent, driver, sizeof(type), \
> +offsetof(type, member)))
> +
> struct drm_device *drm_dev_alloc(struct drm_driver *driver,
>struct device *parent);
> int drm_dev_register(struct drm_device *dev, unsigned long flags);
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 16/44] drm/udl: don't set drm_device->dev_private
Am 03.04.20 um 15:58 schrieb Daniel Vetter:
> We're mostly there already, just a handful of places that didn't use
> the to_udl container_of cast. To make sure no new appear, don't set
> ->dev_private.
>
> Signed-off-by: Daniel Vetter
> Cc: Dave Airlie
> Cc: Sean Paul
> Cc: Emil Velikov
> Cc: Thomas Zimmermann
> Cc: Daniel Vetter
> Cc: Alexios Zavras
> Cc: Laurent Pinchart
> Cc: Thomas Gleixner
> Cc: "José Roberto de Souza"
> Cc: Sam Ravnborg
> Cc: Gerd Hoffmann
> Cc: Allison Randal
Reviewed-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/udl/udl_connector.c | 4 ++--
> drivers/gpu/drm/udl/udl_drv.c | 1 -
> drivers/gpu/drm/udl/udl_modeset.c | 6 +++---
> 3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_connector.c
> b/drivers/gpu/drm/udl/udl_connector.c
> index 0afdfb0d1fe1..cdc1c42e1669 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -59,7 +59,7 @@ static int udl_get_modes(struct drm_connector *connector)
> static enum drm_mode_status udl_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> - struct udl_device *udl = connector->dev->dev_private;
> + struct udl_device *udl = to_udl(connector->dev);
> if (!udl->sku_pixel_limit)
> return 0;
>
> @@ -72,7 +72,7 @@ static enum drm_mode_status udl_mode_valid(struct
> drm_connector *connector,
> static enum drm_connector_status
> udl_detect(struct drm_connector *connector, bool force)
> {
> - struct udl_device *udl = connector->dev->dev_private;
> + struct udl_device *udl = to_udl(connector->dev);
> struct udl_drm_connector *udl_connector =
> container_of(connector,
> struct udl_drm_connector,
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 4ba5149fdd57..126545428895 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -63,7 +63,6 @@ static struct udl_device *udl_driver_create(struct
> usb_interface *interface)
> return udl;
>
> udl->udev = udev;
> - udl->drm.dev_private = udl;
>
> r = udl_init(udl);
> if (r)
> diff --git a/drivers/gpu/drm/udl/udl_modeset.c
> b/drivers/gpu/drm/udl/udl_modeset.c
> index 8cad01f3d163..99518a826435 100644
> --- a/drivers/gpu/drm/udl/udl_modeset.c
> +++ b/drivers/gpu/drm/udl/udl_modeset.c
> @@ -215,7 +215,7 @@ static char *udl_dummy_render(char *wrptr)
> static int udl_crtc_write_mode_to_hw(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> - struct udl_device *udl = dev->dev_private;
> + struct udl_device *udl = to_udl(dev);
> struct urb *urb;
> char *buf;
> int retval;
> @@ -369,7 +369,7 @@ udl_simple_display_pipe_enable(struct
> drm_simple_display_pipe *pipe,
> struct drm_crtc *crtc = &pipe->crtc;
> struct drm_device *dev = crtc->dev;
> struct drm_framebuffer *fb = plane_state->fb;
> - struct udl_device *udl = dev->dev_private;
> + struct udl_device *udl = to_udl(dev);
> struct drm_display_mode *mode = &crtc_state->mode;
> char *buf;
> char *wrptr;
> @@ -464,7 +464,7 @@ static const struct drm_mode_config_funcs udl_mode_funcs
> = {
> int udl_modeset_init(struct drm_device *dev)
> {
> size_t format_count = ARRAY_SIZE(udl_simple_display_pipe_formats);
> - struct udl_device *udl = dev->dev_private;
> + struct udl_device *udl = to_udl(dev);
> struct drm_connector *connector;
> int ret;
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] dt-bindings: display: bridge: renesas,lvds: Convert binding to YAML
Hi Laurent,
On Mon, Apr 6, 2020 at 1:09 PM Laurent Pinchart
wrote:
> On Mon, Apr 06, 2020 at 10:47:37AM +0200, Geert Uytterhoeven wrote:
> > On Mon, Apr 6, 2020 at 1:24 AM Laurent Pinchart wrote:
> > > Convert the Renesas R-Car LVDS encoder text binding to YAML.
> > >
> > > Signed-off-by: Laurent Pinchart
> > >
> > > +if:
> > > + properties:
> > > +compatible:
> > > + enum:
> > > +- renesas,r8a774c0-lvds
> > > +- renesas,r8a77990-lvds
> > > +- renesas,r8a77995-lvds
> > > +then:
> > > + properties:
> > > +clocks:
> > > + minItems: 1
> > > + maxItems: 4
> > > + items:
> > > +- description: Functional clock
> > > +- description: EXTAL input clock
> > > +- description: DU_DOTCLKIN0 input clock
> > > +- description: DU_DOTCLKIN1 input clock
> > > +
> > > +clock-names:
> > > + minItems: 1
> > > + maxItems: 4
> > > + items:
> > > +- const: fck
> > > +# The LVDS encoder can use the EXTAL or DU_DOTCLKINx clocks.
> > > +# These clocks are optional.
> > > +- enum:
> > > + - extal
> > > + - dclkin.0
> > > + - dclkin.1
> > > +- enum:
> > > + - extal
> > > + - dclkin.0
> > > + - dclkin.1
> > > +- enum:
> > > + - extal
> > > + - dclkin.0
> > > + - dclkin.1
> >
> > Can the duplication of the last 3 entries be avoided?
> > Perhaps like in
> > Documentation/devicetree/bindings/serial/renesas,scif.yaml?
>
> I'd love to, if you can tell me how to make sure the fck entry is
> mandatory. The following
>
> minItems: 1
> maxItems: 4
> items:
> enum:
> - fck
> - extal
> - dclkin.0
> - dclkin.1
>
> passes the checks, but would accept
>
> clock-names = "extal";
>
> which is not valid. Your
> Documentation/devicetree/bindings/serial/renesas,scif.yaml bindings
> suffer from the same problem :-)
Hmm
> > > +examples:
> > > + - |
> > > +#include
> > > +#include
> > > +
> > > +lvds@feb9 {
> > > +compatible = "renesas,r8a7795-lvds";
> > > +reg = <0 0xfeb9 0 0x14>;
> >
> > Examples are built with #{address,size}-cells = <1>.
>
> Are they ? I don't get any failure from make dt_binding_check.
Hmm... And you do have "reg: maxItems: 1"...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 14/18] drm/i915/gem: Prefer drm_WARN* over WARN*
struct drm_device specific drm_WARN* macros include device information in the backtrace, so we know what device the warnings originate from. Prefer drm_WARN* over WARN* at places where struct drm_device pointer can be extracted. Signed-off-by: Pankaj Bharadiya --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 3 ++- drivers/gpu/drm/i915/gem/i915_gem_userptr.c| 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 9d11bad74e9a..d910eb9b77ef 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1440,7 +1440,7 @@ eb_relocate_entry(struct i915_execbuffer *eb, err = i915_vma_bind(target->vma, target->vma->obj->cache_level, PIN_GLOBAL, NULL); - if (WARN_ONCE(err, + if (drm_WARN_ONCE(&i915->drm, err, "Unexpected failure to bind target VMA!")) return err; } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c index 7fe9831aa9ba..4c1c7232b024 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c @@ -27,7 +27,8 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) void *dst; int i; - if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj))) + if (drm_WARN_ON(obj->base.dev, + i915_gem_object_needs_bit17_swizzle(obj))) return -EINVAL; /* diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 7ffd7afeb7a5..8b0708708671 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -235,7 +235,7 @@ i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, if (flags & I915_USERPTR_UNSYNCHRONIZED) return capable(CAP_SYS_ADMIN) ? 0 : -EPERM; - if (WARN_ON(obj->userptr.mm == NULL)) + if (drm_WARN_ON(obj->base.dev, obj->userptr.mm == NULL)) return -EINVAL; mn = i915_mmu_notifier_find(obj->userptr.mm); -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 17/18] drm/i915/pm: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Conversion is done with below sementic patch:
@@
identifier func, T;
@@
func(...) {
...
struct intel_crtc *T = ...;
<+...
-WARN_ON(
+drm_WARN_ON(T->base.dev,
...)
...+>
}
@@
identifier func, T;
@@
func(struct intel_crtc_state *T,...) {
<+...
-WARN_ON(
+drm_WARN_ON(T->uapi.crtc->dev,
...)
...+>
}
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/intel_pm.c | 57 ++---
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8375054ba27d..b2d22fdaf3db 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1464,8 +1464,8 @@ static int g4x_compute_intermediate_wm(struct
intel_crtc_state *new_crtc_state)
max(optimal->wm.plane[plane_id],
active->wm.plane[plane_id]);
- WARN_ON(intermediate->wm.plane[plane_id] >
- g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
+ drm_WARN_ON(crtc->base.dev, intermediate->wm.plane[plane_id] >
+ g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
}
intermediate->sr.plane = max(optimal->sr.plane,
@@ -1482,21 +1482,25 @@ static int g4x_compute_intermediate_wm(struct
intel_crtc_state *new_crtc_state)
intermediate->hpll.fbc = max(optimal->hpll.fbc,
active->hpll.fbc);
- WARN_ON((intermediate->sr.plane >
-g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
-intermediate->sr.cursor >
-g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
- intermediate->cxsr);
- WARN_ON((intermediate->sr.plane >
-g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
-intermediate->sr.cursor >
-g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
- intermediate->hpll_en);
-
- WARN_ON(intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
- intermediate->fbc_en && intermediate->cxsr);
- WARN_ON(intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
- intermediate->fbc_en && intermediate->hpll_en);
+ drm_WARN_ON(crtc->base.dev,
+ (intermediate->sr.plane >
+g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
+intermediate->sr.cursor >
+g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
+ intermediate->cxsr);
+ drm_WARN_ON(crtc->base.dev,
+ (intermediate->sr.plane >
+g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
+intermediate->sr.cursor >
+g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
+ intermediate->hpll_en);
+
+ drm_WARN_ON(crtc->base.dev,
+ intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
+ intermediate->fbc_en && intermediate->cxsr);
+ drm_WARN_ON(crtc->base.dev,
+ intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
+ intermediate->fbc_en && intermediate->hpll_en);
out:
/*
@@ -1748,11 +1752,11 @@ static int vlv_compute_fifo(struct intel_crtc_state
*crtc_state)
fifo_left -= plane_extra;
}
- WARN_ON(active_planes != 0 && fifo_left != 0);
+ drm_WARN_ON(crtc->base.dev, active_planes != 0 && fifo_left != 0);
/* give it all to the first plane if none are active */
if (active_planes == 0) {
- WARN_ON(fifo_left != fifo_size);
+ drm_WARN_ON(crtc->base.dev, fifo_left != fifo_size);
fifo_state->plane[PLANE_PRIMARY] = fifo_left;
}
@@ -4154,7 +4158,8 @@ skl_plane_downscale_amount(const struct intel_crtc_state
*crtc_state,
uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
uint_fixed_16_16_t downscale_h, downscale_w;
- if (WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)))
+ if (drm_WARN_ON(crtc_state->uapi.crtc->dev,
+ !intel_wm_plane_visible(crtc_state, plane_state)))
return u32_to_fixed16(0);
/*
@@ -4815,7 +4820,7 @@ intel_get_linetime_us(const struct intel_crtc_state
*crtc_state)
pixel_rate = crtc_state->pixel_rate;
- if (WARN_ON(pixel_rate == 0))
+ if (drm_WARN_ON(crtc_state->uapi.crtc->dev, pixel_rate == 0))
return u32_to_fixed16(0);
crtc_htotal = crtc_state->hw.adjusted_mode.crtc_htotal;
@@ -4832,7 +4837,8 @@ skl_adjusted_plane_pixel_rate(const struct
intel_crtc_state *crtc_state,
uint_fixed_16_16_t downscale_amount;
/* Shouldn't reach here on disable
[PATCH 13/18] drm/i915/display/vlv_dsi: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c
b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 4e18d4627065..46e2895d916d 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -864,7 +864,7 @@ static void bxt_dsi_enable(struct intel_atomic_state *state,
const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state)
{
- WARN_ON(crtc_state->has_pch_encoder);
+ drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
intel_crtc_vblank_on(crtc_state);
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 16/18] drm/i915/pmu: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/i915_pmu.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 2c062534eac1..1d7fffdd938a 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -458,7 +458,10 @@ static void engine_event_destroy(struct perf_event *event)
static void i915_pmu_event_destroy(struct perf_event *event)
{
- WARN_ON(event->parent);
+ struct drm_i915_private *i915 =
+ container_of(event->pmu, typeof(*i915), pmu.base);
+
+ drm_WARN_ON(&i915->drm, event->parent);
if (is_engine_event(event))
engine_event_destroy(event);
@@ -1085,8 +1088,10 @@ static int i915_pmu_register_cpuhp_state(struct i915_pmu
*pmu)
static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
{
- WARN_ON(pmu->cpuhp.slot == CPUHP_INVALID);
- WARN_ON(cpuhp_state_remove_instance(pmu->cpuhp.slot, &pmu->cpuhp.node));
+ struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
+
+ drm_WARN_ON(&i915->drm, pmu->cpuhp.slot == CPUHP_INVALID);
+ drm_WARN_ON(&i915->drm, cpuhp_state_remove_instance(pmu->cpuhp.slot,
&pmu->cpuhp.node));
cpuhp_remove_multi_state(pmu->cpuhp.slot);
pmu->cpuhp.slot = CPUHP_INVALID;
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 18/18] drm/i915/runtime_pm: Prefer drm_WARN* over WARN*
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN*.
Conversion is done with below semantic patch:
@@
identifier func, T;
@@
func(struct intel_runtime_pm *T,...) {
+ struct drm_i915_private *i915 = container_of(T, struct drm_i915_private,
runtime_pm);
<+...
(
-WARN(
+drm_WARN(&i915->drm,
...)
|
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(&i915->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&i915->drm,
...)
)
...+>
}
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 39 ++---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ad719c9602af..31ccd0559c55 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -116,6 +116,9 @@ track_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm)
static void untrack_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm,
depot_stack_handle_t stack)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
unsigned long flags, n;
bool found = false;
@@ -134,9 +137,9 @@ static void untrack_intel_runtime_pm_wakeref(struct
intel_runtime_pm *rpm,
}
spin_unlock_irqrestore(&rpm->debug.lock, flags);
- if (WARN(!found,
-"Unmatched wakeref (tracking %lu), count %u\n",
-rpm->debug.count, atomic_read(&rpm->wakeref_count))) {
+ if (drm_WARN(&i915->drm, !found,
+"Unmatched wakeref (tracking %lu), count %u\n",
+rpm->debug.count, atomic_read(&rpm->wakeref_count))) {
char *buf;
buf = kmalloc(PAGE_SIZE, GFP_NOWAIT | __GFP_NOWARN);
@@ -355,10 +358,14 @@ intel_runtime_pm_release(struct intel_runtime_pm *rpm,
int wakelock)
static intel_wakeref_t __intel_runtime_pm_get(struct intel_runtime_pm *rpm,
bool wakelock)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
int ret;
ret = pm_runtime_get_sync(rpm->kdev);
- WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+ drm_WARN_ONCE(&i915->drm, ret < 0,
+ "pm_runtime_get_sync() failed: %d\n", ret);
intel_runtime_pm_acquire(rpm, wakelock);
@@ -539,6 +546,9 @@ void intel_runtime_pm_put(struct intel_runtime_pm *rpm,
intel_wakeref_t wref)
*/
void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
struct device *kdev = rpm->kdev;
/*
@@ -565,7 +575,8 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
pm_runtime_dont_use_autosuspend(kdev);
ret = pm_runtime_get_sync(kdev);
- WARN(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+ drm_WARN(&i915->drm, ret < 0,
+"pm_runtime_get_sync() failed: %d\n", ret);
} else {
pm_runtime_use_autosuspend(kdev);
}
@@ -580,11 +591,14 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
void intel_runtime_pm_disable(struct intel_runtime_pm *rpm)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
struct device *kdev = rpm->kdev;
/* Transfer rpm ownership back to core */
- WARN(pm_runtime_get_sync(kdev) < 0,
-"Failed to pass rpm ownership back to core\n");
+ drm_WARN(&i915->drm, pm_runtime_get_sync(kdev) < 0,
+"Failed to pass rpm ownership back to core\n");
pm_runtime_dont_use_autosuspend(kdev);
@@ -594,12 +608,15 @@ void intel_runtime_pm_disable(struct intel_runtime_pm
*rpm)
void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
int count = atomic_read(&rpm->wakeref_count);
- WARN(count,
-"i915 raw-wakerefs=%d wakelocks=%d on cleanup\n",
-intel_rpm_raw_wakeref_count(count),
-intel_rpm_wakelock_count(count));
+ drm_
[PATCH 15/18] drm/i915/i915_drv: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e9ee4daa9320..be33cab6403d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1647,7 +1647,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_DISPLAY(dev_priv) (INTEL_INFO(dev_priv)->pipe_mask != 0)
/* Only valid when HAS_DISPLAY() is true */
-#define INTEL_DISPLAY_ENABLED(dev_priv) (WARN_ON(!HAS_DISPLAY(dev_priv)),
!i915_modparams.disable_display)
+#define INTEL_DISPLAY_ENABLED(dev_priv) \
+ (drm_WARN_ON(&dev_priv->drm, !HAS_DISPLAY(dev_priv)),
!i915_modparams.disable_display)
static inline bool intel_vtd_active(void)
{
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/18] drm/i915/display/display: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON at places where struct drm_device
pointer can be extracted.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/intel_display.c | 24
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 70ec301fe6e3..71ff6b2dc9df 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1973,16 +1973,16 @@ static bool is_aux_plane(const struct drm_framebuffer
*fb, int plane)
static int main_to_ccs_plane(const struct drm_framebuffer *fb, int main_plane)
{
- WARN_ON(!is_ccs_modifier(fb->modifier) ||
- (main_plane && main_plane >= fb->format->num_planes / 2));
+ drm_WARN_ON(fb->dev, !is_ccs_modifier(fb->modifier) ||
+ (main_plane && main_plane >= fb->format->num_planes / 2));
return fb->format->num_planes / 2 + main_plane;
}
static int ccs_to_main_plane(const struct drm_framebuffer *fb, int ccs_plane)
{
- WARN_ON(!is_ccs_modifier(fb->modifier) ||
- ccs_plane < fb->format->num_planes / 2);
+ drm_WARN_ON(fb->dev, !is_ccs_modifier(fb->modifier) ||
+ ccs_plane < fb->format->num_planes / 2);
return ccs_plane - fb->format->num_planes / 2;
}
@@ -2992,7 +2992,7 @@ setup_fb_rotation(int plane, const struct
intel_remapped_plane_info *plane_info,
fb->modifier != I915_FORMAT_MOD_Yf_TILED)
return 0;
- if (WARN_ON(plane >= ARRAY_SIZE(rot_info->plane)))
+ if (drm_WARN_ON(fb->dev, plane >= ARRAY_SIZE(rot_info->plane)))
return 0;
rot_info->plane[plane] = *plane_info;
@@ -6621,7 +6621,7 @@ intel_connector_primary_encoder(struct intel_connector
*connector)
return &dp_to_dig_port(connector->mst_port)->base;
encoder = intel_attached_encoder(connector);
- WARN_ON(!encoder);
+ drm_WARN_ON(connector->base.dev, !encoder);
return encoder;
}
@@ -7946,7 +7946,8 @@ static u32 ilk_pipe_pixel_rate(const struct
intel_crtc_state *pipe_config)
if (pipe_h < pfit_h)
pipe_h = pfit_h;
- if (WARN_ON(!pfit_w || !pfit_h))
+ if (drm_WARN_ON(pipe_config->uapi.crtc->dev,
+ !pfit_w || !pfit_h))
return pixel_rate;
pixel_rate = div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
@@ -12426,8 +12427,10 @@ static int icl_add_linked_planes(struct
intel_atomic_state *state)
if (IS_ERR(linked_plane_state))
return PTR_ERR(linked_plane_state);
- WARN_ON(linked_plane_state->planar_linked_plane != plane);
- WARN_ON(linked_plane_state->planar_slave ==
plane_state->planar_slave);
+ drm_WARN_ON(state->base.dev,
+ linked_plane_state->planar_linked_plane != plane);
+ drm_WARN_ON(state->base.dev,
+ linked_plane_state->planar_slave ==
plane_state->planar_slave);
}
return 0;
@@ -13150,7 +13153,8 @@ static void intel_crtc_copy_hw_to_uapi_state(struct
intel_crtc_state *crtc_state
{
crtc_state->uapi.enable = crtc_state->hw.enable;
crtc_state->uapi.active = crtc_state->hw.active;
- WARN_ON(drm_atomic_set_mode_for_crtc(&crtc_state->uapi,
&crtc_state->hw.mode) < 0);
+ drm_WARN_ON(crtc_state->uapi.crtc->dev,
+ drm_atomic_set_mode_for_crtc(&crtc_state->uapi,
&crtc_state->hw.mode) < 0);
crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/18] drm/i915/display/overlay: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information in the backtrace, so we know what device the warnings originate from. Prefer drm_WARN_ON over WARN_ON. Signed-off-by: Pankaj Bharadiya --- drivers/gpu/drm/i915/display/intel_overlay.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c index 6e1d66323223..66711e62fa71 100644 --- a/drivers/gpu/drm/i915/display/intel_overlay.c +++ b/drivers/gpu/drm/i915/display/intel_overlay.c @@ -281,7 +281,7 @@ static void intel_overlay_flip_prepare(struct intel_overlay *overlay, enum pipe pipe = overlay->crtc->pipe; struct intel_frontbuffer *from = NULL, *to = NULL; - WARN_ON(overlay->old_vma); + drm_WARN_ON(&overlay->i915->drm, overlay->old_vma); if (overlay->vma) from = intel_frontbuffer_get(overlay->vma->obj); @@ -350,7 +350,7 @@ static void intel_overlay_release_old_vma(struct intel_overlay *overlay) struct i915_vma *vma; vma = fetch_and_zero(&overlay->old_vma); - if (WARN_ON(!vma)) + if (drm_WARN_ON(&overlay->i915->drm, !vma)) return; intel_frontbuffer_flip_complete(overlay->i915, @@ -396,7 +396,7 @@ static int intel_overlay_off(struct intel_overlay *overlay) struct i915_request *rq; u32 *cs, flip_addr = overlay->flip_addr; - WARN_ON(!overlay->active); + drm_WARN_ON(&overlay->i915->drm, !overlay->active); /* According to intel docs the overlay hw may hang (when switching * off) without loading the filter coeffs. It is however unclear whether -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/18] drm/i915/display/dpll_mgr: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON at places where struct drm_device
pointer can be extracted.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 2d47f1f756a2..b45185b80bec 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -80,7 +80,7 @@ intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s)
{
struct intel_atomic_state *state = to_intel_atomic_state(s);
- WARN_ON(!drm_modeset_is_locked(&s->dev->mode_config.connection_mutex));
+ drm_WARN_ON(s->dev,
!drm_modeset_is_locked(&s->dev->mode_config.connection_mutex));
if (!state->dpll_set) {
state->dpll_set = true;
@@ -979,7 +979,7 @@ hsw_ddi_spll_get_dpll(struct intel_atomic_state *state,
struct intel_crtc_state *crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
- if (WARN_ON(crtc_state->port_clock / 2 != 135000))
+ if (drm_WARN_ON(crtc->base.dev, crtc_state->port_clock / 2 != 135000))
return NULL;
crtc_state->dpll_hw_state.spll = SPLL_PLL_ENABLE | SPLL_FREQ_1350MHz |
@@ -1616,7 +1616,7 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private
*i915,
dco_freq += ((pll_state->cfgcr1 & DPLL_CFGCR1_DCO_FRACTION_MASK) >> 9) *
ref_clock / 0x8000;
- if (WARN_ON(p0 == 0 || p1 == 0 || p2 == 0))
+ if (drm_WARN_ON(&i915->drm, p0 == 0 || p1 == 0 || p2 == 0))
return 0;
return dco_freq / (p0 * p1 * p2 * 5);
@@ -2074,7 +2074,7 @@ bxt_ddi_hdmi_pll_dividers(struct intel_crtc_state
*crtc_state,
clk_div->p1 = best_clock.p1;
clk_div->p2 = best_clock.p2;
- WARN_ON(best_clock.m1 != 2);
+ drm_WARN_ON(&i915->drm, best_clock.m1 != 2);
clk_div->n = best_clock.n;
clk_div->m2_int = best_clock.m2 >> 22;
clk_div->m2_frac = best_clock.m2 & ((1 << 22) - 1);
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/18] drm/i915/display/dp: Prefer drm_WARN* over WARN*
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN* at places where struct intel_dp struct
pointer is available.
Conversion is done with below sementic patch:
@@
identifier func, T;
@@
func(struct intel_dp *T,...) {
+ struct drm_i915_private *i915 = dp_to_i915(T);
<+...
(
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&i915->drm,
...)
)
...+>
}
@@
identifier func, T;
@@
func(...) {
...
struct intel_dp *T = ...;
+ struct drm_i915_private *i915 = dp_to_i915(T);
<+...
(
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&i915->drm,
...)
)
...+>
}
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/intel_dp.c | 26 -
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index db6ae8e9af6e..5076a7cfe0e5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -410,7 +410,10 @@ static int intel_dp_rate_index(const int *rates, int len,
int rate)
static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
{
- WARN_ON(!intel_dp->num_source_rates || !intel_dp->num_sink_rates);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+ drm_WARN_ON(&i915->drm,
+ !intel_dp->num_source_rates || !intel_dp->num_sink_rates);
intel_dp->num_common_rates = intersect_rates(intel_dp->source_rates,
intel_dp->num_source_rates,
@@ -419,7 +422,7 @@ static void intel_dp_set_common_rates(struct intel_dp
*intel_dp)
intel_dp->common_rates);
/* Paranoia, there should always be something in common. */
- if (WARN_ON(intel_dp->num_common_rates == 0)) {
+ if (drm_WARN_ON(&i915->drm, intel_dp->num_common_rates == 0)) {
intel_dp->common_rates[0] = 162000;
intel_dp->num_common_rates = 1;
}
@@ -1549,6 +1552,7 @@ static ssize_t
intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
{
struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
u8 txbuf[20], rxbuf[20];
size_t txsize, rxsize;
int ret;
@@ -1562,10 +1566,10 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
txsize = msg->size ? HEADER_SIZE + msg->size :
BARE_ADDRESS_SIZE;
rxsize = 2; /* 0 or 1 data bytes */
- if (WARN_ON(txsize > 20))
+ if (drm_WARN_ON(&i915->drm, txsize > 20))
return -E2BIG;
- WARN_ON(!msg->buffer != !msg->size);
+ drm_WARN_ON(&i915->drm, !msg->buffer != !msg->size);
if (msg->buffer)
memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
@@ -1590,7 +1594,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
rxsize = msg->size + 1;
- if (WARN_ON(rxsize > 20))
+ if (drm_WARN_ON(&i915->drm, rxsize > 20))
return -E2BIG;
ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
@@ -1864,10 +1868,11 @@ static void intel_dp_print_rates(struct intel_dp
*intel_dp)
int
intel_dp_max_link_rate(struct intel_dp *intel_dp)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int len;
len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
- if (WARN_ON(len <= 0))
+ if (drm_WARN_ON(&i915->drm, len <= 0))
return 162000;
return intel_dp->common_rates[len - 1];
@@ -1875,10 +1880,11 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int i = intel_dp_rate_index(intel_dp->sink_rates,
intel_dp->num_sink_rates, rate);
- if (WARN_ON(i < 0))
+ if (drm_WARN_ON(&i915->drm, i < 0))
i = 0;
return i;
@@ -5404,6 +5410,7 @@ static void intel_dp_handle_test_request(struct intel_dp
*intel_dp)
static int
intel_dp_check_mst_status(struct intel_dp *intel_dp)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
bool bret;
if (intel_dp->is_mst) {
@@ -5412,7 +5419,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
int retry;
bool handled;
- WARN_ON_ONCE(intel_dp->active_mst_links < 0);
+ drm_WARN_ON_ONCE(&i915->drm, intel_dp->active_mst_links < 0);
bre
[PATCH 03/18] drm/i915/display/ddi: Prefer drm_WARN* over WARN*
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN* calls.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/intel_ddi.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 7184f3b8f091..353a32f5bf46 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2654,8 +2654,9 @@ static void tgl_ddi_vswing_sequence(struct intel_encoder
*encoder,
tgl_dkl_phy_ddi_vswing_sequence(encoder, link_clock, level);
}
-static u32 translate_signal_level(int signal_levels)
+static u32 translate_signal_level(struct intel_dp *intel_dp, int signal_levels)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int i;
for (i = 0; i < ARRAY_SIZE(index_to_dp_signal_levels); i++) {
@@ -2663,8 +2664,9 @@ static u32 translate_signal_level(int signal_levels)
return i;
}
- WARN(1, "Unsupported voltage swing/pre-emphasis level: 0x%x\n",
-signal_levels);
+ drm_WARN(&i915->drm, 1,
+"Unsupported voltage swing/pre-emphasis level: 0x%x\n",
+signal_levels);
return 0;
}
@@ -2675,7 +2677,7 @@ static u32 intel_ddi_dp_level(struct intel_dp *intel_dp)
int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
DP_TRAIN_PRE_EMPHASIS_MASK);
- return translate_signal_level(signal_levels);
+ return translate_signal_level(intel_dp, signal_levels);
}
u32 bxt_signal_levels(struct intel_dp *intel_dp)
@@ -3766,7 +3768,7 @@ static void intel_enable_ddi(struct intel_atomic_state
*state,
const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state)
{
- WARN_ON(crtc_state->has_pch_encoder);
+ drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
intel_enable_pipe(crtc_state);
@@ -3877,7 +3879,7 @@ intel_ddi_update_prepare(struct intel_atomic_state *state,
crtc ? intel_atomic_get_new_crtc_state(state, crtc) : NULL;
int required_lanes = crtc_state ? crtc_state->lane_count : 1;
- WARN_ON(crtc && crtc->active);
+ drm_WARN_ON(state->base.dev, crtc && crtc->active);
intel_tc_port_get_link(enc_to_dig_port(encoder),
required_lanes);
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/18] drm/i915/display/frontbuffer: Prefer drm_WARN_ON over WARN_ON
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/intel_frontbuffer.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 6cb02c912acc..2979ed2588eb 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -302,12 +302,14 @@ void intel_frontbuffer_track(struct intel_frontbuffer
*old,
BITS_PER_TYPE(atomic_t));
if (old) {
- WARN_ON(!(atomic_read(&old->bits) & frontbuffer_bits));
+ drm_WARN_ON(old->obj->base.dev,
+ !(atomic_read(&old->bits) & frontbuffer_bits));
atomic_andnot(frontbuffer_bits, &old->bits);
}
if (new) {
- WARN_ON(atomic_read(&new->bits) & frontbuffer_bits);
+ drm_WARN_ON(new->obj->base.dev,
+ atomic_read(&new->bits) & frontbuffer_bits);
atomic_or(frontbuffer_bits, &new->bits);
}
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
