[PATCH v3 08/14] drm/i915/display: Add handling for new "active color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color format" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 +++-
 drivers/gpu/drm/i915/display/intel_dp.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c|  1 +
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index ee3669bd4662..2cf09599ddc3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10609,6 +10609,21 @@ static void 
intel_atomic_prepare_plane_clear_colors(struct intel_atomic_state *s
}
 }
 
+static int convert_intel_output_format_into_drm_color_format(enum 
intel_output_format output_format)
+{
+   switch (output_format) {
+   case INTEL_OUTPUT_FORMAT_RGB:
+   return DRM_COLOR_FORMAT_RGB444;
+   case INTEL_OUTPUT_FORMAT_YCBCR420:
+   return DRM_COLOR_FORMAT_YCRCB420;
+   case INTEL_OUTPUT_FORMAT_YCBCR444:
+   return DRM_COLOR_FORMAT_YCRCB444;
+   default:
+   break;
+   }
+   return 0;
+}
+
 static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 {
struct drm_device *dev = state->base.dev;
@@ -10916,9 +10931,13 @@ static int intel_atomic_commit(struct drm_device *dev,
if (crtc) {
struct intel_crtc_state *new_crtc_state = 
intel_atomic_get_new_crtc_state(state, crtc);
drm_connector_set_active_bpc_property(connector, 
new_crtc_state->pipe_bpp / 3);
+   
drm_connector_set_active_color_format_property(connector,
+   
convert_intel_output_format_into_drm_color_format(new_crtc_state->output_format));
}
-   else
+   else {
drm_connector_set_active_bpc_property(connector, 0);
+   
drm_connector_set_active_color_format_property(connector, 0);
+   }
}
 
drm_atomic_state_get(>base);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 404a27e56ceb..01fdb9141592 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4691,10 +4691,12 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
drm_connector_attach_active_bpc_property(connector, 6, 10);
+   drm_connector_attach_active_color_format_property(connector);
}
else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
drm_connector_attach_active_bpc_property(connector, 6, 12);
+   drm_connector_attach_active_color_format_property(connector);
}
 
/* Register HDMI colorspace for case of lspcon */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 16bfc59570a5..3e4237df3360 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -856,6 +856,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(connector, 6, 12);
 
+   connector->active_color_format_property =
+   intel_dp->attached_connector->base.active_color_format_property;
+   if (connector->active_color_format_property)
+   drm_connector_attach_active_color_format_property(connector);
+
return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9160e21ac9d6..367aba57b55f 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2516,6 +2516,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
drm_connector_attach_active_bpc_property(connector, 8, 12);
+   drm_connector_attach_active_color_format_property(connector);
}
 }
 
-- 
2.25.1



[PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color range" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +++
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 +++
 2 files changed, 36 insertions(+)

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 b66336a1403e..705cd2e015af 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6732,6 +6732,33 @@ static int 
convert_dc_pixel_encoding_into_drm_color_format(enum dc_pixel_encodin
return 0;
 }
 
+static int convert_dc_color_space_into_drm_mode_color_range(enum 
dc_color_space color_space)
+{
+   if (color_space == COLOR_SPACE_SRGB ||
+   color_space == COLOR_SPACE_XR_RGB ||
+   color_space == COLOR_SPACE_MSREF_SCRGB ||
+   color_space == COLOR_SPACE_2020_RGB_FULLRANGE ||
+   color_space == COLOR_SPACE_ADOBERGB ||
+   color_space == COLOR_SPACE_DCIP3 ||
+   color_space == COLOR_SPACE_DOLBYVISION ||
+   color_space == COLOR_SPACE_YCBCR601 ||
+   color_space == COLOR_SPACE_XV_YCC_601 ||
+   color_space == COLOR_SPACE_YCBCR709 ||
+   color_space == COLOR_SPACE_XV_YCC_709 ||
+   color_space == COLOR_SPACE_2020_YCBCR ||
+   color_space == COLOR_SPACE_YCBCR709_BLACK ||
+   color_space == COLOR_SPACE_DISPLAYNATIVE ||
+   color_space == COLOR_SPACE_APPCTRL ||
+   color_space == COLOR_SPACE_CUSTOMPOINTS)
+   return DRM_MODE_COLOR_RANGE_FULL;
+   if (color_space == COLOR_SPACE_SRGB_LIMITED ||
+   color_space == COLOR_SPACE_2020_RGB_LIMITEDRANGE ||
+   color_space == COLOR_SPACE_YCBCR601_LIMITED ||
+   color_space == COLOR_SPACE_YCBCR709_LIMITED)
+   return DRM_MODE_COLOR_RANGE_LIMITED_16_235;
+   return DRM_MODE_COLOR_RANGE_UNSET;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
  struct drm_crtc_state *crtc_state,
  struct drm_connector_state 
*conn_state)
@@ -7734,6 +7761,7 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
drm_connector_attach_max_bpc_property(>base, 8, 16);
drm_connector_attach_active_bpc_property(>base, 8, 
16);

drm_connector_attach_active_color_format_property(>base);
+   
drm_connector_attach_active_color_range_property(>base);
}
 
/* This defaults to the max in the range, but we want 8bpc for non-edp. 
*/
@@ -9116,11 +9144,15 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)

drm_connector_set_active_color_format_property(connector,

convert_dc_pixel_encoding_into_drm_color_format(

dm_new_crtc_state->stream->timing.pixel_encoding));
+   
drm_connector_set_active_color_range_property(connector,
+   
convert_dc_color_space_into_drm_mode_color_range(
+   
dm_new_crtc_state->stream->output_color_space));
}
}
else {
drm_connector_set_active_bpc_property(connector, 0);

drm_connector_set_active_color_format_property(connector, 0);
+   
drm_connector_set_active_color_range_property(connector, 
DRM_MODE_COLOR_RANGE_UNSET);
}
}
 
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 13151d13aa73..b5d57bbbdd20 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
@@ -417,6 +417,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->active_color_format_property)

drm_connector_attach_active_color_format_property(>base);
 
+   connector->active_color_range_property = 
master->base.active_color_range_property;
+   if (connector->active_color_range_property)
+   
drm_connector_attach_active_color_range_property(>base);
+
connector->vrr_capable_property = master->base.vrr_capable_property;
if (connector->vrr_capable_property)
drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1



[PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "preferred color format" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ++-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 705cd2e015af..ead246b2ec57 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5351,15 +5351,26 @@ static void 
fill_stream_properties_from_drm_display_mode(
timing_out->h_border_right = 0;
timing_out->v_border_top = 0;
timing_out->v_border_bottom = 0;
-   /* TODO: un-hardcode */
-   if (drm_mode_is_420_only(info, mode_in)
-   || (drm_mode_is_420_also(info, mode_in) && 
aconnector->force_yuv420_output))
+
+   if (connector_state && (connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB420
+   || aconnector->force_yuv420_output) && 
drm_mode_is_420(info, mode_in))
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-   else if ((connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
-   && stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+   else if (connector_state && connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB444
+   && connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
-   else
+   else if (connector_state && connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_RGB444
+   && !drm_mode_is_420_only(info, mode_in))
timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
+   else /* connector_state->preferred_color_format not possible
+   || connector_state->preferred_color_format == 0 (auto)
+   || connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB422 */
+   if (drm_mode_is_420_only(info, mode_in))
+   timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
+   else if ((connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
+   && stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+   timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
+   else
+   timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
 
timing_out->timing_3d_format = TIMING_3D_FORMAT_NONE;
timing_out->display_color_depth = convert_color_depth_from_display_info(
@@ -7760,6 +7771,7 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
if (!aconnector->mst_port) {
drm_connector_attach_max_bpc_property(>base, 8, 16);
drm_connector_attach_active_bpc_property(>base, 8, 
16);
+   
drm_connector_attach_preferred_color_format_property(>base);

drm_connector_attach_active_color_format_property(>base);

drm_connector_attach_active_color_range_property(>base);
}
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 b5d57bbbdd20..2563788ba95a 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
@@ -413,6 +413,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(>base, 8, 
16);
 
+   connector->preferred_color_format_property = 
master->base.preferred_color_format_property;
+   if (connector->preferred_color_format_property)
+   
drm_connector_attach_preferred_color_format_property(>base);
+
connector->active_color_format_property = 
master->base.active_color_format_property;
if (connector->active_color_format_property)

drm_connector_attach_active_color_format_property(>base);
-- 
2.25.1



[PATCH v3 11/14] drm/i915/display: Add handling for new "active color range" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color range" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_display.c | 4 
 drivers/gpu/drm/i915/display/intel_dp.c  | 2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c| 1 +
 4 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 2cf09599ddc3..09d7c3da105a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10933,10 +10933,14 @@ static int intel_atomic_commit(struct drm_device *dev,
drm_connector_set_active_bpc_property(connector, 
new_crtc_state->pipe_bpp / 3);

drm_connector_set_active_color_format_property(connector,

convert_intel_output_format_into_drm_color_format(new_crtc_state->output_format));
+   drm_connector_set_active_color_range_property(connector,
+   new_crtc_state->limited_color_range? 
DRM_MODE_COLOR_RANGE_LIMITED_16_235
+  : 
DRM_MODE_COLOR_RANGE_FULL);
}
else {
drm_connector_set_active_bpc_property(connector, 0);

drm_connector_set_active_color_format_property(connector, 0);
+   
drm_connector_set_active_color_range_property(connector, 
DRM_MODE_COLOR_RANGE_UNSET);
}
}
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 01fdb9141592..d648582d0786 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4692,11 +4692,13 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
drm_connector_attach_max_bpc_property(connector, 6, 10);
drm_connector_attach_active_bpc_property(connector, 6, 10);
drm_connector_attach_active_color_format_property(connector);
+   drm_connector_attach_active_color_range_property(connector);
}
else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
drm_connector_attach_active_bpc_property(connector, 6, 12);
drm_connector_attach_active_color_format_property(connector);
+   drm_connector_attach_active_color_range_property(connector);
}
 
/* Register HDMI colorspace for case of lspcon */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 3e4237df3360..cb876175258f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -861,6 +861,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->active_color_format_property)
drm_connector_attach_active_color_format_property(connector);
 
+   connector->active_color_range_property =
+   intel_dp->attached_connector->base.active_color_range_property;
+   if (connector->active_color_range_property)
+   drm_connector_attach_active_color_range_property(connector);
+
return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 367aba57b55f..dacac23a6c30 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2517,6 +2517,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
drm_connector_attach_max_bpc_property(connector, 8, 12);
drm_connector_attach_active_bpc_property(connector, 8, 12);
drm_connector_attach_active_color_format_property(connector);
+   drm_connector_attach_active_color_range_property(connector);
}
 }
 
-- 
2.25.1



[PATCH v3 14/14] drm/i915/display: Add handling for new "preferred color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "preferred color format" drm property for the Intel 
GPU
driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 7 ++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c   | 5 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index d648582d0786..5c83ae624ad1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -616,9 +616,12 @@ intel_dp_output_format(struct drm_connector *connector,
 {
struct intel_dp *intel_dp = 
intel_attached_dp(to_intel_connector(connector));
const struct drm_display_info *info = >display_info;
+   const struct drm_connector_state *connector_state = connector->state;
 
if (!connector->ycbcr_420_allowed ||
-   !drm_mode_is_420_only(info, mode))
+   !(drm_mode_is_420_only(info, mode) ||
+   (drm_mode_is_420_also(info, mode) && connector_state &&
+   connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB420)))
return INTEL_OUTPUT_FORMAT_RGB;
 
if (intel_dp->dfp.rgb_to_ycbcr &&
@@ -4691,12 +4694,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
drm_connector_attach_active_bpc_property(connector, 6, 10);
+   drm_connector_attach_preferred_color_format_property(connector);
drm_connector_attach_active_color_format_property(connector);
drm_connector_attach_active_color_range_property(connector);
}
else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
drm_connector_attach_active_bpc_property(connector, 6, 12);
+   drm_connector_attach_preferred_color_format_property(connector);
drm_connector_attach_active_color_format_property(connector);
drm_connector_attach_active_color_range_property(connector);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index cb876175258f..67f0fb649876 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -856,6 +856,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(connector, 6, 12);
 
+   connector->preferred_color_format_property =
+   
intel_dp->attached_connector->base.preferred_color_format_property;
+   if (connector->preferred_color_format_property)
+   drm_connector_attach_preferred_color_format_property(connector);
+
connector->active_color_format_property =
intel_dp->attached_connector->base.active_color_format_property;
if (connector->active_color_format_property)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index dacac23a6c30..0d917d61482d 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2153,6 +2153,10 @@ static int intel_hdmi_compute_output_format(struct 
intel_encoder *encoder,
crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
}
 
+   if (connector->ycbcr_420_allowed && conn_state->preferred_color_format 
== DRM_COLOR_FORMAT_YCRCB420 &&
+   drm_mode_is_420_also(>display_info, adjusted_mode))
+   crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+
ret = intel_hdmi_compute_clock(encoder, crtc_state);
if (ret) {
if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
@@ -2516,6 +2520,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
drm_connector_attach_active_bpc_property(connector, 8, 12);
+   drm_connector_attach_preferred_color_format_property(connector);
drm_connector_attach_active_color_format_property(connector);
drm_connector_attach_active_color_range_property(connector);
}
-- 
2.25.1



[PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color format" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 28 +--
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 +++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f31bbcb11f03..b66336a1403e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6715,6 +6715,23 @@ static int convert_dc_color_depth_into_bpc (enum 
dc_color_depth display_color_de
return 0;
 }
 
+static int convert_dc_pixel_encoding_into_drm_color_format(enum 
dc_pixel_encoding display_pixel_encoding)
+{
+   switch (display_pixel_encoding) {
+   case PIXEL_ENCODING_RGB:
+   return DRM_COLOR_FORMAT_RGB444;
+   case PIXEL_ENCODING_YCBCR422:
+   return DRM_COLOR_FORMAT_YCRCB422;
+   case PIXEL_ENCODING_YCBCR444:
+   return DRM_COLOR_FORMAT_YCRCB444;
+   case PIXEL_ENCODING_YCBCR420:
+   return DRM_COLOR_FORMAT_YCRCB420;
+   default:
+   break;
+   }
+   return 0;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
  struct drm_crtc_state *crtc_state,
  struct drm_connector_state 
*conn_state)
@@ -7716,6 +7733,7 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
if (!aconnector->mst_port) {
drm_connector_attach_max_bpc_property(>base, 8, 16);
drm_connector_attach_active_bpc_property(>base, 8, 
16);
+   
drm_connector_attach_active_color_format_property(>base);
}
 
/* This defaults to the max in the range, but we want 8bpc for non-edp. 
*/
@@ -9091,13 +9109,19 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
if (crtc) {
new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-   if (dm_new_crtc_state->stream)
+   if (dm_new_crtc_state->stream) {
drm_connector_set_active_bpc_property(connector,
convert_dc_color_depth_into_bpc(

dm_new_crtc_state->stream->timing.display_color_depth));
+   
drm_connector_set_active_color_format_property(connector,
+   
convert_dc_pixel_encoding_into_drm_color_format(
+   
dm_new_crtc_state->stream->timing.pixel_encoding));
+   }
}
-   else
+   else {
drm_connector_set_active_bpc_property(connector, 0);
+   
drm_connector_set_active_color_format_property(connector, 0);
+   }
}
 
/* Count number of newly disabled CRTCs for dropping PM refs later. */
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 0cf38743ec47..13151d13aa73 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
@@ -413,6 +413,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(>base, 8, 
16);
 
+   connector->active_color_format_property = 
master->base.active_color_format_property;
+   if (connector->active_color_format_property)
+   
drm_connector_attach_active_color_format_property(>base);
+
connector->vrr_capable_property = master->base.vrr_capable_property;
if (connector->vrr_capable_property)
drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1



[PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace

2021-06-15 Thread Werner Sembach
Add a new general drm property "preferred color format" which can be used by
userspace to tell the graphic drivers to which color format to use.

Possible options are:
- auto (default/current behaviour)
- rgb
- ycbcr444
- ycbcr422 (not supported by both amdgpu and i915)
- ycbcr420

In theory the auto option should choose the best available option for the
current setup, but because of bad internal conversion some monitors look better
with rgb and some with ycbcr444.

Also, because of bad shielded connectors and/or cables, it might be preferable
to use the less bandwidth heavy ycbcr422 and ycbcr420 formats for a signal that
is less deceptible to interference.

In the future, automatic color calibration for screens might also depend on this
option being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_atomic_helper.c |  4 +++
 drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
 drivers/gpu/drm/drm_connector.c | 46 -
 include/drm/drm_connector.h | 17 +++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index bc3487964fb5..90d62f305257 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -687,6 +687,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (old_connector_state->max_requested_bpc !=
new_connector_state->max_requested_bpc)
new_crtc_state->connectors_changed = true;
+
+   if (old_connector_state->preferred_color_format !=
+   new_connector_state->preferred_color_format)
+   new_crtc_state->connectors_changed = true;
}
 
if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 438e9585b225..c536f5e22016 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -796,6 +796,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
   fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+   } else if (property == connector->preferred_color_format_property) {
+   state->preferred_color_format = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -873,6 +875,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+   } else if (property == connector->preferred_color_format_property) {
+   *val = state->preferred_color_format;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b4aff7d6910f..1dc537514c71 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list 
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native,  "Native"}, /* DP */
 };
 
+static const struct drm_prop_enum_list drm_preferred_color_format_enum_list[] 
= {
+   { 0, "auto" },
+   { DRM_COLOR_FORMAT_RGB444, "rgb" },
+   { DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+   { DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+   { DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
 static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
{ 0, "unknown" },
{ DRM_COLOR_FORMAT_RGB444, "rgb" },
@@ -1219,11 +1227,19 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * Drivers shall use drm_connector_attach_active_bpc_property() to install
  * this property.
  *
+ * preferred color format:
+ * This property is used by userspace to change the used color format. When
+ * used the driver will use the selected format if valid for the hardware,
+ * sink, and current resolution and refresh rate combination. Drivers to
+ * use the function drm_connector_attach_preferred_color_format_property()
+ * to create and attach the property to the connector during
+ * initialization.
+ *
  * active color format:
  * This read-only property tells userspace the color format actually used
  * by the hardware display engine on "the cable" on a connector. The chosen
  * value depends on hardware capabilities, both display engine and
- * connected monitor. Drivers shall use
+ * 

[PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property

2021-06-15 Thread Werner Sembach
Add a new general drm property "active bpc" which can be used by graphic drivers
to report the applied bit depth per pixel back to userspace.

While "max bpc" can be used to change the color depth, there was no way to check
which one actually got used. While in theory the driver chooses the best/highest
color depth within the max bpc setting a user might not be fully aware what his
hardware is or isn't capable off. This is meant as a quick way to double check
the setup.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_connector.c | 50 +
 include/drm/drm_connector.h |  8 ++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da39e7ff6965..02c310c1ac2d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * drm_connector_attach_max_bpc_property() to create and attach the
  * property to the connector during initialization.
  *
+ * active bpc:
+ * This read-only range property tells userspace the pixel color bit depth
+ * actually used by the hardware display engine on "the cable" on a
+ * connector. The chosen value depends on hardware capabilities, both
+ * display engine and connected monitor, and the "max bpc" property.
+ * Drivers shall use drm_connector_attach_active_bpc_property() to install
+ * this property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2152,6 +2160,48 @@ int drm_connector_attach_max_bpc_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
 
+/**
+ * drm_connector_attach_active_bpc_property - attach "active bpc" property
+ * @connector: connector to attach active bpc property on.
+ * @min: The minimum bit depth supported by the connector.
+ * @max: The maximum bit depth supported by the connector.
+ *
+ * This is used to check the applied bit depth on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, 
int min, int max)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_bpc_property) {
+   prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE, 
"active bpc", min, max);
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_bpc_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_bpc_property);
+
+/**
+ * drm_connector_set_active_bpc_property - sets the active bits per color 
property for a connector
+ * @connector: drm connector
+ * @active_bpc: bits per color for the connector currently active on "the 
cable"
+ *
+ * Should be used by atomic drivers to update the active bits per color over a 
connector.
+ */
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, 
int active_bpc)
+{
+   drm_object_property_set_value(>base, 
connector->active_bpc_property, active_bpc);
+}
+EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 714d1a01c065..eee86de62a5f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1380,6 +1380,12 @@ struct drm_connector {
 */
struct drm_property *max_bpc_property;
 
+   /**
+* @active_bpc_property: Default connector property for the active bpc
+* to be driven out of the connector.
+*/
+   struct drm_property *active_bpc_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1702,6 +1708,8 @@ int drm_connector_set_panel_orientation_with_quirk(
int width, int height);
 int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
  int min, int max);
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, 
int min, int max);
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, 
int active_bpc);
 
 /**
  * struct drm_tile_group - Tile group metadata
-- 
2.25.1



[PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace

2021-06-15 Thread Werner Sembach
Add a new general drm property "active color format" which can be used by
graphic drivers to report the used color format back to userspace.

There was no way to check which color format got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of the
monitor, the GPU, and the connection used and what the default behaviour of the
used driver is (e.g. amdgpu prefers YCbCr 4:4:4 while i915 prefers RGB). This
property helps eliminating the guessing on this point.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_connector.c | 56 +
 include/drm/drm_connector.h |  8 +
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 02c310c1ac2d..2411b964c342 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list 
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native,  "Native"}, /* DP */
 };
 
+static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
+   { 0, "unknown" },
+   { DRM_COLOR_FORMAT_RGB444, "rgb" },
+   { DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+   { DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+   { DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 drm_dp_subconnector_enum_list)
 
@@ -1205,6 +1213,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * Drivers shall use drm_connector_attach_active_bpc_property() to install
  * this property.
  *
+ * active color format:
+ * This read-only property tells userspace the color format actually used
+ * by the hardware display engine on "the cable" on a connector. The chosen
+ * value depends on hardware capabilities, both display engine and
+ * connected monitor. Drivers shall use
+ * drm_connector_attach_active_color_format_property() to install this
+ * property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2202,6 +2218,46 @@ void drm_connector_set_active_bpc_property(struct 
drm_connector *connector, int
 }
 EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
 
+/**
+ * drm_connector_attach_active_color_format_property - attach "active color 
format" property
+ * @connector: connector to attach active color format property on.
+ *
+ * This is used to check the applied color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_format_property(struct drm_connector 
*connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_color_format_property) {
+   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, 
"active color format", drm_active_color_format_enum_list, 
ARRAY_SIZE(drm_active_color_format_enum_list));
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_color_format_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_format_property);
+
+/**
+ * drm_connector_set_active_color_format_property - sets the active color 
format property for a connector
+ * @connector: drm connector
+ * @active_color_format: color format for the connector currently active on 
"the cable"
+ *
+ * Should be used by atomic drivers to update the active color format over a 
connector.
+ */
+void drm_connector_set_active_color_format_property(struct drm_connector 
*connector, u32 active_color_format)
+{
+   drm_object_property_set_value(>base, 
connector->active_color_format_property, active_color_format);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index eee86de62a5f..99e4989dd6b3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1386,6 +1386,12 @@ struct drm_connector {
 */
struct drm_property *active_bpc_property;
 
+   /**
+* @active_color_format_property: Default connector property for the
+* active color format to be driven out of the connector.
+*/
+   struct drm_property *active_color_format_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1710,6 +1716,8 @@ int drm_connector_attach_max_bpc_property(struct 
drm_connector *connector,
   

[PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property

2021-06-15 Thread Werner Sembach
This commit implements the "active bpc" drm property for the AMD GPU driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 
 2 files changed, 22 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 cd1df5cf4815..f31bbcb11f03 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7713,8 +7713,10 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
adev->mode_info.underscan_vborder_property,
0);
 
-   if (!aconnector->mst_port)
+   if (!aconnector->mst_port) {
drm_connector_attach_max_bpc_property(>base, 8, 16);
+   drm_connector_attach_active_bpc_property(>base, 8, 
16);
+   }
 
/* This defaults to the max in the range, but we want 8bpc for non-edp. 
*/
aconnector->base.state->max_bpc = (connector_type == 
DRM_MODE_CONNECTOR_eDP) ? 16 : 8;
@@ -9083,6 +9085,21 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
mutex_unlock(>dc_lock);
}
 
+   /* Extract information from crtc to communicate it to userspace as 
connector properties */
+   for_each_new_connector_in_state(state, connector, new_con_state, i) {
+   struct drm_crtc *crtc = new_con_state->crtc;
+   if (crtc) {
+   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
+   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+   if (dm_new_crtc_state->stream)
+   drm_connector_set_active_bpc_property(connector,
+   convert_dc_color_depth_into_bpc(
+   
dm_new_crtc_state->stream->timing.display_color_depth));
+   }
+   else
+   drm_connector_set_active_bpc_property(connector, 0);
+   }
+
/* Count number of newly disabled CRTCs for dropping PM refs later. */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
  new_crtc_state, i) {
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 5568d4e518e6..0cf38743ec47 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
@@ -409,6 +409,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->max_bpc_property)
drm_connector_attach_max_bpc_property(connector, 8, 16);
 
+   connector->active_bpc_property = master->base.active_bpc_property;
+   if (connector->active_bpc_property)
+   drm_connector_attach_active_bpc_property(>base, 8, 
16);
+
connector->vrr_capable_property = master->base.vrr_capable_property;
if (connector->vrr_capable_property)
drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1



[PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace

2021-06-15 Thread Werner Sembach
Add a new general drm property "active color range" which can be used by graphic
drivers to report the used color range back to userspace.

There was no way to check which color range got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of the
monitor and what the default behaviour of the used driver is. This property
helps eliminating the guessing at this point.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_connector.c | 54 +
 include/drm/drm_connector.h | 26 
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2411b964c342..b4aff7d6910f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -897,6 +897,12 @@ static const struct drm_prop_enum_list 
drm_active_color_format_enum_list[] = {
{ DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
 };
 
+static const struct drm_prop_enum_list drm_active_color_range_enum_list[] = {
+   { DRM_MODE_COLOR_RANGE_UNSET, "Unknown" },
+   { DRM_MODE_COLOR_RANGE_FULL, "Full" },
+   { DRM_MODE_COLOR_RANGE_LIMITED_16_235, "Limited 16:235" },
+};
+
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 drm_dp_subconnector_enum_list)
 
@@ -1221,6 +1227,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * drm_connector_attach_active_color_format_property() to install this
  * property.
  *
+ * active color range:
+ * This read-only property tells userspace the color range actually used by
+ * the hardware display engine on "the cable" on a connector. The chosen
+ * value depends on hardware capabilities of the monitor and the used color
+ * format. Drivers shall use
+ * drm_connector_attach_active_color_range_property() to install this
+ * property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2258,6 +2272,46 @@ void 
drm_connector_set_active_color_format_property(struct drm_connector *connec
 }
 EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
 
+/**
+ * drm_connector_attach_active_color_range_property - attach "active color 
range" property
+ * @connector: connector to attach active color range property on.
+ *
+ * This is used to check the applied color range on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_range_property(struct drm_connector 
*connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_color_range_property) {
+   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, 
"active color range", drm_active_color_range_enum_list, 
ARRAY_SIZE(drm_active_color_range_enum_list));
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_color_range_property = prop;
+   drm_object_attach_property(>base, prop, 
DRM_MODE_COLOR_RANGE_UNSET);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_range_property);
+
+/**
+ * drm_connector_set_active_color_range_property - sets the active color range 
property for a connector
+ * @connector: drm connector
+ * @active_color_range: color range for the connector currently active on "the 
cable"
+ *
+ * Should be used by atomic drivers to update the active color range over a 
connector.
+ */
+void drm_connector_set_active_color_range_property(struct drm_connector 
*connector, enum drm_mode_color_range active_color_range)
+{
+   drm_object_property_set_value(>base, 
connector->active_color_range_property, active_color_range);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_range_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 99e4989dd6b3..5cb122812727 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -648,6 +648,24 @@ struct drm_tv_connector_state {
unsigned int hue;
 };
 
+/**
+ * enum drm_mode_color_range - color_range info for _connector
+ *
+ * This enum is used to represent full or limited color range on the display
+ * connector signal.
+ *
+ * @DRM_MODE_COLOR_RANGE_UNSET:Color range is 
unspecified/default.
+ * @DRM_MODE_COLOR_RANGE_FULL: Color range is full range, 0-255 for
+ * 8-Bit color depth.
+ * DRM_MODE_COLOR_RANGE_LIMITED_16_235:Color range is limited range, 
16-235 for
+ * 8-Bit color depth.
+ */
+enum drm_mode_color_range {
+   DRM_MODE_COLOR_RANGE_UNSET,
+   DRM_MODE_COLOR_RANGE_FULL,
+

[PATCH v3 05/14] drm/i915/display: Add handling for new "active bpc" property

2021-06-15 Thread Werner Sembach
This commit implements the "active bpc" drm property for the Intel GPU driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_display.c | 14 ++
 drivers/gpu/drm/i915/display/intel_dp.c  |  8 ++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c|  4 +++-
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 6be1b31af07b..ee3669bd4662 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10839,6 +10839,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 {
struct intel_atomic_state *state = to_intel_atomic_state(_state);
struct drm_i915_private *dev_priv = to_i915(dev);
+   struct drm_connector *connector;
+   struct drm_connector_state *new_conn_state;
+   int i;
int ret = 0;
 
state->wakeref = intel_runtime_pm_get(_priv->runtime_pm);
@@ -10907,6 +10910,17 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_swap_state(state);
intel_atomic_track_fbs(state);
 
+   /* Extract information from crtc to communicate it to userspace as 
connector properties */
+   for_each_new_connector_in_state(>base, connector, 
new_conn_state, i) {
+   struct intel_crtc *crtc = to_intel_crtc(new_conn_state->crtc);
+   if (crtc) {
+   struct intel_crtc_state *new_crtc_state = 
intel_atomic_get_new_crtc_state(state, crtc);
+   drm_connector_set_active_bpc_property(connector, 
new_crtc_state->pipe_bpp / 3);
+   }
+   else
+   drm_connector_set_active_bpc_property(connector, 0);
+   }
+
drm_atomic_state_get(>base);
INIT_WORK(>base.commit_work, intel_atomic_commit_work);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 5c983044..404a27e56ceb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4688,10 +4688,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
intel_attach_force_audio_property(connector);
 
intel_attach_broadcast_rgb_property(connector);
-   if (HAS_GMCH(dev_priv))
+   if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
-   else if (DISPLAY_VER(dev_priv) >= 5)
+   drm_connector_attach_active_bpc_property(connector, 6, 10);
+   }
+   else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
+   drm_connector_attach_active_bpc_property(connector, 6, 12);
+   }
 
/* Register HDMI colorspace for case of lspcon */
if (intel_bios_is_lspcon_present(dev_priv, port)) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index b170e272bdee..16bfc59570a5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -851,6 +851,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->max_bpc_property)
drm_connector_attach_max_bpc_property(connector, 6, 12);
 
+   connector->active_bpc_property =
+   intel_dp->attached_connector->base.active_bpc_property;
+   if (connector->active_bpc_property)
+   drm_connector_attach_active_bpc_property(connector, 6, 12);
+
return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 7e51c98c475e..9160e21ac9d6 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2513,8 +2513,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
if (DISPLAY_VER(dev_priv) >= 10)
drm_connector_attach_hdr_output_metadata_property(connector);
 
-   if (!HAS_GMCH(dev_priv))
+   if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
+   drm_connector_attach_active_bpc_property(connector, 8, 12);
+   }
 }
 
 /*
-- 
2.25.1



[PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc

2021-06-15 Thread Werner Sembach
convert_dc_color_depth_into_bpc() that converts the enum dc_color_depth to an
integer had the casses for COLOR_DEPTH_999 and COLOR_DEPTH_11 missing.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
 1 file changed, 4 insertions(+)

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 44757720b15f..cd1df5cf4815 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6705,6 +6705,10 @@ static int convert_dc_color_depth_into_bpc (enum 
dc_color_depth display_color_de
return 14;
case COLOR_DEPTH_161616:
return 16;
+   case COLOR_DEPTH_999:
+   return 9;
+   case COLOR_DEPTH_11:
+   return 11;
default:
break;
}
-- 
2.25.1



[PATCH v3 00/14] New uAPI drm properties for color management

2021-06-15 Thread Werner Sembach
I started work on my proposal for better color handling in Linux display
drivers: https://lkml.org/lkml/2021/5/12/764

In this 3rd revision everything except the generalised Broadcast RGB
implementation is included. I did however not yet include everything suggested
in the feedback for v1 and v2.

I rebased the patch series on drm-tip to have the latest changes in i915's
YCbCr420 handling and to make the intel-gfx ci not fail on merge anymore.

The read only properties are now correctly marked as immutable.

Some questions I already have:

I think Broadcast RGB is really no good name for the property as, at least in
theory, YCbCr can also be "Limited" or "Full". Should the new implementation
have a different name and make "Broadcast RGB" an alias for it? I propose
"preferred color range" as the new name.

I have not tested dp mst (both on AMD and Intel) as i have no adapter for it at
hand. If one can test it, please let me know if things break or not.

I found the DRM_MODE_PROP_ATOMIC flag and from the documentation it sounds like
"max bpc" (and "preferred color format" and "Broadcast RGB") should have it. Is
there a reason it doesn't?

I have not yet looked into dsc and dithering behaviour.

I have already submitted the first two patches separately to the lkml as they 
fix
potential bugs and don't introduce new uAPI.



[PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check

2021-06-15 Thread Werner Sembach
Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check that was performed in the
drm_mode_is_420_only() case, but not in the drm_mode_is_420_also() &&
force_yuv420_output case.

Without further knowledge if YCbCr 4:2:0 is supported outside of HDMI, there is
no reason to use RGB when the display reports drm_mode_is_420_only() even on a
non HDMI connection.

This patch also moves both checks in the same if-case. This  eliminates an extra
else-if-case.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6fda0dfb78f8..44757720b15f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5353,10 +5353,7 @@ static void fill_stream_properties_from_drm_display_mode(
timing_out->v_border_bottom = 0;
/* TODO: un-hardcode */
if (drm_mode_is_420_only(info, mode_in)
-   && stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-   timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-   else if (drm_mode_is_420_also(info, mode_in)
-   && aconnector->force_yuv420_output)
+   || (drm_mode_is_420_also(info, mode_in) && 
aconnector->force_yuv420_output))
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
else if ((connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-- 
2.25.1



Re: [PATCH 10/10] vfio/mbochs: Convert to use vfio_register_group_dev()

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:19PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe 
> 
> This is straightforward conversion, the mdev_state is actually serving as
> the vfio_device and we can replace all the mdev_get_drvdata()'s and the
> wonky dead code with a simple container_of().
> 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  samples/vfio-mdev/mbochs.c | 163 +
>  1 file changed, 91 insertions(+), 72 deletions(-)

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 09/10] vfio/mdpy: Convert to use vfio_register_group_dev()

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:18PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe 
> 
> This is straightforward conversion, the mdev_state is actually serving as
> the vfio_device and we can replace all the mdev_get_drvdata()'s and the
> wonky dead code with a simple container_of().
> 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  samples/vfio-mdev/mdpy.c | 159 ++-
>  1 file changed, 88 insertions(+), 71 deletions(-)

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 08/10] vfio/mtty: Convert to use vfio_register_group_dev()

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:17PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe 
> 
> This is straightforward conversion, the mdev_state is actually serving as
> the vfio_device and we can replace all the mdev_get_drvdata()'s and the
> wonky dead code with a simple container_of()
> 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  samples/vfio-mdev/mtty.c | 185 ++-
>  1 file changed, 83 insertions(+), 102 deletions(-)


Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe 
> 
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
> 
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>   struct vfio_device_ops
> 
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 

Messy, but ok...

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 06/10] vfio/mdev: Remove CONFIG_VFIO_MDEV_DEVICE

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:15PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe 
> 
> For some reason the vfio_mdev shim mdev_driver has its own module and
> kconfig. As the next patch requires access to it from mdev.ko merge the
> two modules together and remove VFIO_MDEV_DEVICE.
> 
> A later patch deletes this driver entirely.
> 
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Cornelia Huck 
> ---
>  Documentation/s390/vfio-ap.rst   |  1 -
>  arch/s390/Kconfig|  2 +-
>  drivers/gpu/drm/i915/Kconfig |  2 +-
>  drivers/vfio/mdev/Kconfig|  7 ---
>  drivers/vfio/mdev/Makefile   |  3 +--
>  drivers/vfio/mdev/mdev_core.c| 16 ++--
>  drivers/vfio/mdev/mdev_private.h |  2 ++
>  drivers/vfio/mdev/vfio_mdev.c| 24 +---
>  samples/Kconfig  |  6 +++---
>  9 files changed, 23 insertions(+), 40 deletions(-)
> 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 05/10] driver core: Export device_driver_attach()

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:14PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe 
> 
> This is intended as a replacement API for device_bind_driver(). It has at
> least the following benefits:
> 
> - Internal locking. Few of the users of device_bind_driver() follow the
>   locking rules
> 
> - Calls device driver probe() internally. Notably this means that devm
>   support for probe works correctly as probe() error will call
>   devres_release_all()
> 
> - struct device_driver -> dev_groups is supported
> 
> - Simplified calling convention, no need to manually call probe().
> 
> The general usage is for situations that already know what driver to bind
> and need to ensure the bind is synchronized with other logic. Call
> device_driver_attach() after device_add().
> 
> If probe() returns a failure then this will be preserved up through to the
> error return of device_driver_attach().
> 
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Cornelia Huck 
> ---

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:35:11PM +0200, Christoph Hellwig wrote:
> really_probe tries to special case errors from ->probe, but due to all
> other initialization added to the function over time now a lot of
> internal errors hit that code path as well.  Untangle that by adding
> a new probe_err local variable and apply the special casing only to
> that.
> 
> Signed-off-by: Christoph Hellwig 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

2021-06-15 Thread Greg Kroah-Hartman
On Tue, Jun 15, 2021 at 03:53:46PM +0200, Cornelia Huck wrote:
> On Tue, Jun 15 2021, Christoph Hellwig  wrote:
> 
> > really_probe tries to special case errors from ->probe, but due to all
> > other initialization added to the function over time now a lot of
> > internal errors hit that code path as well.  Untangle that by adding
> > a new probe_err local variable and apply the special casing only to
> > that.
> >
> > Signed-off-by: Christoph Hellwig 
> > ---
> >  drivers/base/dd.c | 72 +++
> >  1 file changed, 42 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 7477d3322b3a..fd83817240e6 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(state_synced);
> >  
> > +
> > +static int call_driver_probe(struct device *dev, struct device_driver *drv)
> > +{
> > +   int ret = 0;
> > +
> > +   if (dev->bus->probe)
> > +   ret = dev->bus->probe(dev);
> > +   else if (drv->probe)
> > +   ret = drv->probe(dev);
> > +
> > +   switch (ret) {
> > +   case 0:
> > +   break;
> > +   case -EPROBE_DEFER:
> > +   /* Driver requested deferred probing */
> > +   dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name);
> > +   break;
> > +   case -ENODEV:
> > +   case -ENXIO:
> > +   pr_debug("%s: probe of %s rejects match %d\n",
> > +drv->name, dev_name(dev), ret);
> > +   break;
> > +   default:
> > +   /* driver matched but the probe failed */
> > +   pr_warn("%s: probe of %s failed with error %d\n",
> > +   drv->name, dev_name(dev), ret);
> 
> Convert these two pr_* to dev_* when touching the code anyway?

It can wait, it's just moving code around for now.

thanks,

greg k-h


Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

2021-06-15 Thread Cornelia Huck
On Tue, Jun 15 2021, Christoph Hellwig  wrote:

> From: Jason Gunthorpe 
>
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
>
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>   struct vfio_device_ops
>
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/vfio/mdev/mdev_core.c   | 30 ++
>  drivers/vfio/mdev/mdev_driver.c | 10 ++
>  include/linux/mdev.h|  2 ++
>  3 files changed, 34 insertions(+), 8 deletions(-)
>

Reviewed-by: Cornelia Huck 



Re: [PATCH 04/10] driver core: Don't return EPROBE_DEFER to userspace during sysfs bind

2021-06-15 Thread Cornelia Huck
On Tue, Jun 15 2021, Christoph Hellwig  wrote:

> EPROBE_DEFER is an internal kernel error code and it should not be leaked
> to userspace via the bind_store() sysfs. Userspace doesn't have this
> constant and cannot understand it.
>
> Further, it doesn't really make sense to have userspace trigger a deferred
> probe via bind_store(), which could eventually succeed, while
> simultaneously returning an error back.
>
> Resolve this by splitting driver_probe_device so that the version used
> by the sysfs binding that turns EPROBE_DEFER into -EAGAIN, while the one
> used for internally binding keeps the error code, and calls
> driver_deferred_probe_add where needed.  This also allows to nicely split
> out the defer_all_probes / probe_count checks so that they actually allow
> for full device_{block,unblock}_probing protection while not bothering
> the sysfs bind case.
>
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Greg Kroah-Hartman 
> ---
>  drivers/base/dd.c | 78 +--
>  1 file changed, 42 insertions(+), 36 deletions(-)
>

Reviewed-by: Cornelia Huck 



Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

2021-06-15 Thread Cornelia Huck
On Tue, Jun 15 2021, Christoph Hellwig  wrote:

> really_probe tries to special case errors from ->probe, but due to all
> other initialization added to the function over time now a lot of
> internal errors hit that code path as well.  Untangle that by adding
> a new probe_err local variable and apply the special casing only to
> that.
>
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/base/dd.c | 72 +++
>  1 file changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 7477d3322b3a..fd83817240e6 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(state_synced);
>  
> +
> +static int call_driver_probe(struct device *dev, struct device_driver *drv)
> +{
> + int ret = 0;
> +
> + if (dev->bus->probe)
> + ret = dev->bus->probe(dev);
> + else if (drv->probe)
> + ret = drv->probe(dev);
> +
> + switch (ret) {
> + case 0:
> + break;
> + case -EPROBE_DEFER:
> + /* Driver requested deferred probing */
> + dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name);
> + break;
> + case -ENODEV:
> + case -ENXIO:
> + pr_debug("%s: probe of %s rejects match %d\n",
> +  drv->name, dev_name(dev), ret);
> + break;
> + default:
> + /* driver matched but the probe failed */
> + pr_warn("%s: probe of %s failed with error %d\n",
> + drv->name, dev_name(dev), ret);

Convert these two pr_* to dev_* when touching the code anyway?

> + break;
> + }
> +
> + return ret;
> +}

(...)

Reviewed-by: Cornelia Huck 



Re: [PATCH v9 00/14] Restricted DMA

2021-06-15 Thread Claire Chang
v10 here: https://lore.kernel.org/patchwork/cover/1446882/


[PATCH v10 12/12] of: Add plumbing for restricted DMA pool

2021-06-15 Thread Claire Chang
If a device is not behind an IOMMU, we look up the device node and set
up the restricted DMA when the restricted-dma-pool is presented.

Signed-off-by: Claire Chang 
---
 drivers/of/address.c| 33 +
 drivers/of/device.c |  3 +++
 drivers/of/of_private.h |  6 ++
 3 files changed, 42 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 3b2acca7e363..c8066d95ff0e 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1001,6 +1002,38 @@ int of_dma_get_range(struct device_node *np, const 
struct bus_dma_region **map)
of_node_put(node);
return ret;
 }
+
+int of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
+{
+   struct device_node *node, *of_node = dev->of_node;
+   int count, i;
+
+   count = of_property_count_elems_of_size(of_node, "memory-region",
+   sizeof(u32));
+   /*
+* If dev->of_node doesn't exist or doesn't contain memory-region, try
+* the OF node having DMA configuration.
+*/
+   if (count <= 0) {
+   of_node = np;
+   count = of_property_count_elems_of_size(
+   of_node, "memory-region", sizeof(u32));
+   }
+
+   for (i = 0; i < count; i++) {
+   node = of_parse_phandle(of_node, "memory-region", i);
+   /*
+* There might be multiple memory regions, but only one
+* restricted-dma-pool region is allowed.
+*/
+   if (of_device_is_compatible(node, "restricted-dma-pool") &&
+   of_device_is_available(node))
+   return of_reserved_mem_device_init_by_idx(dev, of_node,
+ i);
+   }
+
+   return 0;
+}
 #endif /* CONFIG_HAS_DMA */
 
 /**
diff --git a/drivers/of/device.c b/drivers/of/device.c
index c5a9473a5fb1..2defdca418ec 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -165,6 +165,9 @@ int of_dma_configure_id(struct device *dev, struct 
device_node *np,
 
arch_setup_dma_ops(dev, dma_start, size, iommu, coherent);
 
+   if (!iommu)
+   return of_dma_set_restricted_buffer(dev, np);
+
return 0;
 }
 EXPORT_SYMBOL_GPL(of_dma_configure_id);
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 631489f7f8c0..376462798f7e 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -163,12 +163,18 @@ struct bus_dma_region;
 #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_HAS_DMA)
 int of_dma_get_range(struct device_node *np,
const struct bus_dma_region **map);
+int of_dma_set_restricted_buffer(struct device *dev, struct device_node *np);
 #else
 static inline int of_dma_get_range(struct device_node *np,
const struct bus_dma_region **map)
 {
return -ENODEV;
 }
+static inline int of_dma_set_restricted_buffer(struct device *dev,
+  struct device_node *np)
+{
+   return -ENODEV;
+}
 #endif
 
 void fdt_init_reserved_mem(void);
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 11/12] dt-bindings: of: Add restricted DMA pool

2021-06-15 Thread Claire Chang
Introduce the new compatible string, restricted-dma-pool, for restricted
DMA. One can specify the address and length of the restricted DMA memory
region by restricted-dma-pool in the reserved-memory node.

Signed-off-by: Claire Chang 
---
 .../reserved-memory/reserved-memory.txt   | 36 +--
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index e8d3096d922c..46804f24df05 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,23 @@ compatible (optional) - standard definition
   used as a shared pool of DMA buffers for a set of devices. It can
   be used by an operating system to instantiate the necessary pool
   management subsystem if necessary.
+- restricted-dma-pool: This indicates a region of memory meant to be
+  used as a pool of restricted DMA buffers for a set of devices. The
+  memory region would be the only region accessible to those devices.
+  When using this, the no-map and reusable properties must not be set,
+  so the operating system can create a virtual mapping that will be 
used
+  for synchronization. The main purpose for restricted DMA is to
+  mitigate the lack of DMA access control on systems without an IOMMU,
+  which could result in the DMA accessing the system memory at
+  unexpected times and/or unexpected addresses, possibly leading to 
data
+  leakage or corruption. The feature on its own provides a basic level
+  of protection against the DMA overwriting buffer contents at
+  unexpected times. However, to protect against general data leakage 
and
+  system memory corruption, the system needs to provide way to lock 
down
+  the memory access, e.g., MPU. Note that since coherent allocation
+  needs remapping, one must set up another device coherent pool by
+  shared-dma-pool and use dma_alloc_from_dev_coherent instead for 
atomic
+  coherent allocation.
 - vendor specific string in the form ,[-]
 no-map (optional) - empty property
 - Indicates the operating system must not create a virtual mapping
@@ -85,10 +102,11 @@ memory-region-names (optional) - a list of names, one for 
each corresponding
 
 Example
 ---
-This example defines 3 contiguous regions are defined for Linux kernel:
+This example defines 4 contiguous regions for Linux kernel:
 one default of all device drivers (named linux,cma@7200 and 64MiB in size),
-one dedicated to the framebuffer device (named framebuffer@7800, 8MiB), and
-one for multimedia processing (named multimedia-memory@7700, 64MiB).
+one dedicated to the framebuffer device (named framebuffer@7800, 8MiB),
+one for multimedia processing (named multimedia-memory@7700, 64MiB), and
+one for restricted dma pool (named restricted_dma_reserved@0x5000, 64MiB).
 
 / {
#address-cells = <1>;
@@ -120,6 +138,11 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+   restricted_dma_reserved: restricted_dma_reserved {
+   compatible = "restricted-dma-pool";
+   reg = <0x5000 0x400>;
+   };
};
 
/* ... */
@@ -138,4 +161,11 @@ one for multimedia processing (named 
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+   pcie_device: pcie_device@0,0 {
+   reg = <0x8301 0x0 0x 0x0 0x0010
+  0x8301 0x0 0x0010 0x0 0x0010>;
+   memory-region = <_dma_mem_reserved>;
+   /* ... */
+   };
 };
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 10/12] swiotlb: Add restricted DMA alloc/free support

2021-06-15 Thread Claire Chang
Add the functions, swiotlb_{alloc,free} to support the memory allocation
from restricted DMA pool.

The restricted DMA pool is preferred if available.

Note that since coherent allocation needs remapping, one must set up
another device coherent pool by shared-dma-pool and use
dma_alloc_from_dev_coherent instead for atomic coherent allocation.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h | 15 +
 kernel/dma/direct.c | 50 ++---
 kernel/dma/swiotlb.c| 42 --
 3 files changed, 92 insertions(+), 15 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index e76ac469..9616346b727f 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -157,4 +157,19 @@ static inline void swiotlb_adjust_size(unsigned long size)
 extern void swiotlb_print_info(void);
 extern void swiotlb_set_max_segment(unsigned int);
 
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+struct page *swiotlb_alloc(struct device *dev, size_t size);
+bool swiotlb_free(struct device *dev, struct page *page, size_t size);
+#else
+static inline struct page *swiotlb_alloc(struct device *dev, size_t size)
+{
+   return NULL;
+}
+static inline bool swiotlb_free(struct device *dev, struct page *page,
+   size_t size)
+{
+   return false;
+}
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
+
 #endif /* __LINUX_SWIOTLB_H */
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 3713461d6fe0..da0e09621230 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -75,6 +75,15 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t 
phys, size_t size)
min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit);
 }
 
+static void __dma_direct_free_pages(struct device *dev, struct page *page,
+   size_t size)
+{
+   if (IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL) &&
+   swiotlb_free(dev, page, size))
+   return;
+   dma_free_contiguous(dev, page, size);
+}
+
 static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
gfp_t gfp)
 {
@@ -86,7 +95,16 @@ static struct page *__dma_direct_alloc_pages(struct device 
*dev, size_t size,
 
gfp |= dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
   _limit);
-   page = dma_alloc_contiguous(dev, size, gfp);
+   if (IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL)) {
+   page = swiotlb_alloc(dev, size);
+   if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
+   __dma_direct_free_pages(dev, page, size);
+   return NULL;
+   }
+   }
+
+   if (!page)
+   page = dma_alloc_contiguous(dev, size, gfp);
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
dma_free_contiguous(dev, page, size);
page = NULL;
@@ -142,7 +160,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
gfp |= __GFP_NOWARN;
 
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-   !force_dma_unencrypted(dev)) {
+   !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) {
page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO);
if (!page)
return NULL;
@@ -155,18 +173,23 @@ void *dma_direct_alloc(struct device *dev, size_t size,
}
 
if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
-   !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
-   !dev_is_dma_coherent(dev))
+   !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) &&
+   !is_dev_swiotlb_force(dev))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
 
/*
 * Remapping or decrypting memory may block. If either is required and
 * we can't block, allocate the memory from the atomic pools.
+* If restricted DMA (i.e., is_dev_swiotlb_force) is required, one must
+* set up another device coherent pool by shared-dma-pool and use
+* dma_alloc_from_dev_coherent instead.
 */
if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
!gfpflags_allow_blocking(gfp) &&
(force_dma_unencrypted(dev) ||
-(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && 
!dev_is_dma_coherent(dev
+(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
+ !dev_is_dma_coherent(dev))) &&
+   !is_dev_swiotlb_force(dev))
return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
/* we always manually zero the memory once we are done */
@@ -237,7 +260,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
return NULL;
}
 out_free_pages:
-   dma_free_contiguous(dev, page, size);
+   __dma_direct_free_pages(dev, page, size);

[PATCH v10 09/12] swiotlb: Add restricted DMA pool initialization

2021-06-15 Thread Claire Chang
Add the initialization function to create restricted DMA pools from
matching reserved-memory nodes.

Regardless of swiotlb setting, the restricted DMA pool is preferred if
available.

The restricted DMA pools provide a basic level of protection against the
DMA overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system
needs to provide a way to lock down the memory access, e.g., MPU.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h |  3 +-
 kernel/dma/Kconfig  | 14 
 kernel/dma/swiotlb.c| 78 +
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index efcd56e3a16c..e76ac469 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -73,7 +73,8 @@ extern enum swiotlb_force swiotlb_force;
  * range check to see if the memory was in fact allocated by this
  * API.
  * @nslabs:The number of IO TLB blocks (in groups of 64) between @start and
- * @end. This is command line adjustable via setup_io_tlb_npages.
+ * @end. For default swiotlb, this is command line adjustable via
+ * setup_io_tlb_npages.
  * @used:  The number of used IO TLB block.
  * @list:  The free list describing the number of free entries available
  * from each index.
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 77b405508743..3e961dc39634 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -80,6 +80,20 @@ config SWIOTLB
bool
select NEED_DMA_MAP_STATE
 
+config DMA_RESTRICTED_POOL
+   bool "DMA Restricted Pool"
+   depends on OF && OF_RESERVED_MEM
+   select SWIOTLB
+   help
+ This enables support for restricted DMA pools which provide a level of
+ DMA memory protection on systems with limited hardware protection
+ capabilities, such as those lacking an IOMMU.
+
+ For more information see
+ 

+ and .
+ If unsure, say "n".
+
 #
 # Should be selected if we can mmap non-coherent mappings to userspace.
 # The only thing that is really required is a way to set an uncached bit
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 3c01162c400b..ef1ccd63534d 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -39,6 +39,13 @@
 #ifdef CONFIG_DEBUG_FS
 #include 
 #endif
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
 
 #include 
 #include 
@@ -693,3 +700,74 @@ static int __init swiotlb_create_default_debugfs(void)
 late_initcall(swiotlb_create_default_debugfs);
 
 #endif
+
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
+   struct device *dev)
+{
+   struct io_tlb_mem *mem = rmem->priv;
+   unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
+
+   /*
+* Since multiple devices can share the same pool, the private data,
+* io_tlb_mem struct, will be initialized by the first device attached
+* to it.
+*/
+   if (!mem) {
+   mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL);
+   if (!mem)
+   return -ENOMEM;
+
+   swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false);
+   mem->force = true;
+   set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
+rmem->size >> PAGE_SHIFT);
+
+   rmem->priv = mem;
+
+   if (IS_ENABLED(CONFIG_DEBUG_FS)) {
+   mem->debugfs =
+   debugfs_create_dir(rmem->name, debugfs_dir);
+   swiotlb_create_debugfs_files(mem);
+   }
+   }
+
+   dev->dma_io_tlb_mem = mem;
+
+   return 0;
+}
+
+static void rmem_swiotlb_device_release(struct reserved_mem *rmem,
+   struct device *dev)
+{
+   dev->dma_io_tlb_mem = io_tlb_default_mem;
+}
+
+static const struct reserved_mem_ops rmem_swiotlb_ops = {
+   .device_init = rmem_swiotlb_device_init,
+   .device_release = rmem_swiotlb_device_release,
+};
+
+static int __init rmem_swiotlb_setup(struct reserved_mem *rmem)
+{
+   unsigned long node = rmem->fdt_node;
+
+   if (of_get_flat_dt_prop(node, "reusable", NULL) ||
+   of_get_flat_dt_prop(node, "linux,cma-default", NULL) ||
+   of_get_flat_dt_prop(node, "linux,dma-default", NULL) ||
+   of_get_flat_dt_prop(node, "no-map", NULL))
+   return -EINVAL;
+
+   if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base {
+   pr_err("Restricted DMA pool must be accessible within the 
linear mapping.");
+   return -EINVAL;
+   }
+
+   rmem->ops = _swiotlb_ops;
+   pr_info("Reserved 

[PATCH v10 08/12] swiotlb: Refactor swiotlb_tbl_unmap_single

2021-06-15 Thread Claire Chang
Add a new function, swiotlb_release_slots, to make the code reusable for
supporting different bounce buffer pools.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e498f11e150e..3c01162c400b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -546,27 +546,15 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, 
phys_addr_t orig_addr,
return tlb_addr;
 }
 
-/*
- * tlb_addr is the physical address of the bounce buffer to unmap.
- */
-void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
- size_t mapping_size, enum dma_data_direction dir,
- unsigned long attrs)
+static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
 {
-   struct io_tlb_mem *mem = hwdev->dma_io_tlb_mem;
+   struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned long flags;
-   unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr);
+   unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
int nslots = nr_slots(mem->slots[index].alloc_size + offset);
int count, i;
 
-   /*
-* First, sync the memory before unmapping the entry
-*/
-   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-   (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL))
-   swiotlb_bounce(hwdev, tlb_addr, mapping_size, DMA_FROM_DEVICE);
-
/*
 * Return the buffer to the free list by setting the corresponding
 * entries to indicate the number of contiguous entries available.
@@ -601,6 +589,23 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, 
phys_addr_t tlb_addr,
spin_unlock_irqrestore(>lock, flags);
 }
 
+/*
+ * tlb_addr is the physical address of the bounce buffer to unmap.
+ */
+void swiotlb_tbl_unmap_single(struct device *dev, phys_addr_t tlb_addr,
+ size_t mapping_size, enum dma_data_direction dir,
+ unsigned long attrs)
+{
+   /*
+* First, sync the memory before unmapping the entry
+*/
+   if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
+   (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL))
+   swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_FROM_DEVICE);
+
+   swiotlb_release_slots(dev, tlb_addr);
+}
+
 void swiotlb_sync_single_for_device(struct device *dev, phys_addr_t tlb_addr,
size_t size, enum dma_data_direction dir)
 {
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 07/12] swiotlb: Move alloc_size to swiotlb_find_slots

2021-06-15 Thread Claire Chang
Rename find_slots to swiotlb_find_slots and move the maintenance of
alloc_size to it for better code reusability later.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 5af47a8f68b8..e498f11e150e 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -422,8 +422,8 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, 
unsigned int index)
  * Find a suitable number of IO TLB entries size that will fit this request and
  * allocate a buffer from that IO TLB pool.
  */
-static int find_slots(struct device *dev, phys_addr_t orig_addr,
-   size_t alloc_size)
+static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
+ size_t alloc_size)
 {
struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
@@ -478,8 +478,11 @@ static int find_slots(struct device *dev, phys_addr_t 
orig_addr,
return -1;
 
 found:
-   for (i = index; i < index + nslots; i++)
+   for (i = index; i < index + nslots; i++) {
mem->slots[i].list = 0;
+   mem->slots[i].alloc_size =
+   alloc_size - ((i - index) << IO_TLB_SHIFT);
+   }
for (i = index - 1;
 io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
 mem->slots[i].list; i--)
@@ -520,7 +523,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, 
phys_addr_t orig_addr,
return (phys_addr_t)DMA_MAPPING_ERROR;
}
 
-   index = find_slots(dev, orig_addr, alloc_size + offset);
+   index = swiotlb_find_slots(dev, orig_addr, alloc_size + offset);
if (index == -1) {
if (!(attrs & DMA_ATTR_NO_WARN))
dev_warn_ratelimited(dev,
@@ -534,11 +537,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, 
phys_addr_t orig_addr,
 * This is needed when we sync the memory.  Then we sync the buffer if
 * needed.
 */
-   for (i = 0; i < nr_slots(alloc_size + offset); i++) {
+   for (i = 0; i < nr_slots(alloc_size + offset); i++)
mem->slots[index + i].orig_addr = slot_addr(orig_addr, i);
-   mem->slots[index + i].alloc_size =
-   alloc_size - (i << IO_TLB_SHIFT);
-   }
tlb_addr = slot_addr(mem->start, index) + offset;
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
(dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 06/12] swiotlb: Use is_dev_swiotlb_force for swiotlb data bouncing

2021-06-15 Thread Claire Chang
Propagate the swiotlb_force setting into io_tlb_default_mem->force and
use it to determine whether to bounce the data or not. This will be
useful later to allow for different pools.

Signed-off-by: Claire Chang 
---
 include/linux/swiotlb.h | 11 +++
 kernel/dma/direct.c |  2 +-
 kernel/dma/direct.h |  2 +-
 kernel/dma/swiotlb.c|  4 
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index dd1c30a83058..efcd56e3a16c 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -84,6 +84,7 @@ extern enum swiotlb_force swiotlb_force;
  * unmap calls.
  * @debugfs:   The dentry to debugfs.
  * @late_alloc:%true if allocated using the page allocator
+ * @force:  %true if swiotlb is forced
  */
 struct io_tlb_mem {
phys_addr_t start;
@@ -94,6 +95,7 @@ struct io_tlb_mem {
spinlock_t lock;
struct dentry *debugfs;
bool late_alloc;
+   bool force;
struct io_tlb_slot {
phys_addr_t orig_addr;
size_t alloc_size;
@@ -109,6 +111,11 @@ static inline bool is_swiotlb_buffer(struct device *dev, 
phys_addr_t paddr)
return mem && paddr >= mem->start && paddr < mem->end;
 }
 
+static inline bool is_dev_swiotlb_force(struct device *dev)
+{
+   return dev->dma_io_tlb_mem->force;
+}
+
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
@@ -120,6 +127,10 @@ static inline bool is_swiotlb_buffer(struct device *dev, 
phys_addr_t paddr)
 {
return false;
 }
+static inline bool is_dev_swiotlb_force(struct device *dev)
+{
+   return false;
+}
 static inline void swiotlb_exit(void)
 {
 }
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 7a88c34d0867..3713461d6fe0 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -496,7 +496,7 @@ size_t dma_direct_max_mapping_size(struct device *dev)
 {
/* If SWIOTLB is active, use its maximum mapping size */
if (is_swiotlb_active(dev) &&
-   (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
+   (dma_addressing_limited(dev) || is_dev_swiotlb_force(dev)))
return swiotlb_max_mapping_size(dev);
return SIZE_MAX;
 }
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index 13e9e7158d94..6c4d13caceb1 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -87,7 +87,7 @@ static inline dma_addr_t dma_direct_map_page(struct device 
*dev,
phys_addr_t phys = page_to_phys(page) + offset;
dma_addr_t dma_addr = phys_to_dma(dev, phys);
 
-   if (unlikely(swiotlb_force == SWIOTLB_FORCE))
+   if (is_dev_swiotlb_force(dev))
return swiotlb_map(dev, phys, size, dir, attrs);
 
if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d07e32020edf..5af47a8f68b8 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -179,6 +179,10 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem 
*mem, phys_addr_t start,
mem->end = mem->start + bytes;
mem->index = 0;
mem->late_alloc = late_alloc;
+
+   if (swiotlb_force == SWIOTLB_FORCE)
+   mem->force = true;
+
spin_lock_init(>lock);
for (i = 0; i < mem->nslabs; i++) {
mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 05/12] swiotlb: Update is_swiotlb_active to add a struct device argument

2021-06-15 Thread Claire Chang
Update is_swiotlb_active to add a struct device argument. This will be
useful later to allow for different pools.

Signed-off-by: Claire Chang 
---
 drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_ttm.c| 2 +-
 drivers/pci/xen-pcifront.c   | 2 +-
 include/linux/swiotlb.h  | 4 ++--
 kernel/dma/direct.c  | 2 +-
 kernel/dma/swiotlb.c | 4 ++--
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c 
b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index ce6b664b10aa..89a894354263 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -42,7 +42,7 @@ static int i915_gem_object_get_pages_internal(struct 
drm_i915_gem_object *obj)
 
max_order = MAX_ORDER;
 #ifdef CONFIG_SWIOTLB
-   if (is_swiotlb_active()) {
+   if (is_swiotlb_active(obj->base.dev->dev)) {
unsigned int max_segment;
 
max_segment = swiotlb_max_segment();
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c 
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index f4c2e46b6fe1..2ca9d9a9e5d5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -276,7 +276,7 @@ nouveau_ttm_init(struct nouveau_drm *drm)
}
 
 #if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
-   need_swiotlb = is_swiotlb_active();
+   need_swiotlb = is_swiotlb_active(dev->dev);
 #endif
 
ret = ttm_device_init(>ttm.bdev, _bo_driver, drm->dev->dev,
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index b7a8f3a1921f..0d56985bfe81 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -693,7 +693,7 @@ static int pcifront_connect_and_init_dma(struct 
pcifront_device *pdev)
 
spin_unlock(_dev_lock);
 
-   if (!err && !is_swiotlb_active()) {
+   if (!err && !is_swiotlb_active(>xdev->dev)) {
err = pci_xen_swiotlb_init_late();
if (err)
dev_err(>xdev->dev, "Could not setup SWIOTLB!\n");
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index d1f3d95881cd..dd1c30a83058 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -112,7 +112,7 @@ static inline bool is_swiotlb_buffer(struct device *dev, 
phys_addr_t paddr)
 void __init swiotlb_exit(void);
 unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
-bool is_swiotlb_active(void);
+bool is_swiotlb_active(struct device *dev);
 void __init swiotlb_adjust_size(unsigned long size);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
@@ -132,7 +132,7 @@ static inline size_t swiotlb_max_mapping_size(struct device 
*dev)
return SIZE_MAX;
 }
 
-static inline bool is_swiotlb_active(void)
+static inline bool is_swiotlb_active(struct device *dev)
 {
return false;
 }
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 84c9feb5474a..7a88c34d0867 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -495,7 +495,7 @@ int dma_direct_supported(struct device *dev, u64 mask)
 size_t dma_direct_max_mapping_size(struct device *dev)
 {
/* If SWIOTLB is active, use its maximum mapping size */
-   if (is_swiotlb_active() &&
+   if (is_swiotlb_active(dev) &&
(dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
return swiotlb_max_mapping_size(dev);
return SIZE_MAX;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 949a6bb21343..d07e32020edf 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -654,9 +654,9 @@ size_t swiotlb_max_mapping_size(struct device *dev)
return ((size_t)IO_TLB_SIZE) * IO_TLB_SEGSIZE;
 }
 
-bool is_swiotlb_active(void)
+bool is_swiotlb_active(struct device *dev)
 {
-   return io_tlb_default_mem != NULL;
+   return dev->dma_io_tlb_mem != NULL;
 }
 EXPORT_SYMBOL_GPL(is_swiotlb_active);
 
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 04/12] swiotlb: Update is_swiotlb_buffer to add a struct device argument

2021-06-15 Thread Claire Chang
Update is_swiotlb_buffer to add a struct device argument. This will be
useful later to allow for different pools.

Signed-off-by: Claire Chang 
---
 drivers/iommu/dma-iommu.c | 12 ++--
 drivers/xen/swiotlb-xen.c |  2 +-
 include/linux/swiotlb.h   |  7 ---
 kernel/dma/direct.c   |  6 +++---
 kernel/dma/direct.h   |  6 +++---
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 5d96fcc45fec..1a6a08908245 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -506,7 +506,7 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, 
dma_addr_t dma_addr,
 
__iommu_dma_unmap(dev, dma_addr, size);
 
-   if (unlikely(is_swiotlb_buffer(phys)))
+   if (unlikely(is_swiotlb_buffer(dev, phys)))
swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
 }
 
@@ -577,7 +577,7 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device 
*dev, phys_addr_t phys,
}
 
iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
-   if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
+   if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(dev, phys))
swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
return iova;
 }
@@ -783,7 +783,7 @@ static void iommu_dma_sync_single_for_cpu(struct device 
*dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(phys, size, dir);
 
-   if (is_swiotlb_buffer(phys))
+   if (is_swiotlb_buffer(dev, phys))
swiotlb_sync_single_for_cpu(dev, phys, size, dir);
 }
 
@@ -796,7 +796,7 @@ static void iommu_dma_sync_single_for_device(struct device 
*dev,
return;
 
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
-   if (is_swiotlb_buffer(phys))
+   if (is_swiotlb_buffer(dev, phys))
swiotlb_sync_single_for_device(dev, phys, size, dir);
 
if (!dev_is_dma_coherent(dev))
@@ -817,7 +817,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
 
-   if (is_swiotlb_buffer(sg_phys(sg)))
+   if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_sync_single_for_cpu(dev, sg_phys(sg),
sg->length, dir);
}
@@ -834,7 +834,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
return;
 
for_each_sg(sgl, sg, nelems, i) {
-   if (is_swiotlb_buffer(sg_phys(sg)))
+   if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_sync_single_for_device(dev, sg_phys(sg),
   sg->length, dir);
 
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 24d11861ac7d..0c4fb34f11ab 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -100,7 +100,7 @@ static int is_xen_swiotlb_buffer(struct device *dev, 
dma_addr_t dma_addr)
 * in our domain. Therefore _only_ check address within our domain.
 */
if (pfn_valid(PFN_DOWN(paddr)))
-   return is_swiotlb_buffer(paddr);
+   return is_swiotlb_buffer(dev, paddr);
return 0;
 }
 
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 216854a5e513..d1f3d95881cd 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -2,6 +2,7 @@
 #ifndef __LINUX_SWIOTLB_H
 #define __LINUX_SWIOTLB_H
 
+#include 
 #include 
 #include 
 #include 
@@ -101,9 +102,9 @@ struct io_tlb_mem {
 };
 extern struct io_tlb_mem *io_tlb_default_mem;
 
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
-   struct io_tlb_mem *mem = io_tlb_default_mem;
+   struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 
return mem && paddr >= mem->start && paddr < mem->end;
 }
@@ -115,7 +116,7 @@ bool is_swiotlb_active(void);
 void __init swiotlb_adjust_size(unsigned long size);
 #else
 #define swiotlb_force SWIOTLB_NO_FORCE
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
return false;
 }
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index f737e3347059..84c9feb5474a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -343,7 +343,7 @@ void dma_direct_sync_sg_for_device(struct device *dev,
for_each_sg(sgl, sg, nents, i) {
phys_addr_t paddr = dma_to_phys(dev, sg_dma_address(sg));
 
-   if (unlikely(is_swiotlb_buffer(paddr)))
+   if (unlikely(is_swiotlb_buffer(dev, paddr)))
swiotlb_sync_single_for_device(dev, paddr, sg->length,

[PATCH v10 03/12] swiotlb: Set dev->dma_io_tlb_mem to the swiotlb pool used

2021-06-15 Thread Claire Chang
Always have the pointer to the swiotlb pool used in struct device. This
could help simplify the code for other pools.

Signed-off-by: Claire Chang 
---
 drivers/base/core.c| 4 
 include/linux/device.h | 4 
 kernel/dma/swiotlb.c   | 8 
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index b8a8c96dca58..eeb2d49d3aa3 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include  /* for dma_default_coherent */
 
@@ -2846,6 +2847,9 @@ void device_initialize(struct device *dev)
 defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
dev->dma_coherent = dma_default_coherent;
 #endif
+#ifdef CONFIG_SWIOTLB
+   dev->dma_io_tlb_mem = io_tlb_default_mem;
+#endif
 }
 EXPORT_SYMBOL_GPL(device_initialize);
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 4443e12238a0..2e9a378c9100 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -432,6 +432,7 @@ struct dev_links_info {
  * @dma_pools: Dma pools (if dma'ble device).
  * @dma_mem:   Internal for coherent mem override.
  * @cma_area:  Contiguous memory area for dma allocations
+ * @dma_io_tlb_mem: Pointer to the swiotlb pool used.  Not for driver use.
  * @archdata:  For arch-specific additions.
  * @of_node:   Associated device tree node.
  * @fwnode:Associated device node supplied by platform firmware.
@@ -540,6 +541,9 @@ struct device {
 #ifdef CONFIG_DMA_CMA
struct cma *cma_area;   /* contiguous memory area for dma
   allocations */
+#endif
+#ifdef CONFIG_SWIOTLB
+   struct io_tlb_mem *dma_io_tlb_mem;
 #endif
/* arch specific additions */
struct dev_archdata archdata;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 97c6ad50fdc2..949a6bb21343 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -339,7 +339,7 @@ void __init swiotlb_exit(void)
 static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t 
size,
   enum dma_data_direction dir)
 {
-   struct io_tlb_mem *mem = io_tlb_default_mem;
+   struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
phys_addr_t orig_addr = mem->slots[index].orig_addr;
size_t alloc_size = mem->slots[index].alloc_size;
@@ -421,7 +421,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, 
unsigned int index)
 static int find_slots(struct device *dev, phys_addr_t orig_addr,
size_t alloc_size)
 {
-   struct io_tlb_mem *mem = io_tlb_default_mem;
+   struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -498,7 +498,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, 
phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
 {
-   struct io_tlb_mem *mem = io_tlb_default_mem;
+   struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned int i;
int index;
@@ -549,7 +549,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, 
phys_addr_t tlb_addr,
  size_t mapping_size, enum dma_data_direction dir,
  unsigned long attrs)
 {
-   struct io_tlb_mem *mem = io_tlb_default_mem;
+   struct io_tlb_mem *mem = hwdev->dma_io_tlb_mem;
unsigned long flags;
unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr);
int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 02/12] swiotlb: Refactor swiotlb_create_debugfs

2021-06-15 Thread Claire Chang
Split the debugfs creation to make the code reusable for supporting
different bounce buffer pools.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c64298e416c8..97c6ad50fdc2 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -661,19 +661,26 @@ bool is_swiotlb_active(void)
 EXPORT_SYMBOL_GPL(is_swiotlb_active);
 
 #ifdef CONFIG_DEBUG_FS
+static struct dentry *debugfs_dir;
 
-static int __init swiotlb_create_debugfs(void)
+static void swiotlb_create_debugfs_files(struct io_tlb_mem *mem)
 {
-   struct io_tlb_mem *mem = io_tlb_default_mem;
-
-   if (!mem)
-   return 0;
-   mem->debugfs = debugfs_create_dir("swiotlb", NULL);
debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, >nslabs);
debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, >used);
+}
+
+static int __init swiotlb_create_default_debugfs(void)
+{
+   struct io_tlb_mem *mem = io_tlb_default_mem;
+
+   debugfs_dir = debugfs_create_dir("swiotlb", NULL);
+   if (mem) {
+   mem->debugfs = debugfs_dir;
+   swiotlb_create_debugfs_files(mem);
+   }
return 0;
 }
 
-late_initcall(swiotlb_create_debugfs);
+late_initcall(swiotlb_create_default_debugfs);
 
 #endif
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 01/12] swiotlb: Refactor swiotlb init functions

2021-06-15 Thread Claire Chang
Add a new function, swiotlb_init_io_tlb_mem, for the io_tlb_mem struct
initialization to make the code reusable.

Signed-off-by: Claire Chang 
---
 kernel/dma/swiotlb.c | 49 ++--
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 8ca7d505d61c..c64298e416c8 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -168,9 +168,28 @@ void __init swiotlb_update_mem_attributes(void)
memset(vaddr, 0, bytes);
 }
 
-int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
+static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
+   unsigned long nslabs, bool late_alloc)
 {
+   void *vaddr = phys_to_virt(start);
unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
+
+   mem->nslabs = nslabs;
+   mem->start = start;
+   mem->end = mem->start + bytes;
+   mem->index = 0;
+   mem->late_alloc = late_alloc;
+   spin_lock_init(>lock);
+   for (i = 0; i < mem->nslabs; i++) {
+   mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
+   mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
+   mem->slots[i].alloc_size = 0;
+   }
+   memset(vaddr, 0, bytes);
+}
+
+int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
+{
struct io_tlb_mem *mem;
size_t alloc_size;
 
@@ -186,16 +205,8 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long 
nslabs, int verbose)
if (!mem)
panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
  __func__, alloc_size, PAGE_SIZE);
-   mem->nslabs = nslabs;
-   mem->start = __pa(tlb);
-   mem->end = mem->start + bytes;
-   mem->index = 0;
-   spin_lock_init(>lock);
-   for (i = 0; i < mem->nslabs; i++) {
-   mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
-   mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
-   mem->slots[i].alloc_size = 0;
-   }
+
+   swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false);
 
io_tlb_default_mem = mem;
if (verbose)
@@ -282,8 +293,8 @@ swiotlb_late_init_with_default_size(size_t default_size)
 int
 swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
 {
-   unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
struct io_tlb_mem *mem;
+   unsigned long bytes = nslabs << IO_TLB_SHIFT;
 
if (swiotlb_force == SWIOTLB_NO_FORCE)
return 0;
@@ -297,20 +308,8 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
if (!mem)
return -ENOMEM;
 
-   mem->nslabs = nslabs;
-   mem->start = virt_to_phys(tlb);
-   mem->end = mem->start + bytes;
-   mem->index = 0;
-   mem->late_alloc = 1;
-   spin_lock_init(>lock);
-   for (i = 0; i < mem->nslabs; i++) {
-   mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
-   mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
-   mem->slots[i].alloc_size = 0;
-   }
-
+   swiotlb_init_io_tlb_mem(mem, virt_to_phys(tlb), nslabs, true);
set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
-   memset(tlb, 0, bytes);
 
io_tlb_default_mem = mem;
swiotlb_print_info();
-- 
2.32.0.272.g935e593368-goog



[PATCH v10 00/12] Restricted DMA

2021-06-15 Thread Claire Chang
This series implements mitigations for lack of DMA access control on
systems without an IOMMU, which could result in the DMA accessing the
system memory at unexpected times and/or unexpected addresses, possibly
leading to data leakage or corruption.

For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is
not behind an IOMMU. As PCI-e, by design, gives the device full access to
system memory, a vulnerability in the Wi-Fi firmware could easily escalate
to a full system exploit (remote wifi exploits: [1a], [1b] that shows a
full chain of exploits; [2], [3]).

To mitigate the security concerns, we introduce restricted DMA. Restricted
DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a
specially allocated region and does memory allocation from the same region.
The feature on its own provides a basic level of protection against the DMA
overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system needs
to provide a way to restrict the DMA to a predefined memory region (this is
usually done at firmware level, e.g. MPU in ATF on some ARM platforms [4]).

[1a] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html
[1b] 
https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html
[2] https://blade.tencent.com/en/advisories/qualpwn/
[3] 
https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/
[4] 
https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132

v10:
Address the comments in v9 to
  - fix the dev->dma_io_tlb_mem assignment
  - propagate swiotlb_force setting into io_tlb_default_mem->force
  - move set_memory_decrypted out of swiotlb_init_io_tlb_mem
  - move debugfs_dir declaration into the main CONFIG_DEBUG_FS block
  - add swiotlb_ prefix to find_slots and release_slots
  - merge the 3 alloc/free related patches
  - move the CONFIG_DMA_RESTRICTED_POOL later

v9:
Address the comments in v7 to
  - set swiotlb active pool to dev->dma_io_tlb_mem
  - get rid of get_io_tlb_mem
  - dig out the device struct for is_swiotlb_active
  - move debugfs_create_dir out of swiotlb_create_debugfs
  - do set_memory_decrypted conditionally in swiotlb_init_io_tlb_mem
  - use IS_ENABLED in kernel/dma/direct.c
  - fix redefinition of 'of_dma_set_restricted_buffer'
https://lore.kernel.org/patchwork/cover/1445081/

v8:
- Fix reserved-memory.txt and add the reg property in example.
- Fix sizeof for of_property_count_elems_of_size in
  drivers/of/address.c#of_dma_set_restricted_buffer.
- Apply Will's suggestion to try the OF node having DMA configuration in
  drivers/of/address.c#of_dma_set_restricted_buffer.
- Fix typo in the comment of drivers/of/address.c#of_dma_set_restricted_buffer.
- Add error message for PageHighMem in
  kernel/dma/swiotlb.c#rmem_swiotlb_device_init and move it to
  rmem_swiotlb_setup.
- Fix the message string in rmem_swiotlb_setup.
https://lore.kernel.org/patchwork/cover/1437112/

v7:
Fix debugfs, PageHighMem and comment style in rmem_swiotlb_device_init
https://lore.kernel.org/patchwork/cover/1431031/

v6:
Address the comments in v5
https://lore.kernel.org/patchwork/cover/1423201/

v5:
Rebase on latest linux-next
https://lore.kernel.org/patchwork/cover/1416899/

v4:
- Fix spinlock bad magic
- Use rmem->name for debugfs entry
- Address the comments in v3
https://lore.kernel.org/patchwork/cover/1378113/

v3:
Using only one reserved memory region for both streaming DMA and memory
allocation.
https://lore.kernel.org/patchwork/cover/1360992/

v2:
Building on top of swiotlb.
https://lore.kernel.org/patchwork/cover/1280705/

v1:
Using dma_map_ops.
https://lore.kernel.org/patchwork/cover/1271660/


Claire Chang (12):
  swiotlb: Refactor swiotlb init functions
  swiotlb: Refactor swiotlb_create_debugfs
  swiotlb: Set dev->dma_io_tlb_mem to the swiotlb pool used
  swiotlb: Update is_swiotlb_buffer to add a struct device argument
  swiotlb: Update is_swiotlb_active to add a struct device argument
  swiotlb: Use is_dev_swiotlb_force for swiotlb data bouncing
  swiotlb: Move alloc_size to swiotlb_find_slots
  swiotlb: Refactor swiotlb_tbl_unmap_single
  swiotlb: Add restricted DMA pool initialization
  swiotlb: Add restricted DMA alloc/free support
  dt-bindings: of: Add restricted DMA pool
  of: Add plumbing for restricted DMA pool

 .../reserved-memory/reserved-memory.txt   |  36 ++-
 drivers/base/core.c   |   4 +
 drivers/gpu/drm/i915/gem/i915_gem_internal.c  |   2 +-
 drivers/gpu/drm/nouveau/nouveau_ttm.c |   2 +-
 drivers/iommu/dma-iommu.c |  12 +-
 drivers/of/address.c  |  33 +++
 drivers/of/device.c   |   3 +
 drivers/of/of_private.h   |   6 +
 drivers/pci/xen-pcifront.c|   2 

[PATCH] drm/nouveau: fix double free in nouveau_gem_new()

2021-06-15 Thread Dan Carpenter
The ttm_bo_init_reserved() function calls ttm_bo_put(bo) which calls
nouveau_bo_del_ttm() which frees the "nvbo.bo" so the nouveau_bo_ref()
call leads to a double free.

Fixes: 019cbd4a4feb ("drm/nouveau: Initialize GEM object before TTM object")
Signed-off-by: Dan Carpenter 
---
This fix is obvious enough and fixes my double free, but unfortunately
there are other bugs here so my system still hangs when I try to open
ten latest version Firefox windows in a row.

 drivers/gpu/drm/nouveau/nouveau_gem.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index c88cbb85f101..d612c1a720f8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -211,10 +211,8 @@ nouveau_gem_new(struct nouveau_cli *cli, u64 size, int 
align, uint32_t domain,
}
 
ret = nouveau_bo_init(nvbo, size, align, domain, NULL, NULL);
-   if (ret) {
-   nouveau_bo_ref(NULL, );
+   if (ret)
return ret;
-   }
 
/* we restrict allowed domains on nv50+ to only the types
 * that were requested at creation time.  not possibly on
-- 
2.30.2



Re: [PATCH] drm: display: Fix duplicate field initialization in dcn31

2021-06-15 Thread Rodrigo Siqueira
On 06/15, Wan Jiabing wrote:
> Fix the following coccicheck warning:
> drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c:917:56-57:
> pstate_enabled: first occurrence line 935, second occurrence line 937
> 
> Signed-off-by: Wan Jiabing 
> ---
>  drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> index 0d6cb6caad81..c67bc9544f5d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> @@ -934,7 +934,6 @@ static const struct dc_debug_options debug_defaults_drv = 
> {
>   .dmub_command_table = true,
>   .pstate_enabled = true,
>   .use_max_lb = true,
> - .pstate_enabled = true,
>   .enable_mem_low_power = {
>   .bits = {
>   .vga = false,
> -- 
> 2.20.1
> 

Reviewed-by: Rodrigo Siqueira 

Thanks

-- 
Rodrigo Siqueira
https://siqueira.tech


signature.asc
Description: PGP signature


Re: [PATCH] drm/i915/ttm: Fix memory leaks

2021-06-15 Thread Maarten Lankhorst
Op 15-06-2021 om 14:24 schreef Thomas Hellström:
> Fix two memory leaks introduced with the ttm backend.
>
> Fixes: 213d50927763 ("drm/i915/ttm: Introduce a TTM i915 gem object backend")
> Signed-off-by: Thomas Hellström 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index 08b72c280cb5..8059cb61bc3c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -122,6 +122,7 @@ static void i915_ttm_tt_destroy(struct ttm_device *bdev, 
> struct ttm_tt *ttm)
>   struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm);
>  
>   ttm_tt_destroy_common(bdev, ttm);
> + ttm_tt_fini(ttm);
>   kfree(i915_tt);
>  }
>  
> @@ -217,6 +218,7 @@ static void i915_ttm_delete_mem_notify(struct 
> ttm_buffer_object *bo)
>  
>   if (likely(obj)) {
>   /* This releases all gem object bindings to the backend. */
> + i915_ttm_free_cached_io_st(obj);
>   __i915_gem_free_object(obj);
>   }
>  }

Reviewed-by: Maarten Lankhorst 



[PATCH] drm/i915/ttm: Fix memory leaks

2021-06-15 Thread Thomas Hellström
Fix two memory leaks introduced with the ttm backend.

Fixes: 213d50927763 ("drm/i915/ttm: Introduce a TTM i915 gem object backend")
Signed-off-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 08b72c280cb5..8059cb61bc3c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -122,6 +122,7 @@ static void i915_ttm_tt_destroy(struct ttm_device *bdev, 
struct ttm_tt *ttm)
struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm);
 
ttm_tt_destroy_common(bdev, ttm);
+   ttm_tt_fini(ttm);
kfree(i915_tt);
 }
 
@@ -217,6 +218,7 @@ static void i915_ttm_delete_mem_notify(struct 
ttm_buffer_object *bo)
 
if (likely(obj)) {
/* This releases all gem object bindings to the backend. */
+   i915_ttm_free_cached_io_st(obj);
__i915_gem_free_object(obj);
}
 }
-- 
2.31.1



Re: [PATCH 2/2] drm/gud: Use scatter-gather USB bulk transfer

2021-06-15 Thread Noralf Trønnes



Den 15.06.2021 11.17, skrev Peter Stuge:
> Hi Noralf,
> 
> Noralf Trønnes wrote:
 +static int gud_usb_bulk(struct gud_device *gdrm, size_t len)
> ..
 +   timer_setup_on_stack(, gud_usb_bulk_timeout, 0);
 +   mod_timer(, jiffies + msecs_to_jiffies(3000));
 +
 +   usb_sg_wait();
 +
 +   if (!del_timer_sync())
 +   ret = -ETIMEDOUT;
> ..
>>> Mention in the commit message that sending USB bulk transfers with
>>> an sglist could be unstable
> 
> Can you explain a bit about /how/ it is unstable?
> 
> As you write, usb_bulk_msg() (as used before) has a timeout which is
> passed to the host controller hardware and implemented there.
> 
> I haven't used SG with kernel USB but I would expect such a timeout
> to still be available with SG?
> 

I have taken a closer look and usb_bulk_msg() calls usb_start_wait_urb()
which uses wait_for_completion_timeout() so the timeout isn't handled by
the hardware.

usb_sg_wait() on the other hand uses plain wait_for_completion() without
the timeout. So ideally usb_sg_wait() should have had a timeout
parameter and used wait_for_completion_timeout().

> 
>> usb_bulk_msg() has builtin timeout handling and during development of
>> a microcontroller gadget implementation I've triggered this timeout
>> several times when the uC usb interrupts stopped firing.
> 
> The device not responding to bulk packets scheduled and sent by the host
> is a real error /in the device/ and thus not neccessarily something the
> kernel must handle gracefully.. I think it's quite nice to do so, but
> one can argue that it's not strictly required.
> 
> But more importantly: Remember that bulk transfer has no delivery time
> guarantee. It can take indefinitely long until a bulk transfer is
> scheduled by the host on a busy bus which is starved with more
> important things (control, interrupt, iso transfers) - that's not
> an error at all, and may be indistinguishable from the device not
> responding to packets actually sent by the host.
> 
> Having a timeout is important, I just expect the USB SG interface to
> support it since it is the hardware that times out in the non-SG case.
> 
> 
> And since this is essentially real time data maybe a shorter timeout
> is better? 3 seconds seems really long.
> 

I have looked at what the others are using:
- rtsx_usb uses 10 seconds in one place
- vub300 uses 2 seconds plus a length based addition
- usbtest uses 10 seconds.

The other USB DRM driver gm12u320 uses a 1 second timeout per block, so
a worst case timeout of 20 seconds per frame.

3 seconds is a "long time", but compared to the default control request
timeout USB_CTRL_GET_TIMEOUT which is 5 seconds, and which gud uses,
it's not that long. I don't want to put too much limitation on the
device, but ofc can't allow it to hang the driver.

And a timeout is an exception so hitting that probably means something
is seriously wrong. I though of adding some kind of usb bus reset
handling to the driver that kicks in after the device has been
unresponsive for some time, but dropped that since I have so limited
understanding of things USB.

Noralf.

> The timeout must include all latency for a frame, so e.g. 16ms (60 Hz)
> is too short for sure. But maybe something like 500ms?
> 
> 
> //Peter
> 


Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Christian König

Am 15.06.21 um 14:11 schrieb Pan, Xinhui:

2021年6月15日 20:01,Christian König  写道:

Am 15.06.21 um 13:57 schrieb xinhui pan:

Amdgpu set SG flag in populate callback. So TTM still count pages in SG
BO.

It's probably better to fix this instead. E.g. why does amdgpu modify the SG 
flag during populate and not during initial creation? That doesn't seem to make 
sense.

fair enough. Let me have a try.
No idea why we set SG flag in populate years ago.


That's pretty recent IIRC. Felix moved the code around a bit to fix 
another problem.


Christian.




Christian.


One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
  1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
  - if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_add(ttm->num_pages, _pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_add(ttm->num_pages,
-   _dma32_pages_allocated);
-   }
-
while (atomic_long_read(_pages_allocated) > ttm_pages_limit ||
   atomic_long_read(_dma32_pages_allocated) >
   ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ret)
goto error;
  + if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+   atomic_long_add(ttm->num_pages, _pages_allocated);
+   if (bdev->pool.use_dma32)
+   atomic_long_add(ttm->num_pages,
+   _dma32_pages_allocated);
+   }
+
ttm_tt_add_mapping(bdev, ttm);
ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
error:
-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_sub(ttm->num_pages, _pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_sub(ttm->num_pages,
-   _dma32_pages_allocated);
-   }
return ret;
  }
  EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
if (!ttm_tt_is_populated(ttm))
return;
  - ttm_tt_clear_mapping(ttm);
-   if (bdev->funcs->ttm_tt_unpopulate)
-   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-   else
-   ttm_pool_free(>pool, ttm);
-
if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
atomic_long_sub(ttm->num_pages, _pages_allocated);
if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
_dma32_pages_allocated);
}
  + ttm_tt_clear_mapping(ttm);
+   if (bdev->funcs->ttm_tt_unpopulate)
+   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+   else
+   ttm_pool_free(>pool, ttm);
+
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
  }
  




Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Pan, Xinhui


> 2021年6月15日 20:01,Christian König  写道:
> 
> Am 15.06.21 um 13:57 schrieb xinhui pan:
>> Amdgpu set SG flag in populate callback. So TTM still count pages in SG
>> BO.
> 
> It's probably better to fix this instead. E.g. why does amdgpu modify the SG 
> flag during populate and not during initial creation? That doesn't seem to 
> make sense.

fair enough. Let me have a try.
No idea why we set SG flag in populate years ago.

> 
> Christian.
> 
>> One easy way to fix this is lets count pages after populate callback.
>> 
>> We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
>> again and again even if swapout does not swap SG BOs at all.
>> 
>> Signed-off-by: xinhui pan 
>> ---
>>  drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
>>  1 file changed, 13 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>> index a1a25410ec74..4fa0a8cd71c0 100644
>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>> @@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
>>  if (ttm_tt_is_populated(ttm))
>>  return 0;
>>  -   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> -atomic_long_add(ttm->num_pages, _pages_allocated);
>> -if (bdev->pool.use_dma32)
>> -atomic_long_add(ttm->num_pages,
>> -_dma32_pages_allocated);
>> -}
>> -
>>  while (atomic_long_read(_pages_allocated) > ttm_pages_limit ||
>> atomic_long_read(_dma32_pages_allocated) >
>> ttm_dma32_pages_limit) {
>> @@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
>>  if (ret)
>>  goto error;
>>  +   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> +atomic_long_add(ttm->num_pages, _pages_allocated);
>> +if (bdev->pool.use_dma32)
>> +atomic_long_add(ttm->num_pages,
>> +_dma32_pages_allocated);
>> +}
>> +
>>  ttm_tt_add_mapping(bdev, ttm);
>>  ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
>>  if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
>> @@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
>>  return 0;
>>error:
>> -if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> -atomic_long_sub(ttm->num_pages, _pages_allocated);
>> -if (bdev->pool.use_dma32)
>> -atomic_long_sub(ttm->num_pages,
>> -_dma32_pages_allocated);
>> -}
>>  return ret;
>>  }
>>  EXPORT_SYMBOL(ttm_tt_populate);
>> @@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
>> ttm_tt *ttm)
>>  if (!ttm_tt_is_populated(ttm))
>>  return;
>>  -   ttm_tt_clear_mapping(ttm);
>> -if (bdev->funcs->ttm_tt_unpopulate)
>> -bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
>> -else
>> -ttm_pool_free(>pool, ttm);
>> -
>>  if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>>  atomic_long_sub(ttm->num_pages, _pages_allocated);
>>  if (bdev->pool.use_dma32)
>> @@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
>> ttm_tt *ttm)
>>  _dma32_pages_allocated);
>>  }
>>  +   ttm_tt_clear_mapping(ttm);
>> +if (bdev->funcs->ttm_tt_unpopulate)
>> +bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
>> +else
>> +ttm_pool_free(>pool, ttm);
>> +
>>  ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
>>  }
>>  
> 



Re: [PATCH v3] Documentation: gpu: Mention the requirements for new properties

2021-06-15 Thread Simon Ser
On Tuesday, June 15th, 2021 at 12:16, Pekka Paalanen  
wrote:

> Good reminder about CRCs. CRCs have zero tolerance, so they are not
> useful for testing properties that have any leeway, are they?

IIRC, IGT's alpha blending test currently computes the CRC for all
possible roundings, then checks that the hw returns one of the
acceptable CRCs.

With more complex color management properties, this approach might not
be possible and write-back support in hw drivers would really help.


Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Christian König

Am 15.06.21 um 13:57 schrieb xinhui pan:

Amdgpu set SG flag in populate callback. So TTM still count pages in SG
BO.


It's probably better to fix this instead. E.g. why does amdgpu modify 
the SG flag during populate and not during initial creation? That 
doesn't seem to make sense.


Christian.


One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
  1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
  
-	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {

-   atomic_long_add(ttm->num_pages, _pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_add(ttm->num_pages,
-   _dma32_pages_allocated);
-   }
-
while (atomic_long_read(_pages_allocated) > ttm_pages_limit ||
   atomic_long_read(_dma32_pages_allocated) >
   ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ret)
goto error;
  
+	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {

+   atomic_long_add(ttm->num_pages, _pages_allocated);
+   if (bdev->pool.use_dma32)
+   atomic_long_add(ttm->num_pages,
+   _dma32_pages_allocated);
+   }
+
ttm_tt_add_mapping(bdev, ttm);
ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
  
  error:

-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_sub(ttm->num_pages, _pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_sub(ttm->num_pages,
-   _dma32_pages_allocated);
-   }
return ret;
  }
  EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
if (!ttm_tt_is_populated(ttm))
return;
  
-	ttm_tt_clear_mapping(ttm);

-   if (bdev->funcs->ttm_tt_unpopulate)
-   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-   else
-   ttm_pool_free(>pool, ttm);
-
if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
atomic_long_sub(ttm->num_pages, _pages_allocated);
if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
_dma32_pages_allocated);
}
  
+	ttm_tt_clear_mapping(ttm);

+   if (bdev->funcs->ttm_tt_unpopulate)
+   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+   else
+   ttm_pool_free(>pool, ttm);
+
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
  }
  




[RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread xinhui pan
Amdgpu set SG flag in populate callback. So TTM still count pages in SG
BO.
One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
 drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
 
-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_add(ttm->num_pages, _pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_add(ttm->num_pages,
-   _dma32_pages_allocated);
-   }
-
while (atomic_long_read(_pages_allocated) > ttm_pages_limit ||
   atomic_long_read(_dma32_pages_allocated) >
   ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ret)
goto error;
 
+   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+   atomic_long_add(ttm->num_pages, _pages_allocated);
+   if (bdev->pool.use_dma32)
+   atomic_long_add(ttm->num_pages,
+   _dma32_pages_allocated);
+   }
+
ttm_tt_add_mapping(bdev, ttm);
ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
 
 error:
-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_sub(ttm->num_pages, _pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_sub(ttm->num_pages,
-   _dma32_pages_allocated);
-   }
return ret;
 }
 EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
if (!ttm_tt_is_populated(ttm))
return;
 
-   ttm_tt_clear_mapping(ttm);
-   if (bdev->funcs->ttm_tt_unpopulate)
-   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-   else
-   ttm_pool_free(>pool, ttm);
-
if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
atomic_long_sub(ttm->num_pages, _pages_allocated);
if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
_dma32_pages_allocated);
}
 
+   ttm_tt_clear_mapping(ttm);
+   if (bdev->funcs->ttm_tt_unpopulate)
+   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+   else
+   ttm_pool_free(>pool, ttm);
+
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
 }
 
-- 
2.25.1



Re: [PATCH v3] Documentation: gpu: Mention the requirements for new properties

2021-06-15 Thread Pekka Paalanen
On Tue, 15 Jun 2021 12:45:57 +0300
Laurent Pinchart  wrote:

> On Tue, Jun 15, 2021 at 07:15:18AM +, Simon Ser wrote:
> > On Tuesday, June 15th, 2021 at 09:03, Pekka Paalanen  
> > wrote:
> >   
> > > indeed it will, but what else could one do to test userspace KMS
> > > clients in generic CI where all you can have is virtual hardware? Maybe
> > > in the long run VKMS needs to loop back to a userspace daemon that
> > > implements all the complex processing and returns the writeback result
> > > via VKMS again? That daemon would then need a single upstream, like the
> > > kernel, where it is maintained and correctness verified.  
> > 
> > The complex processing must be implemented even without write-back, because
> > user-space can ask for CRCs of the CRTC.
> >   
> > > Or an LD_PRELOAD that hijacks all KMS ioctls and implements virtual
> > > stuff in userspace? Didn't someone already have something like that?
> > > It would need to be lifted to be a required part of kernel UAPI
> > > submissions, I suppose like IGT is nowadays.  
> > 
> > FWIW, I have a mock libdrm [1] for libliftoff. This is nowhere near a full
> > software implementation with write-back connectors, but allows to expose
> > virtual planes and check atomic commits in CI.
> > 
> > [1]: https://github.com/emersion/libliftoff/blob/master/test/libdrm_mock.c
> >   
> > > For compositor developers like me knowing the exact formulas would be a 
> > > huge
> > > benefit as it would allow me to use KMS to off-load precision-sensitive
> > > operations (e.g.  professional color management). Otherwise, compositors
> > > probably need a switch: "high quality color management? Then do not use 
> > > KMS
> > > features."  
> > 
> > I think for alpha blending there are already rounding issues depending on 
> > the
> > hardware. I wouldn't keep my hopes up for any guarantee that all hw uses the
> > exact same formulae for color management stuff.  
> 
> Good, because otherwise you would be very quickly disappointed :-)
> 
> For scaling we would also need to replicate the exact same filter taps,
> which are often not documented.

That is where the documented tolerances come into play.

Userspace projects need screenshot-based testing, and we need to know
how much tolerance we should allow or expect.

Good reminder about CRCs. CRCs have zero tolerance, so they are not
useful for testing properties that have any leeway, are they?


Thanks,
pq


pgpRWRCpP3fJU.pgp
Description: OpenPGP digital signature


[PATCH] drm: display: Fix duplicate field initialization in dcn31

2021-06-15 Thread Wan Jiabing
Fix the following coccicheck warning:
drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c:917:56-57:
pstate_enabled: first occurrence line 935, second occurrence line 937

Signed-off-by: Wan Jiabing 
---
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
index 0d6cb6caad81..c67bc9544f5d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
@@ -934,7 +934,6 @@ static const struct dc_debug_options debug_defaults_drv = {
.dmub_command_table = true,
.pstate_enabled = true,
.use_max_lb = true,
-   .pstate_enabled = true,
.enable_mem_low_power = {
.bits = {
.vga = false,
-- 
2.20.1



[PATCH] drm/i915: Perform execbuffer object locking as a separate step

2021-06-15 Thread Thomas Hellström
To help avoid evicting already resident buffers from the batch we're
processing, perform locking as a separate step.

Signed-off-by: Thomas Hellström 
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 25 ---
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 201fed19d120..394eb40c95b5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -922,21 +922,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
return err;
 }
 
-static int eb_validate_vmas(struct i915_execbuffer *eb)
+static int eb_lock_vmas(struct i915_execbuffer *eb)
 {
unsigned int i;
int err;
 
-   INIT_LIST_HEAD(>unbound);
-
for (i = 0; i < eb->buffer_count; i++) {
-   struct drm_i915_gem_exec_object2 *entry = >exec[i];
struct eb_vma *ev = >vma[i];
struct i915_vma *vma = ev->vma;
 
err = i915_gem_object_lock(vma->obj, >ww);
if (err)
return err;
+   }
+
+   return 0;
+}
+
+static int eb_validate_vmas(struct i915_execbuffer *eb)
+{
+   unsigned int i;
+   int err;
+
+   INIT_LIST_HEAD(>unbound);
+
+   err = eb_lock_vmas(eb);
+   if (err)
+   return err;
+
+   for (i = 0; i < eb->buffer_count; i++) {
+   struct drm_i915_gem_exec_object2 *entry = >exec[i];
+   struct eb_vma *ev = >vma[i];
+   struct i915_vma *vma = ev->vma;
 
err = eb_pin_vma(eb, entry, ev);
if (err == -EDEADLK)
-- 
2.31.1



[PATCH] drm/i915: Remove duplicate include of intel_region_lmem.h

2021-06-15 Thread Wan Jiabing
Fix the following checkinclude.pl warning:
drivers/gpu/drm/i915/gt/intel_region_lmem.c
8   #include "intel_region_lmem.h"
 12 #include "intel_region_lmem.h"

Signed-off-by: Wan Jiabing 
---
 drivers/gpu/drm/i915/gt/intel_region_lmem.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index f7366b054f8e..119eeec98837 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -9,7 +9,6 @@
 #include "intel_region_ttm.h"
 #include "gem/i915_gem_lmem.h"
 #include "gem/i915_gem_region.h"
-#include "intel_region_lmem.h"
 
 static int init_fake_lmem_bar(struct intel_memory_region *mem)
 {
-- 
2.20.1



[PATCH] dma-buf: fix and rework dma_buf_poll

2021-06-15 Thread Christian König
Daniel pointed me towards this function and there are multiple obvious problems
in the implementation.

First of all the retry loop is not working as intended. In general the retry
makes only sense if you grab the reference first and then check the retry. Then
we skipped checking the exclusive fence when shared fences were present. And
last the whole implementation was unnecessary complex and rather hard to
understand which could lead to probably unexpected behavior of the IOCTL.

Fix all this by reworking the implementation from scratch.

Only mildly tested and needs a thoughtful review of the code.

Signed-off-by: Christian König 
---
 drivers/dma-buf/dma-buf.c | 132 +++---
 include/linux/dma-buf.h   |   2 +-
 2 files changed, 54 insertions(+), 80 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..1bd00e18291f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -72,7 +72,7 @@ static void dma_buf_release(struct dentry *dentry)
 * If you hit this BUG() it means someone dropped their ref to the
 * dma-buf while still having pending operation to the buffer.
 */
-   BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
+   BUG_ON(dmabuf->cb_in.active || dmabuf->cb_out.active);
 
dmabuf->ops->release(dmabuf);
 
@@ -206,12 +206,15 @@ static void dma_buf_poll_cb(struct dma_fence *fence, 
struct dma_fence_cb *cb)
 
 static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
 {
+   struct dma_buf_poll_cb_t *dcb;
struct dma_buf *dmabuf;
struct dma_resv *resv;
struct dma_resv_list *fobj;
struct dma_fence *fence_excl;
-   __poll_t events;
unsigned shared_count, seq;
+   struct dma_fence *fence;
+   __poll_t events;
+   int r, i;
 
dmabuf = file->private_data;
if (!dmabuf || !dmabuf->resv)
@@ -225,99 +228,70 @@ static __poll_t dma_buf_poll(struct file *file, 
poll_table *poll)
if (!events)
return 0;
 
+   dcb = events & EPOLLOUT ? >cb_out : >cb_in;
+
+   /* Only queue a new one if we are not still waiting for the old one */
+   spin_lock_irq(>poll.lock);
+   if (dcb->active)
+   events = 0;
+   else
+   dcb->active = events;
+   spin_unlock_irq(>poll.lock);
+   if (!events)
+   return 0;
+
 retry:
seq = read_seqcount_begin(>seq);
rcu_read_lock();
 
fobj = rcu_dereference(resv->fence);
-   if (fobj)
+   if (fobj && events & EPOLLOUT)
shared_count = fobj->shared_count;
else
shared_count = 0;
-   fence_excl = dma_resv_excl_fence(resv);
-   if (read_seqcount_retry(>seq, seq)) {
-   rcu_read_unlock();
-   goto retry;
-   }
 
-   if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) {
-   struct dma_buf_poll_cb_t *dcb = >cb_excl;
-   __poll_t pevents = EPOLLIN;
-
-   if (shared_count == 0)
-   pevents |= EPOLLOUT;
-
-   spin_lock_irq(>poll.lock);
-   if (dcb->active) {
-   dcb->active |= pevents;
-   events &= ~pevents;
-   } else
-   dcb->active = pevents;
-   spin_unlock_irq(>poll.lock);
-
-   if (events & pevents) {
-   if (!dma_fence_get_rcu(fence_excl)) {
-   /* force a recheck */
-   events &= ~pevents;
-   dma_buf_poll_cb(NULL, >cb);
-   } else if (!dma_fence_add_callback(fence_excl, >cb,
-  dma_buf_poll_cb)) {
-   events &= ~pevents;
-   dma_fence_put(fence_excl);
-   } else {
-   /*
-* No callback queued, wake up any additional
-* waiters.
-*/
-   dma_fence_put(fence_excl);
-   dma_buf_poll_cb(NULL, >cb);
-   }
+   for (i = 0; i < shared_count; ++i) {
+   fence = rcu_dereference(fobj->shared[i]);
+   fence = dma_fence_get_rcu(fence);
+   if (!fence || read_seqcount_retry(>seq, seq)) {
+   /* Concurrent modify detected, force re-check */
+   dma_fence_put(fence);
+   rcu_read_unlock();
+   goto retry;
}
-   }
 
-   if ((events & EPOLLOUT) && shared_count > 0) {
-   struct dma_buf_poll_cb_t *dcb = >cb_shared;
-   int i;
-
-   /* Only queue a new callback if no event has fired yet */
-   

Re: [PATCH v6 RESEND 1/3] gpu: drm: separate panel orientation property creating and value setting

2021-06-15 Thread Hsin-Yi Wang
On Thu, May 27, 2021 at 3:42 PM Hsin-Yi Wang  wrote:
>
> drm_dev_register() sets connector->registration_state to
> DRM_CONNECTOR_REGISTERED and dev->registered to true. If
> drm_connector_set_panel_orientation() is first called after
> drm_dev_register(), it will fail several checks and results in following
> warning.
>
> Add a function to create panel orientation property and set default value
> to UNKNOWN, so drivers can call this function to init the property earlier
> , and let the panel set the real value later.
>
> [4.480976] [ cut here ]
> [4.485603] WARNING: CPU: 5 PID: 369 at 
> drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xb4/0xbc
> 
> [4.609772] Call trace:
> [4.612208]  __drm_mode_object_add+0xb4/0xbc
> [4.616466]  drm_mode_object_add+0x20/0x2c
> [4.620552]  drm_property_create+0xdc/0x174
> [4.624723]  drm_property_create_enum+0x34/0x98
> [4.629241]  drm_connector_set_panel_orientation+0x64/0xa0
> [4.634716]  boe_panel_get_modes+0x88/0xd8
> [4.638802]  drm_panel_get_modes+0x2c/0x48
> [4.642887]  panel_bridge_get_modes+0x1c/0x28
> [4.647233]  drm_bridge_connector_get_modes+0xa0/0xd4
> [4.652273]  drm_helper_probe_single_connector_modes+0x218/0x700
> [4.658266]  drm_mode_getconnector+0x1b4/0x45c
> [4.662699]  drm_ioctl_kernel+0xac/0x128
> [4.11]  drm_ioctl+0x268/0x410
> [4.670002]  drm_compat_ioctl+0xdc/0xf0
> [4.673829]  __arm64_compat_sys_ioctl+0xc8/0x100
> [4.678436]  el0_svc_common+0xf4/0x1c0
> [4.682174]  do_el0_svc_compat+0x28/0x3c
> [4.686088]  el0_svc_compat+0x10/0x1c
> [4.689738]  el0_sync_compat_handler+0xa8/0xcc
> [4.694171]  el0_sync_compat+0x178/0x180
> [4.698082] ---[ end trace b4f2db9d9c88610b ]---
> [4.702721] [ cut here ]
> [4.707329] WARNING: CPU: 5 PID: 369 at 
> drivers/gpu/drm/drm_mode_object.c:243 drm_object_attach_property+0x48/0xb8
> 
> [4.833830] Call trace:
> [4.836266]  drm_object_attach_property+0x48/0xb8
> [4.840958]  drm_connector_set_panel_orientation+0x84/0xa0
> [4.846432]  boe_panel_get_modes+0x88/0xd8
> [4.850516]  drm_panel_get_modes+0x2c/0x48
> [4.854600]  panel_bridge_get_modes+0x1c/0x28
> [4.858946]  drm_bridge_connector_get_modes+0xa0/0xd4
> [4.863984]  drm_helper_probe_single_connector_modes+0x218/0x700
> [4.869978]  drm_mode_getconnector+0x1b4/0x45c
> [4.874410]  drm_ioctl_kernel+0xac/0x128
> [4.878320]  drm_ioctl+0x268/0x410
> [4.881711]  drm_compat_ioctl+0xdc/0xf0
> [4.885536]  __arm64_compat_sys_ioctl+0xc8/0x100
> [4.890142]  el0_svc_common+0xf4/0x1c0
> [4.893879]  do_el0_svc_compat+0x28/0x3c
> [4.897791]  el0_svc_compat+0x10/0x1c
> [4.901441]  el0_sync_compat_handler+0xa8/0xcc
> [4.905873]  el0_sync_compat+0x178/0x180
> [4.909783] ---[ end trace b4f2db9d9c88610c ]---
>
> Signed-off-by: Hsin-Yi Wang 
> Reviewed-by: Sean Paul 
> ---

Hi maintainers,

Gentle ping on the thread. Can you help review or pick this series? Thanks!

>  drivers/gpu/drm/drm_connector.c | 58 ++---
>  drivers/gpu/drm/i915/display/icl_dsi.c  |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c |  1 +
>  drivers/gpu/drm/i915/display/vlv_dsi.c  |  1 +
>  include/drm/drm_connector.h |  2 +
>  5 files changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 7631f76e7f34..7189baaabf41 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1210,7 +1210,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
> = {
>   * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
>   * coordinates, so if userspace rotates the picture to adjust for
>   * the orientation it must also apply the same transformation to the
> - * touchscreen input coordinates. This property is initialized by calling
> + * touchscreen input coordinates. This property value is set by calling
>   * drm_connector_set_panel_orientation() or
>   * drm_connector_set_panel_orientation_with_quirk()
>   *
> @@ -2173,8 +2173,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
>   * @connector: connector for which to set the panel-orientation property.
>   * @panel_orientation: drm_panel_orientation value to set
>   *
> - * This function sets the connector's panel_orientation and attaches
> - * a "panel orientation" property to the connector.
> + * This function sets the connector's panel_orientation value. If the 
> property
> + * doesn't exist, it will return an error.
>   *
>   * Calling this function on a connector where the panel_orientation has
>   * already been set is a no-op (e.g. the orientation has been overridden with
> @@ -2205,19 +2205,11 @@ int drm_connector_set_panel_orientation(
> info->panel_orientation = panel_orientation;
>
> prop = 

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

2021-06-15 Thread Cornelia Huck
On Mon, Jun 14 2021, Christoph Hellwig  wrote:

> From: Jason Gunthorpe 
>
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
>
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>   struct vfio_device_ops
>
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/vfio/mdev/mdev_core.c   | 30 ++
>  drivers/vfio/mdev/mdev_driver.c | 10 ++
>  include/linux/mdev.h|  2 ++
>  3 files changed, 34 insertions(+), 8 deletions(-)

Reviewed-by: Cornelia Huck 



Re: [PATCH 06/10] vfio/mdev: Remove CONFIG_VFIO_MDEV_DEVICE

2021-06-15 Thread Cornelia Huck
On Mon, Jun 14 2021, Christoph Hellwig  wrote:

> From: Jason Gunthorpe 
>
> For some reason the vfio_mdev shim mdev_driver has its own module and
> kconfig. As the next patch requires access to it from mdev.ko merge the
> two modules together and remove VFIO_MDEV_DEVICE.
>
> A later patch deletes this driver entirely.
>
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  Documentation/s390/vfio-ap.rst   |  1 -
>  arch/s390/Kconfig|  2 +-
>  drivers/gpu/drm/i915/Kconfig |  2 +-
>  drivers/vfio/mdev/Kconfig|  7 ---
>  drivers/vfio/mdev/Makefile   |  3 +--
>  drivers/vfio/mdev/mdev_core.c| 16 ++--
>  drivers/vfio/mdev/mdev_private.h |  2 ++
>  drivers/vfio/mdev/vfio_mdev.c| 24 +---
>  samples/Kconfig  |  6 +++---
>  9 files changed, 23 insertions(+), 40 deletions(-)

Reviewed-by: Cornelia Huck 



Re: [PATCH 05/10] driver core: Export device_driver_attach()

2021-06-15 Thread Cornelia Huck
On Mon, Jun 14 2021, Christoph Hellwig  wrote:

> From: Jason Gunthorpe 
>
> This is intended as a replacement API for device_bind_driver(). It has at
> least the following benefits:
>
> - Internal locking. Few of the users of device_bind_driver() follow the
>   locking rules
>
> - Calls device driver probe() internally. Notably this means that devm
>   support for probe works correctly as probe() error will call
>   devres_release_all()
>
> - struct device_driver -> dev_groups is supported
>
> - Simplified calling convention, no need to manually call probe().
>
> The general usage is for situations that already know what driver to bind
> and need to ensure the bind is synchronized with other logic. Call
> device_driver_attach() after device_add().
>
> If probe() returns a failure then this will be preserved up through to the
> error return of device_driver_attach().
>
> Signed-off-by: Jason Gunthorpe 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/base/base.h| 1 -
>  drivers/base/dd.c  | 3 +++
>  include/linux/device.h | 2 ++
>  3 files changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Cornelia Huck 



[PATCH v2 2/2] drm/panel: Add support for E Ink VB3300-KCA

2021-06-15 Thread Alistair Francis
Add support for the 10.3" E Ink panel described at:
https://www.eink.com/product.html?type=productdetail=7

Signed-off-by: Alistair Francis 
---
v2:
 - Fix build warning
 - Document new string

 .../bindings/display/panel/panel-simple.yaml  |  2 ++
 drivers/gpu/drm/panel/panel-simple.c  | 29 +++
 2 files changed, 31 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index b3797ba2698b..799e20222551 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -128,6 +128,8 @@ properties:
 # Emerging Display Technology Corp. WVGA TFT Display with capacitive 
touch
   - edt,etm0700g0dh6
   - edt,etm0700g0edh6
+# E Ink VB3300-KCA
+  - eink,vb3300-kca
 # Evervision Electronics Co. Ltd. VGG804821 5.0" WVGA TFT LCD Panel
   - evervision,vgg804821
 # Foxlink Group 5" WVGA TFT LCD panel
diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index be312b5c04dd..600e054ec521 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2004,6 +2004,32 @@ static const struct panel_desc edt_etm0700g0bdh6 = {
.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
+static const struct display_timing eink_vb3300_kca_timing = {
+   .pixelclock = { 4000, 4000, 4000 },
+   .hactive = { 334, 334, 334 },
+   .hfront_porch = { 1, 1, 1 },
+   .hback_porch = { 1, 1, 1 },
+   .hsync_len = { 1, 1, 1 },
+   .vactive = { 1405, 1405, 1405 },
+   .vfront_porch = { 1, 1, 1 },
+   .vback_porch = { 1, 1, 1 },
+   .vsync_len = { 1, 1, 1 },
+   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE,
+};
+
+static const struct panel_desc eink_vb3300_kca = {
+   .timings = _vb3300_kca_timing,
+   .num_timings = 1,
+   .bpc = 6,
+   .size = {
+   .width = 157,
+   .height = 209,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+};
+
 static const struct display_timing evervision_vgg804821_timing = {
.pixelclock = { 2760, 3330, 5000 },
.hactive = { 800, 800, 800 },
@@ -4302,6 +4328,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "edt,etm0700g0dh6",
.data = _etm0700g0dh6,
+   }, {
+   .compatible = "eink,vb3300-kca",
+   .data = _vb3300_kca,
}, {
.compatible = "edt,etm0700g0bdh6",
.data = _etm0700g0bdh6,
-- 
2.31.1



[PATCH v2 1/2] dt-bindings: Add E Ink to vendor bindings

2021-06-15 Thread Alistair Francis
Add the E Ink Corporation to the vendor bindings.

Signed-off-by: Alistair Francis 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml 
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index abd878ff23d5..a35cec512530 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -335,6 +335,8 @@ patternProperties:
 description: eGalax_eMPIA Technology Inc
   "^einfochips,.*":
 description: Einfochips
+  "^eink,.*":
+description: E Ink Corporation
   "^elan,.*":
 description: Elan Microelectronic Corp.
   "^element14,.*":
-- 
2.31.1



Re: [PATCH 03/10] driver core: Flow the return code from ->probe() through to sysfs bind

2021-06-15 Thread Cornelia Huck
On Mon, Jun 14 2021, Christoph Hellwig  wrote:

> Currently really_probe() returns 1 on success and 0 if the probe() call
> fails. This return code arrangement is designed to be useful for
> __device_attach_driver() which is walking the device list and trying every
> driver. 0 means to keep trying.
>
> However, it is not useful for the other places that call through to
> really_probe() that do actually want to see the probe() return code.
>
> For instance bind_store() would be better to return the actual error code
> from the driver's probe method, not discarding it and returning -ENODEV.
>
> Reorganize things so that really_probe() returns the error code from
> ->probe as a (inverted) positive number, and 0 for successful attach.
>
> With this, __device_attach_driver can ignore the (positive) probe errors,
> return 1 to exit the loop for a successful binding and pass on the
> other negative errors, while device_driver_attach simplify inverts the
> positive errors and returns all errors to the sysfs code.
>
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/base/bus.c |  6 +-
>  drivers/base/dd.c  | 29 -
>  2 files changed, 21 insertions(+), 14 deletions(-)

Reviewed-by: Cornelia Huck 



Re: [PATCH 01/10] driver core: Pull required checks into driver_probe_device()

2021-06-15 Thread Cornelia Huck
On Mon, Jun 14 2021, Christoph Hellwig  wrote:

> From: Jason Gunthorpe 
>
> Checking if the dev is dead or if the dev is already bound is a required
> precondition to invoking driver_probe_device(). All the call chains
> leading here duplicate these checks.
>
> Add it directly to driver_probe_device() so the precondition is clear and
> remove the checks from device_driver_attach() and
> __driver_attach_async_helper().
>
> The other call chain going through __device_attach_driver() does have
> these same checks but they are inlined into logic higher up the call stack
> and can't be removed.
>
> The sysfs uAPI call chain starting at bind_store() is a bit confused
> because it reads dev->driver unlocked and returns -ENODEV if it is !NULL,
> otherwise it reads it again under lock and returns 0 if it is !NULL. Fix
> this to always return -EBUSY and always read dev->driver under its lock.
>
> Done in preparation for the next patches which will add additional
> callers to driver_probe_device() and will need these checks as well.
>
> Signed-off-by: Jason Gunthorpe 
> [hch: drop the extra checks in device_driver_attach and bind_store]
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/base/bus.c |  2 +-
>  drivers/base/dd.c  | 32 ++--
>  2 files changed, 11 insertions(+), 23 deletions(-)

Reviewed-by: Cornelia Huck 



[PATCH v3] drm/i915/ttm: accelerated move implementation

2021-06-15 Thread Ramalingam C
Invokes the pipelined page migration through blt, for
i915_ttm_move requests of eviction and also obj clear.

v2:
 - subfunction for accel_move (Thomas)
 - engine_pm_get/put around context_move/clear (Thomas)
 - Invalidation at accel_clear (Thomas)

v3:
 - Timeout is set for MAX_SCHEDULE_TIMEOUT (Thomas)
 - s/TTM_PL_PRIV/I915_PL_LMEM0 (Thomas)

Signed-off-by: Ramalingam C 
Reviewed-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 88 +
 1 file changed, 75 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 3748098b42d5..94571757fb42 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -15,6 +15,9 @@
 #include "gem/i915_gem_ttm.h"
 #include "gem/i915_gem_mman.h"
 
+#include "gt/intel_migrate.h"
+#include "gt/intel_engine_pm.h"
+
 #define I915_PL_LMEM0 TTM_PL_PRIV
 #define I915_PL_SYSTEM TTM_PL_SYSTEM
 #define I915_PL_STOLEN TTM_PL_VRAM
@@ -282,6 +285,62 @@ i915_ttm_resource_get_st(struct drm_i915_gem_object *obj,
return intel_region_ttm_node_to_st(obj->mm.region, res->mm_node);
 }
 
+static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
+  struct ttm_resource *dst_mem,
+  struct sg_table *dst_st)
+{
+   struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915),
+bdev);
+   struct ttm_resource_manager *src_man =
+   ttm_manager_type(bo->bdev, bo->mem.mem_type);
+   struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
+   struct sg_table *src_st;
+   struct i915_request *rq;
+   int ret;
+
+   if (!i915->gt.migrate.context)
+   return -EINVAL;
+
+   if (!bo->ttm || !ttm_tt_is_populated(bo->ttm)) {
+   if (bo->type == ttm_bo_type_kernel)
+   return -EINVAL;
+
+   if (bo->ttm &&
+   !(bo->ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC))
+   return 0;
+
+   intel_engine_pm_get(i915->gt.migrate.context->engine);
+   ret = intel_context_migrate_clear(i915->gt.migrate.context, 
NULL,
+ dst_st->sgl, I915_CACHE_NONE,
+ dst_mem->mem_type >= 
I915_PL_LMEM0,
+ 0, );
+
+   if (!ret && rq) {
+   i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
+   i915_request_put(rq);
+   }
+   intel_engine_pm_put(i915->gt.migrate.context->engine);
+   } else {
+   src_st = src_man->use_tt ? i915_ttm_tt_get_st(bo->ttm) :
+   obj->ttm.cached_io_st;
+
+   intel_engine_pm_get(i915->gt.migrate.context->engine);
+   ret = intel_context_migrate_copy(i915->gt.migrate.context,
+NULL, src_st->sgl, 
I915_CACHE_NONE,
+bo->mem.mem_type >= 
I915_PL_LMEM0,
+dst_st->sgl, I915_CACHE_NONE,
+dst_mem->mem_type >= 
I915_PL_LMEM0,
+);
+   if (!ret && rq) {
+   i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
+   i915_request_put(rq);
+   }
+   intel_engine_pm_put(i915->gt.migrate.context->engine);
+   }
+
+   return ret;
+}
+
 static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict,
 struct ttm_operation_ctx *ctx,
 struct ttm_resource *dst_mem,
@@ -332,19 +391,22 @@ static int i915_ttm_move(struct ttm_buffer_object *bo, 
bool evict,
if (IS_ERR(dst_st))
return PTR_ERR(dst_st);
 
-   /* If we start mapping GGTT, we can no longer use man::use_tt here. */
-   dst_iter = dst_man->use_tt ?
-   ttm_kmap_iter_tt_init(&_dst_iter.tt, bo->ttm) :
-   ttm_kmap_iter_iomap_init(&_dst_iter.io, _reg->iomap,
-dst_st, dst_reg->region.start);
-
-   src_iter = src_man->use_tt ?
-   ttm_kmap_iter_tt_init(&_src_iter.tt, bo->ttm) :
-   ttm_kmap_iter_iomap_init(&_src_iter.io, _reg->iomap,
-obj->ttm.cached_io_st,
-src_reg->region.start);
-
-   ttm_move_memcpy(bo, dst_mem->num_pages, dst_iter, src_iter);
+   ret = i915_ttm_accel_move(bo, dst_mem, dst_st);
+   if (ret) {
+   /* If we start mapping GGTT, we can no longer use man::use_tt 
here. */
+   dst_iter = dst_man->use_tt ?
+   

Re: [PATCH v6] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-06-15 Thread Daniel Vetter
On Thu, Jun 03, 2021 at 02:47:51PM -0700, Hridya Valsaraju wrote:
> Overview
> 
> The patch adds DMA-BUF statistics to /sys/kernel/dmabuf/buffers. It
> allows statistics to be enabled for each DMA-BUF in sysfs by enabling
> the config CONFIG_DMABUF_SYSFS_STATS.
> 
> The following stats will be exposed by the interface:
> 
> /sys/kernel/dmabuf/buffers//exporter_name
> /sys/kernel/dmabuf/buffers//size
> /sys/kernel/dmabuf/buffers//attachments//device
> /sys/kernel/dmabuf/buffers//attachments//map_counter
> 
> The inode_number is unique for each DMA-BUF and was added earlier [1]
> in order to allow userspace to track DMA-BUF usage across different
> processes.
> 
> Use Cases
> =
> The interface provides a way to gather DMA-BUF per-buffer statistics
> from production devices. These statistics will be used to derive DMA-BUF
> per-exporter stats and per-device usage stats for Android Bug reports.
> The corresponding userspace changes can be found at [2].
> Telemetry tools will also capture this information(along with other
> memory metrics) periodically as well as on important events like a
> foreground app kill (which might have been triggered by Low Memory
> Killer). It will also contribute to provide a snapshot of the system
> memory usage on other events such as OOM kills and Application Not
> Responding events.
> 
> Background
> ==
> Currently, there are two existing interfaces that provide information
> about DMA-BUFs.
> 1) /sys/kernel/debug/dma_buf/bufinfo
> debugfs is however unsuitable to be mounted in production systems and
> cannot be considered as an alternative to the sysfs interface being
> proposed.
> 2) proc//fdinfo/
> The proc//fdinfo/ files expose information about DMA-BUF fds.
> However, the existing procfs interfaces can only provide information
> about the buffers for which processes hold fds or have the buffers
> mmapped into their address space. Since the procfs interfaces alone
> cannot provide a full picture of all DMA-BUFs in the system, there is
> the need for an alternate interface to provide this information on
> production systems.
> 
> The patch contains the following major improvements over v1:
> 1) Each attachment is represented by its own directory to allow creating
> a symlink to the importing device and to also provide room for future
> expansion.
> 2) The number of distinct mappings of each attachment is exposed in a
> separate file.
> 3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
> inorder to make the interface expandable in future.
> 
> All of the improvements above are based on suggestions/feedback from
> Daniel Vetter and Christian König.
> 
> A shell script that can be run on a classic Linux environment to read
> out the DMA-BUF statistics can be found at [3](suggested by John
> Stultz).
> 
> [1]: https://lore.kernel.org/patchwork/patch/1088791/
> [2]: 
> https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
> [3]: 
> https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734
> 
> Reviewed-by: Greg Kroah-Hartman 
> Signed-off-by: Hridya Valsaraju 
> Reported-by: kernel test robot 
> ---
> 
> Hello Daniel,
> 
> I have added the documentation as a DOC: overview section in the
> dma-buf-sysfs-stats.c file as per your suggestion. Please do take a look
> when you get a chance. Thanks in advance!
> 
> Regards,
> Hridya
> 
> Change in v6:
> -Moved documentation content from Documentation/driver-api/dma-buf.rst
> to drivers/dma-buf/dma-buf-sysfs-stats.c as a DOC section and linked to
> it from Documentation/driver-api/dma-buf.rst. Based on feedback from
> Daniel Vetter.
> 
> Change in v5:
> -Added a section on DMA-BUF statistics to
> Documentation/driver-api/dma-buf.rst. Organized the commit message to
> clearly state the need for the new interface and provide the
> background on why the existing means of DMA-BUF accounting will not
> suffice. Based on feedback from Daniel Vetter.
> 
> Changes in v4:
> -Suppress uevents from kset creation to avoid waking up uevent listeners
> on DMA-BUF export/release.
> 
> Changes in v3:
> -Fix a warning reported by the kernel test robot.
> 
> Changes in v2:
> -Move statistics to /sys/kernel/dmabuf/buffers in oder to allow addition
> of other DMA-BUF-related sysfs stats in future. Based on feedback from
> Daniel Vetter.
> -Each attachment has its own directory to represent attached devices as
> symlinks and to introduce map_count as a separate file. Based on
> feedback from Daniel Vetter and Christian König. Thank you both!
> -Commit messages updated to point to userspace code in AOSP that will
> read the DMA-BUF sysfs stats.

Pushed to drm-misc-next, thanks for your work. I think the timing just
conspired that this might miss the next merge window though :-/
-Daniel

> 
>  .../ABI/testing/sysfs-kernel-dmabuf-buffers   |  52 +++
>  Documentation/driver-api/dma-buf.rst  |   5 +
>  drivers/dma-buf/Kconfig

[PATCH v4] drm/i915: Be more gentle with exiting non-persistent context

2021-06-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

When a non-persistent context exits we currently mark it as banned in
order to trigger fast termination of any outstanding GPU jobs it may have
left running.

In doing so we apply a very strict 1ms limit in which the left over job
has to preempt before we issues an engine resets.

Some workloads are not able to cleanly preempt in that time window and it
can be argued that it would instead be better to give them a bit more
grace since avoiding engine resets is generally preferrable.

To achieve this the patch splits handling of banned contexts from simply
exited non-persistent ones and then applies different timeouts for both
and also extends the criteria which determines if a request should be
scheduled back in after preemption or not.

15ms preempt timeout grace is given to exited non-persistent contexts
which have been empirically tested to satisfy customers requirements
and still provides reasonably quick cleanup post exit.

v2:
 * Streamline fast path checks.

v3:
 * Simplify by using only schedulable status.
 * Increase timeout to 20ms.

v4:
 * Fix live_execlists selftest.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Zhen Han 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 16 +--
 drivers/gpu/drm/i915/gt/intel_context.c   |  2 ++
 drivers/gpu/drm/i915/gt/intel_context.h   | 11 ++
 drivers/gpu/drm/i915/gt/intel_context_types.h |  1 +
 .../drm/i915/gt/intel_execlists_submission.c  | 11 --
 drivers/gpu/drm/i915/gt/selftest_execlists.c  | 20 ---
 drivers/gpu/drm/i915/i915_request.c   |  2 +-
 7 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7720b8c22c81..463d4aa9cf63 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -426,7 +426,8 @@ static struct intel_engine_cs *active_engine(struct 
intel_context *ce)
return engine;
 }
 
-static void kill_engines(struct i915_gem_engines *engines, bool ban)
+static void
+kill_engines(struct i915_gem_engines *engines, bool ban, bool persistent)
 {
struct i915_gem_engines_iter it;
struct intel_context *ce;
@@ -443,6 +444,9 @@ static void kill_engines(struct i915_gem_engines *engines, 
bool ban)
 
if (ban && intel_context_set_banned(ce))
continue;
+   else if (!persistent &&
+!intel_context_clear_schedulable(ce))
+   continue;
 
/*
 * Check the current active state of this context; if we
@@ -454,7 +458,7 @@ static void kill_engines(struct i915_gem_engines *engines, 
bool ban)
engine = active_engine(ce);
 
/* First attempt to gracefully cancel the context */
-   if (engine && !__cancel_engine(engine) && ban)
+   if (engine && !__cancel_engine(engine) && (ban || !persistent))
/*
 * If we are unable to send a preemptive pulse to bump
 * the context from the GPU, we have to resort to a full
@@ -466,8 +470,6 @@ static void kill_engines(struct i915_gem_engines *engines, 
bool ban)
 
 static void kill_context(struct i915_gem_context *ctx)
 {
-   bool ban = (!i915_gem_context_is_persistent(ctx) ||
-   !ctx->i915->params.enable_hangcheck);
struct i915_gem_engines *pos, *next;
 
spin_lock_irq(>stale.lock);
@@ -480,7 +482,8 @@ static void kill_context(struct i915_gem_context *ctx)
 
spin_unlock_irq(>stale.lock);
 
-   kill_engines(pos, ban);
+   kill_engines(pos, !ctx->i915->params.enable_hangcheck,
+i915_gem_context_is_persistent(ctx));
 
spin_lock_irq(>stale.lock);
GEM_BUG_ON(i915_sw_fence_signaled(>fence));
@@ -526,7 +529,8 @@ static void engines_idle_release(struct i915_gem_context 
*ctx,
 
 kill:
if (list_empty(>link)) /* raced, already closed */
-   kill_engines(engines, true);
+   kill_engines(engines, true,
+i915_gem_context_is_persistent(ctx));
 
i915_sw_fence_commit(>fence);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 4033184f13b9..9d539f48d7c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -373,6 +373,8 @@ intel_context_init(struct intel_context *ce, struct 
intel_engine_cs *engine)
ce->sseu = engine->sseu;
ce->ring = __intel_context_ring_size(SZ_4K);
 
+   __set_bit(CONTEXT_SCHEDULABLE, >flags);
+
ewma_runtime_init(>runtime.avg);
 
ce->vm = i915_vm_get(engine->gt->vm);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h 
b/drivers/gpu/drm/i915/gt/intel_context.h
index 

Re: [PATCH v3] Documentation: gpu: Mention the requirements for new properties

2021-06-15 Thread Laurent Pinchart
On Tue, Jun 15, 2021 at 07:15:18AM +, Simon Ser wrote:
> On Tuesday, June 15th, 2021 at 09:03, Pekka Paalanen  
> wrote:
> 
> > indeed it will, but what else could one do to test userspace KMS
> > clients in generic CI where all you can have is virtual hardware? Maybe
> > in the long run VKMS needs to loop back to a userspace daemon that
> > implements all the complex processing and returns the writeback result
> > via VKMS again? That daemon would then need a single upstream, like the
> > kernel, where it is maintained and correctness verified.
> 
> The complex processing must be implemented even without write-back, because
> user-space can ask for CRCs of the CRTC.
> 
> > Or an LD_PRELOAD that hijacks all KMS ioctls and implements virtual
> > stuff in userspace? Didn't someone already have something like that?
> > It would need to be lifted to be a required part of kernel UAPI
> > submissions, I suppose like IGT is nowadays.
> 
> FWIW, I have a mock libdrm [1] for libliftoff. This is nowhere near a full
> software implementation with write-back connectors, but allows to expose
> virtual planes and check atomic commits in CI.
> 
> [1]: https://github.com/emersion/libliftoff/blob/master/test/libdrm_mock.c
> 
> > For compositor developers like me knowing the exact formulas would be a huge
> > benefit as it would allow me to use KMS to off-load precision-sensitive
> > operations (e.g.  professional color management). Otherwise, compositors
> > probably need a switch: "high quality color management? Then do not use KMS
> > features."
> 
> I think for alpha blending there are already rounding issues depending on the
> hardware. I wouldn't keep my hopes up for any guarantee that all hw uses the
> exact same formulae for color management stuff.

Good, because otherwise you would be very quickly disappointed :-)

For scaling we would also need to replicate the exact same filter taps,
which are often not documented.

-- 
Regards,

Laurent Pinchart


Re: [PATCH v2 -next] drm/hyperv: Fix unused const variable 'hyperv_modifiers'

2021-06-15 Thread Thomas Zimmermann

Hi

Am 15.06.21 um 05:14 schrieb Pu Lehui:

There is a gcc '-Wunused-const-variable' warning:
   drivers/gpu/drm/hyperv/hyperv_drm_modeset.c:152:23: warning:
 'hyperv_modifiers' defined but not used [-Wunused-const-variable=]

while the variable should be used in drm_simple_display_pipe_init()
as suggested by Thomas, let's fix it.


Thanks a lot! I added your patch to drm-misc-next-fixes.

Best regards
Thomas



Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video 
device")
Signed-off-by: Pu Lehui 
---
  drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c 
b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 02718e3e859e..3aaee4730ec6 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -163,7 +163,7 @@ static inline int hyperv_pipe_init(struct hyperv_drm_device 
*hv)
   _pipe_funcs,
   hyperv_formats,
   ARRAY_SIZE(hyperv_formats),
-  NULL,
+  hyperv_modifiers,
   >connector);
if (ret)
return 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



OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH 2/2] drm/gud: Use scatter-gather USB bulk transfer

2021-06-15 Thread Peter Stuge
Hi Noralf,

Noralf Trønnes wrote:
> >> +static int gud_usb_bulk(struct gud_device *gdrm, size_t len)
..
> >> +   timer_setup_on_stack(, gud_usb_bulk_timeout, 0);
> >> +   mod_timer(, jiffies + msecs_to_jiffies(3000));
> >> +
> >> +   usb_sg_wait();
> >> +
> >> +   if (!del_timer_sync())
> >> +   ret = -ETIMEDOUT;
..
> > Mention in the commit message that sending USB bulk transfers with
> > an sglist could be unstable

Can you explain a bit about /how/ it is unstable?

As you write, usb_bulk_msg() (as used before) has a timeout which is
passed to the host controller hardware and implemented there.

I haven't used SG with kernel USB but I would expect such a timeout
to still be available with SG?


> usb_bulk_msg() has builtin timeout handling and during development of
> a microcontroller gadget implementation I've triggered this timeout
> several times when the uC usb interrupts stopped firing.

The device not responding to bulk packets scheduled and sent by the host
is a real error /in the device/ and thus not neccessarily something the
kernel must handle gracefully.. I think it's quite nice to do so, but
one can argue that it's not strictly required.

But more importantly: Remember that bulk transfer has no delivery time
guarantee. It can take indefinitely long until a bulk transfer is
scheduled by the host on a busy bus which is starved with more
important things (control, interrupt, iso transfers) - that's not
an error at all, and may be indistinguishable from the device not
responding to packets actually sent by the host.

Having a timeout is important, I just expect the USB SG interface to
support it since it is the hardware that times out in the non-SG case.


And since this is essentially real time data maybe a shorter timeout
is better? 3 seconds seems really long.

The timeout must include all latency for a frame, so e.g. 16ms (60 Hz)
is too short for sure. But maybe something like 500ms?


//Peter


Re: [PATCH 2/3] drm/i915/guc: Update firmware to v62.0.0

2021-06-15 Thread Michal Wajdeczko



On 14.06.2021 21:42, Matthew Brost wrote:
> From: Michal Wajdeczko 
> 
> Most of the changes to the 62.0.0 firmware revolved around CTB
> communication channel. Conform to the new (stable) CTB protocol.
> 
> Signed-off-by: John Harrison 
> Signed-off-by: Michal Wajdeczko 
> Signed-off-by: Matthew Brost 
> ---
>  .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  | 107 ++
>  .../gt/uc/abi/guc_communication_ctb_abi.h | 120 --
>  .../gt/uc/abi/guc_communication_mmio_abi.h|  65 ++--
>  drivers/gpu/drm/i915/gt/uc/intel_guc.c| 107 --
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c|  45 +--
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 356 +-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |   6 +-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  75 +---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_log.c|  29 +-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_log.h|   6 +-
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  |  26 +-
>  11 files changed, 521 insertions(+), 421 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index 90efef8a73e4..dfaea0b54370 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -6,6 +6,113 @@
>  #ifndef _ABI_GUC_ACTIONS_ABI_H
>  #define _ABI_GUC_ACTIONS_ABI_H
>  
> +/**
> + * DOC: HOST2GUC_REGISTER_CTB
> + *
> + * This message is used as part of the `CTB based communication`_ setup.
> + *
> + * This message must be sent as `MMIO HXG Message`_.
> + *
> + *  
> +---+---+--+
> + *  |   | Bits  | Description
>   |
> + *  
> +===+===+==+
> + *  | 0 |31 | ORIGIN = GUC_HXG_ORIGIN_HOST_  
>   |
> + *  |   
> +---+--+
> + *  |   | 30:28 | TYPE = GUC_HXG_TYPE_REQUEST_   
>   |
> + *  |   
> +---+--+
> + *  |   | 27:16 | DATA0 = MBZ
>   |
> + *  |   
> +---+--+
> + *  |   |  15:0 | ACTION = _`GUC_ACTION_HOST2GUC_REGISTER_CTB`   
>   |
> + *  
> +---+---+--+

with removal of the action code number from the documentation we lost
the possibility of searching for this number in case someone hits any of
dmesg that was referring to it that like following:

"mmio request %#x: no reply %x\n",
"mmio request %#x: failure %x/%u\n",
"Sending action %#x failed (err=%d status=%#X)\n",

what's the actual benefit in removal if they were already in the table?

> + *  | 1 | 31:12 | RESERVED = MBZ 
>   |
> + *  |   
> +---+--+
> + *  |   |  11:8 | **TYPE** - type for the `CT Buffer`_   
>   |
> + *  |   |   |
>   |
> + *  |   |   |   - _`GUC_CTB_TYPE_HOST2GUC` = 0   
>   |
> + *  |   |   |   - _`GUC_CTB_TYPE_GUC2HOST` = 1   
>   |
> + *  |   
> +---+--+

note that params are still well documented

> + *  |   |   7:0 | **SIZE** - size of the `CT Buffer`_ in 4K units minus 1
>   |
> + *  
> +---+---+--+
> + *  | 2 |  31:0 | **DESC_ADDR** - GGTT address of the `CTB Descriptor`_  
>   |
> + *  
> +---+---+--+
> + *  | 3 |  31:0 | **BUFF_ADDF** - GGTT address of the `CT Buffer`_   
>   |
> + *  
> +---+---+--+
> + *
> + *  
> +---+---+--+
> + *  |   | Bits  | Description
>   |
> + *  
> +===+===+==+
> + *  | 0 |31 | ORIGIN = GUC_HXG_ORIGIN_GUC_   
>   |
> + *  |   
> +---+--+
> + *  |   | 30:28 | TYPE = GUC_HXG_TYPE_RESPONSE_SUCCESS_  
>   |
> + *  |   
> +---+--+
> + *  |   |  27:0 | DATA0 = MBZ
>   |
> + *  
> +---+---+--+
> + */
> +#define GUC_ACTION_HOST2GUC_REGISTER_CTB

Re: [PATCH 2/2] drm/gud: Use scatter-gather USB bulk transfer

2021-06-15 Thread Noralf Trønnes



Den 14.06.2021 22.54, skrev Linus Walleij:
> Hi Noralf,
> 
> On Mon, Mar 29, 2021 at 8:01 PM Noralf Trønnes  wrote:
> 
>> There'a limit to how big a kmalloc buffer can be, and as memory gets
>> fragmented it becomes more difficult to get big buffers. The downside of
>> smaller buffers is that the driver has to split the transfer up which
>> hampers performance. Compression might also take a hit because of the
>> splitting.
>>
>> Solve this by allocating the transfer buffer using vmalloc and create a
>> SG table to be passed on to the USB subsystem. vmalloc_32() is used to
>> avoid DMA bounce buffers on USB controllers that can only access 32-bit
>> addresses.
>>
>> This also solves the problem that split transfers can give host side
>> tearing since flushing is decoupled from rendering.
>>
>> Signed-off-by: Noralf Trønnes 
> 
>> +   num_pages = PAGE_ALIGN(gdrm->bulk_len) >> PAGE_SHIFT;
> 
> Isn't it the same to write:
> 
> num_pages = round_up(gdrm->bulk_len, PAGE_SIZE)?
> 
> Slightly easier to read IMO.
> 

Yes it's the same, I just copied this from elsewhere in the kernel where
a vmalloc buffer is turned into an sg list. I can change that.

>> +   if (max_buffer_size > SZ_64M)
>> +   max_buffer_size = SZ_64M; /* safeguard */
> 
> Explain this choice of max buffer in the commit message
> or as a comment please because I don't get why this size
> is the roof.
> 
>> +struct gud_usb_bulk_context {
>> +   struct timer_list timer;
>> +   struct usb_sg_request sgr;
>> +};
>> +
>> +static void gud_usb_bulk_timeout(struct timer_list *t)
>> +{
>> +   struct gud_usb_bulk_context *timer = from_timer(timer, t, timer);
>> +
>> +   usb_sg_cancel(>sgr);
> 
> Error message here? "Timeout on sg bulk transfer".
> 

A timeout is detected in gud_usb_bulk() which will return -ETIMEDOUT if
the timer did fire. gud_flush_work() will print an error message.

>> +}
>> +
>> +static int gud_usb_bulk(struct gud_device *gdrm, size_t len)
>> +{
>> +   struct gud_usb_bulk_context ctx;
>> +   int ret;
>> +
>> +   ret = usb_sg_init(, gud_to_usb_device(gdrm), 
>> gdrm->bulk_pipe, 0,
>> + gdrm->bulk_sgt.sgl, gdrm->bulk_sgt.nents, len, 
>> GFP_KERNEL);
>> +   if (ret)
>> +   return ret;
>> +
>> +   timer_setup_on_stack(, gud_usb_bulk_timeout, 0);
>> +   mod_timer(, jiffies + msecs_to_jiffies(3000));
>> +
>> +   usb_sg_wait();
>> +
>> +   if (!del_timer_sync())
>> +   ret = -ETIMEDOUT;
>> +   else if (ctx.sgr.status < 0)
>> +   ret = ctx.sgr.status;
>> +   else if (ctx.sgr.bytes != len)
>> +   ret = -EIO;
>> +
>> +   destroy_timer_on_stack();
>> +
>> +   return ret;
>> +}
> 
> Mention in the commit message that sending USB bulk transfers with
> an sglist could be unstable so you set up a timeout around
> usb_sg_wait() (did this happen to you? then write that)
> 
> The other users of usb_sg_wait() in the kernel do not have these
> timeout wrappers, I suspect the reasoning is something like
> "it's graphics, not storage, so if we timeout and lose an update,
> too bad but let's just continue hoping the lost graphics will be
> less than noticeable" so then we should write that as a comment
> about that in the code or something.
> 

There are 5 users of usb_sg_wait() in the kernel:
drivers/input/touchscreen/sur40.c
drivers/misc/cardreader/rtsx_usb.c
drivers/mmc/host/vub300.c
drivers/usb/misc/usbtest.c
drivers/usb/storage/transport.c

3 of those wrap it in a timer:
drivers/misc/cardreader/rtsx_usb.c: rtsx_usb_bulk_transfer_sglist()
drivers/mmc/host/vub300.c: __command_write_data()
drivers/usb/misc/usbtest.c: perform_sglist()

And it looks to me like usb/storage has some timeout handling through
the scsi layer:
/drivers/usb/storage/scsiglue.c: command_abort() ->
usb_stor_stop_transport() -> usb_sg_cancel()

This leaves 1 out of 5 users without timeout handling?

usb_bulk_msg() has builtin timeout handling and during development of a
microcontroller gadget implementation I've triggered this timeout
several times when the uC usb interrupts stopped firing.

I can add a comment in the commit message about the timer.

Noralf.

> With these comments fixed up:
> Reviewed-by: Linus Walleij 
> 
> Yours,
> Linus Walleij
> 


Re: [Mesa-dev] [PATCH 0/6] dma-buf: Add an API for exporting sync files (v12)

2021-06-15 Thread Christian König

Hi Jason & Daniel,

maybe I should explain once more where the problem with this approach is 
and why I think we need to get that fixed before we can do something 
like this here.


To summarize what this patch here does is that it copies the exclusive 
fence and/or the shared fences into a sync_file. This alone is totally 
unproblematic.


The problem is what this implies. When you need to copy the exclusive 
fence to a sync_file then this means that the driver is at some point 
ignoring the exclusive fence on a buffer object.


When you combine that with complex drivers which use TTM and buffer 
moves underneath you can construct an information leak using this and 
give userspace access to memory which is allocated to the driver, but 
not yet initialized.


This way you can leak things like page tables, passwords, kernel data 
etc... in large amounts to userspace and is an absolutely no-go for 
security.


That's why I'm said we need to get this fixed before we upstream this 
patch set here and especially the driver change which is using that.


Regards,
Christian.

Am 10.06.21 um 23:09 schrieb Jason Ekstrand:

Modern userspace APIs like Vulkan are built on an explicit
synchronization model.  This doesn't always play nicely with the
implicit synchronization used in the kernel and assumed by X11 and
Wayland.  The client -> compositor half of the synchronization isn't too
bad, at least on intel, because we can control whether or not i915
synchronizes on the buffer and whether or not it's considered written.

The harder part is the compositor -> client synchronization when we get
the buffer back from the compositor.  We're required to be able to
provide the client with a VkSemaphore and VkFence representing the point
in time where the window system (compositor and/or display) finished
using the buffer.  With current APIs, it's very hard to do this in such
a way that we don't get confused by the Vulkan driver's access of the
buffer.  In particular, once we tell the kernel that we're rendering to
the buffer again, any CPU waits on the buffer or GPU dependencies will
wait on some of the client rendering and not just the compositor.

This new IOCTL solves this problem by allowing us to get a snapshot of
the implicit synchronization state of a given dma-buf in the form of a
sync file.  It's effectively the same as a poll() or I915_GEM_WAIT only,
instead of CPU waiting directly, it encapsulates the wait operation, at
the current moment in time, in a sync_file so we can check/wait on it
later.  As long as the Vulkan driver does the sync_file export from the
dma-buf before we re-introduce it for rendering, it will only contain
fences from the compositor or display.  This allows to accurately turn
it into a VkFence or VkSemaphore without any over- synchronization.

This patch series actually contains two new ioctls.  There is the export
one mentioned above as well as an RFC for an import ioctl which provides
the other half.  The intention is to land the export ioctl since it seems
like there's no real disagreement on that one.  The import ioctl, however,
has a lot of debate around it so it's intended to be RFC-only for now.

Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037
IGT tests: https://patchwork.freedesktop.org/series/90490/

v10 (Jason Ekstrand, Daniel Vetter):
  - Add reviews/acks
  - Add a patch to rename _rcu to _unlocked
  - Split things better so import is clearly RFC status

v11 (Daniel Vetter):
  - Add more CCs to try and get maintainers
  - Add a patch to document DMA_BUF_IOCTL_SYNC
  - Generally better docs
  - Use separate structs for import/export (easier to document)
  - Fix an issue in the import patch

v12 (Daniel Vetter):
  - Better docs for DMA_BUF_IOCTL_SYNC

v12 (Christian König):
  - Drop the rename patch in favor of Christian's series
  - Add a comment to the commit message for the dma-buf sync_file export
ioctl saying why we made it an ioctl on dma-buf

Cc: Christian König 
Cc: Michel Dänzer 
Cc: Dave Airlie 
Cc: Bas Nieuwenhuizen 
Cc: Daniel Stone 
Cc: mesa-...@lists.freedesktop.org
Cc: wayland-de...@lists.freedesktop.org
Test-with: 20210524205225.872316-1-ja...@jlekstrand.net

Christian König (1):
   dma-buf: Add dma_fence_array_for_each (v2)

Jason Ekstrand (5):
   dma-buf: Add dma_resv_get_singleton (v6)
   dma-buf: Document DMA_BUF_IOCTL_SYNC (v2)
   dma-buf: Add an API for exporting sync files (v12)
   RFC: dma-buf: Add an extra fence to dma_resv_get_singleton_unlocked
   RFC: dma-buf: Add an API for importing sync files (v7)

  Documentation/driver-api/dma-buf.rst |   8 ++
  drivers/dma-buf/dma-buf.c| 103 +
  drivers/dma-buf/dma-fence-array.c|  27 +++
  drivers/dma-buf/dma-resv.c   | 110 +++
  include/linux/dma-fence-array.h  |  17 +
  include/linux/dma-resv.h |   2 +
  include/uapi/linux/dma-buf.h | 103 -
  7 files changed, 

Re: [PATCH 0/8] drm + usb-type-c: Add support for out-of-band hotplug notification (v4)

2021-06-15 Thread Hans de Goede
Hi,

On 6/15/21 9:40 AM, Greg Kroah-Hartman wrote:
> On Fri, Jun 04, 2021 at 09:48:32PM +0200, Hans de Goede wrote:
>> Here is v3 of my patchset making DP over Type-C work on devices where the
>> Type-C controller does not drive the HPD pin on the GPU, but instead
>> we need to forward HPD events from the Type-C controller to the DRM driver.
>>
>> Changes in v4:
>> - Rebase on top of latest drm-tip
>> - Add forward declaration for struct fwnode_handle to drm_crtc_internal.h
>>   (fixes warning reported by kernel test robot )
>> - Add Heikki's Reviewed-by to patch 7 & 8
>> - Add Heikki's Tested-by to the series
>>
>> Changes in v3:
>> - Base on top of latest drm-tip, which should fix the CI being unable to
>>   apply (and thus to test) the patches
>> - Make intel_acpi_assign_connector_fwnodes() take a ref on the fwnode
>>   it stores in connector->fwnode and have drm_connector_cleanup() put
>>   this reference
>> - Drop data argument from drm_connector_oob_hotplug_event()
>> - Make the Type-C DP altmode code only call drm_connector_oob_hotplug_event()
>>   when the HPD bit in the status vdo changes
>> - Drop the platform/x86/intel_cht_int33fe: Correct "displayport" fwnode
>>   reference patch, this will be merged independently through the pdx86 tree
>>
>> Changes in v2:
>> - Replace the bogus "drm/connector: Make the drm_sysfs connector->kdev
>>   device hold a reference to the connector" patch with:
>>   "drm/connector: Give connector sysfs devices there own device_type"
>>   the new patch is a dep for patch 2/9 see the patches
>>
>> - Stop using a class-dev-iter, instead at a global connector list
>>   to drm_connector.c and use that to find the connector by the fwnode,
>>   similar to how we already do this in drm_panel.c and drm_bridge.c
>>
>> - Make drm_connector_oob_hotplug_event() take a fwnode pointer as
>>   argument, rather then a drm_connector pointer and let it do the
>>   lookup itself. This allows making drm_connector_find_by_fwnode() a
>>   drm-internal function and avoids code outside the drm subsystem
>>   potentially holding on the a drm_connector reference for a longer
>>   period.
>>
>> This series not only touches drm subsys files but it also touches
>> drivers/usb/typec/altmodes/typec_displayport.c, that file usually
>> does not see a whole lot of changes. So I believe it would be best
>> to just merge the entire series through drm-misc, Assuming we can
>> get an ack from Greg for merging the typec_displayport.c changes
>> this way.
> 
> No objection from me, I've replied with reviewed-by for those USB
> patches now.

Great, thank you.

drm-devs, can I get an ack / reviewed-by for the rest of the series
so that I can push this to drm-misc-next ?

Regards,

Hans



Re: [RFC PATCH 2/2] drm/etnaviv: add clock gating workaround for GC7000 r6202

2021-06-15 Thread Lucas Stach
Hi Michael,

Am Dienstag, dem 15.06.2021 um 00:17 +0200 schrieb Michael Walle:
> The LS1028A SoC errata sheet mentions A-050121 "GPU hangs if clock
> gating for Rasterizer, Setup Engine and Texture Engine are enabled".
> The workaround is to disable the corresponding clock gatings.
> 
> Signed-off-by: Michael Walle 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 4102bcea3341..574e4e04dddc 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -613,6 +613,12 @@ static void etnaviv_gpu_enable_mlcg(struct etnaviv_gpu 
> *gpu)
>   etnaviv_is_model_rev(gpu, GC2000, 0x5108))
>   pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_TX;
>  
> + /* Disable RS, SE, TE clock gating on affected core revisions. */

This comment is wrong. RS (resolver) is a different engine than RA
(rasterizer) and the texture engine is abbreviated TX throughout the
driver.

Regards,
Lucas

> + if (etnaviv_is_model_rev(gpu, GC7000, 0x6202))
> + pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SE |
> +VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA |
> +VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_TX;
> +
>   pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_HZ;
>   pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_EZ;
>  




Re: [PATCH 0/8] drm + usb-type-c: Add support for out-of-band hotplug notification (v4)

2021-06-15 Thread Greg Kroah-Hartman
On Fri, Jun 04, 2021 at 09:48:32PM +0200, Hans de Goede wrote:
> Here is v3 of my patchset making DP over Type-C work on devices where the
> Type-C controller does not drive the HPD pin on the GPU, but instead
> we need to forward HPD events from the Type-C controller to the DRM driver.
> 
> Changes in v4:
> - Rebase on top of latest drm-tip
> - Add forward declaration for struct fwnode_handle to drm_crtc_internal.h
>   (fixes warning reported by kernel test robot )
> - Add Heikki's Reviewed-by to patch 7 & 8
> - Add Heikki's Tested-by to the series
> 
> Changes in v3:
> - Base on top of latest drm-tip, which should fix the CI being unable to
>   apply (and thus to test) the patches
> - Make intel_acpi_assign_connector_fwnodes() take a ref on the fwnode
>   it stores in connector->fwnode and have drm_connector_cleanup() put
>   this reference
> - Drop data argument from drm_connector_oob_hotplug_event()
> - Make the Type-C DP altmode code only call drm_connector_oob_hotplug_event()
>   when the HPD bit in the status vdo changes
> - Drop the platform/x86/intel_cht_int33fe: Correct "displayport" fwnode
>   reference patch, this will be merged independently through the pdx86 tree
> 
> Changes in v2:
> - Replace the bogus "drm/connector: Make the drm_sysfs connector->kdev
>   device hold a reference to the connector" patch with:
>   "drm/connector: Give connector sysfs devices there own device_type"
>   the new patch is a dep for patch 2/9 see the patches
> 
> - Stop using a class-dev-iter, instead at a global connector list
>   to drm_connector.c and use that to find the connector by the fwnode,
>   similar to how we already do this in drm_panel.c and drm_bridge.c
> 
> - Make drm_connector_oob_hotplug_event() take a fwnode pointer as
>   argument, rather then a drm_connector pointer and let it do the
>   lookup itself. This allows making drm_connector_find_by_fwnode() a
>   drm-internal function and avoids code outside the drm subsystem
>   potentially holding on the a drm_connector reference for a longer
>   period.
> 
> This series not only touches drm subsys files but it also touches
> drivers/usb/typec/altmodes/typec_displayport.c, that file usually
> does not see a whole lot of changes. So I believe it would be best
> to just merge the entire series through drm-misc, Assuming we can
> get an ack from Greg for merging the typec_displayport.c changes
> this way.

No objection from me, I've replied with reviewed-by for those USB
patches now.

thanks,
greg k-h


Re: [PATCH 8/8] usb: typec: altmodes/displayport: Notify drm subsys of hotplug events

2021-06-15 Thread Greg Kroah-Hartman
On Fri, Jun 04, 2021 at 09:48:40PM +0200, Hans de Goede wrote:
> Use the new drm_connector_oob_hotplug_event() functions to let drm/kms
> drivers know about DisplayPort over Type-C hotplug events.
> 
> Reviewed-by: Heikki Krogerus 
> Tested-by: Heikki Krogerus 
> Signed-off-by: Hans de Goede 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH 7/8] usb: typec: altmodes/displayport: Make dp_altmode_notify() more generic

2021-06-15 Thread Greg Kroah-Hartman
On Fri, Jun 04, 2021 at 09:48:39PM +0200, Hans de Goede wrote:
> Make dp_altmode_notify() handle the dp->data.conf == 0 case too,
> rather then having separate code-paths for this in various places
> which call it.
> 
> Reviewed-by: Heikki Krogerus 
> Tested-by: Heikki Krogerus 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/usb/typec/altmodes/displayport.c | 35 +---
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 

Reviewed-by: Greg Kroah-Hartman 


Re: [PATCH v3] Documentation: gpu: Mention the requirements for new properties

2021-06-15 Thread Simon Ser
On Tuesday, June 15th, 2021 at 09:03, Pekka Paalanen  
wrote:

> indeed it will, but what else could one do to test userspace KMS
> clients in generic CI where all you can have is virtual hardware? Maybe
> in the long run VKMS needs to loop back to a userspace daemon that
> implements all the complex processing and returns the writeback result
> via VKMS again? That daemon would then need a single upstream, like the
> kernel, where it is maintained and correctness verified.

The complex processing must be implemented even without write-back, because
user-space can ask for CRCs of the CRTC.

> Or an LD_PRELOAD that hijacks all KMS ioctls and implements virtual
> stuff in userspace? Didn't someone already have something like that?
> It would need to be lifted to be a required part of kernel UAPI
> submissions, I suppose like IGT is nowadays.

FWIW, I have a mock libdrm [1] for libliftoff. This is nowhere near a full
software implementation with write-back connectors, but allows to expose
virtual planes and check atomic commits in CI.

[1]: https://github.com/emersion/libliftoff/blob/master/test/libdrm_mock.c

> For compositor developers like me knowing the exact formulas would be a huge
> benefit as it would allow me to use KMS to off-load precision-sensitive
> operations (e.g.  professional color management). Otherwise, compositors
> probably need a switch: "high quality color management? Then do not use KMS
> features."

I think for alpha blending there are already rounding issues depending on the
hardware. I wouldn't keep my hopes up for any guarantee that all hw uses the
exact same formulae for color management stuff.


Re: [PATCH 3/6] dma-buf: Document DMA_BUF_IOCTL_SYNC (v2)

2021-06-15 Thread Pekka Paalanen
On Thu, 10 Jun 2021 16:14:42 -0500
Jason Ekstrand  wrote:

> This adds a new "DMA Buffer ioctls" section to the dma-buf docs and adds
> documentation for DMA_BUF_IOCTL_SYNC.
> 
> v2 (Daniel Vetter):
>  - Fix a couple typos
>  - Add commentary about synchronization with other devices
>  - Use item list format for describing flags
> 
> Signed-off-by: Jason Ekstrand 
> Reviewed-by: Daniel Vetter 
> Cc: Christian König 
> Cc: Sumit Semwal 
> ---
>  Documentation/driver-api/dma-buf.rst |  8 +
>  include/uapi/linux/dma-buf.h | 46 +++-
>  2 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/driver-api/dma-buf.rst 
> b/Documentation/driver-api/dma-buf.rst
> index 7f21425d9435a..0d4c13ec1a800 100644
> --- a/Documentation/driver-api/dma-buf.rst
> +++ b/Documentation/driver-api/dma-buf.rst
> @@ -88,6 +88,9 @@ consider though:
>  - The DMA buffer FD is also pollable, see `Implicit Fence Poll Support`_ 
> below for
>details.
>  
> +- The DMA buffer FD also supports a few dma-buf-specific ioctls, see
> +  `DMA Buffer ioctls`_ below for details.
> +
>  Basic Operation and Device DMA Access
>  ~
>  
> @@ -106,6 +109,11 @@ Implicit Fence Poll Support
>  .. kernel-doc:: drivers/dma-buf/dma-buf.c
> :doc: implicit fence polling
>  
> +DMA Buffer ioctls
> +~
> +
> +.. kernel-doc:: include/uapi/linux/dma-buf.h
> +
>  Kernel Functions and Structures Reference
>  ~
>  
> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
> index 7f30393b92c3b..1c131002fe1ee 100644
> --- a/include/uapi/linux/dma-buf.h
> +++ b/include/uapi/linux/dma-buf.h
> @@ -22,8 +22,52 @@
>  
>  #include 
>  
> -/* begin/end dma-buf functions used for userspace mmap. */
> +/**
> + * struct dma_buf_sync - Synchronize with CPU access.
> + *
> + * When a DMA buffer is accessed from the CPU via mmap, it is not always
> + * possible to guarantee coherency between the CPU-visible map and underlying
> + * memory.  To manage coherency, DMA_BUF_IOCTL_SYNC must be used to bracket
> + * any CPU access to give the kernel the chance to shuffle memory around if
> + * needed.
> + *
> + * Prior to accessing the map, the client must call DMA_BUF_IOCTL_SYNC
> + * with DMA_BUF_SYNC_START and the appropriate read/write flags.  Once the
> + * access is complete, the client should call DMA_BUF_IOCTL_SYNC with
> + * DMA_BUF_SYNC_END and the same read/write flags.
> + *
> + * The synchronization provided via DMA_BUF_IOCTL_SYNC only provides cache
> + * coherency.  It does not prevent other processes or devices from
> + * accessing the memory at the same time.  If synchronization with a GPU or
> + * other device driver is required, it is the client's responsibility to
> + * wait for buffer to be ready for reading or writing.

... before calling this ioctl.

Maybe that would be worthwhile to add?

Likewise, submit follow-up work to GPU et al. only after calling this
ioctl with SYNC_END?

Anyway, looks nice to me.

Acked-by: Pekka Paalanen 


Thanks,
pq

>  If the driver or
> + * API with which the client is interacting uses implicit synchronization,
> + * this can be done via poll() on the DMA buffer file descriptor.  If the
> + * driver or API requires explicit synchronization, the client may have to
> + * wait on a sync_file or other synchronization primitive outside the scope
> + * of the DMA buffer API.
> + */
>  struct dma_buf_sync {
> + /**
> +  * @flags: Set of access flags
> +  *
> +  * DMA_BUF_SYNC_START:
> +  * Indicates the start of a map access session.
> +  *
> +  * DMA_BUF_SYNC_END:
> +  * Indicates the end of a map access session.
> +  *
> +  * DMA_BUF_SYNC_READ:
> +  * Indicates that the mapped DMA buffer will be read by the
> +  * client via the CPU map.
> +  *
> +  * DMA_BUF_SYNC_WRITE:
> +  * Indicates that the mapped DMA buffer will be written by the
> +  * client via the CPU map.
> +  *
> +  * DMA_BUF_SYNC_RW:
> +  * An alias for DMA_BUF_SYNC_READ | DMA_BUF_SYNC_WRITE.
> +  */
>   __u64 flags;
>  };
>  



pgpIN9d_C3GPN.pgp
Description: OpenPGP digital signature


Re: [PATCH] modified: gpu/drm/panfrost/panfrost_gpu.c

2021-06-15 Thread Chunyou Tang
Hi steve,
After I send the V2,I found I setting a wrong email 
configuration,I hope it doesn't affect the patch submission :)
Did you received my another patch about panfrost_job.c?


Author: tangchunyou 
Date:   Wed Jun 9 14:44:52 2021 +0800

modified: gpu/drm/panfrost/panfrost_job.c

The 'break' can cause 'Memory manager not clean during takedown'
It cannot use break to finish the circulation,it should use
continue to traverse the circulation.

Signed-off-by: tangchunyou 

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 6003cfeb1322..52bccc1d2d42 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -281,7 +281,7 @@ static void panfrost_job_cleanup(struct kref *ref)
if (job->mappings) {
for (i = 0; i < job->bo_count; i++) {
if (!job->mappings[i])
-   break;
+   continue;

atomic_dec(>mappings[i]->obj->gpu_usecount);
panfrost_gem_mapping_put(job->mappings[i]);


Thank you!



?? Fri, 11 Jun 2021 11:10:16 +0100
Steven Price  :

> On 10/06/2021 14:06, Chunyou Tang wrote:
> > Hi Steven,
> 
> Hi Chunyou,
> 
> For some reason I'm not directly receiving your emails (only via the
> list) - can you double check your email configuration?
> 
> >>> The GPU exception fault status register(0x3C),the low 8 bit is the
> >>> EXCEPTION_TYPE.We can see the description at P3-78 in spec.
> > 
> > You can see the spec
> > .
> 
> Thanks - please include that in the commit message - there are many
> TRMs (one for each GPU) so without the information about exactly which
> specification the page number is pretty useless. Sadly this
> documentation isn't public which would be even better but I don't
> think there are any public specs for this information.
> 
> >> However this change is correct - panfrost_exception_name() should
> >> be taking only the lower 8 bits. Even better though would be to to
> >> report the full raw fault information as well as the high bits can
> >> contain useful information:
> >>
> >>dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at
> >> 0x%016llx\n", fault_status,
> >> panfrost_exception_name(pfdev, fault_status &
> >> 0xFF), address);
> > 
> > So I change it according to what you said?
> 
> Yes, please send a v2.
> 
> Thanks,
> 
> Steve
> 
> > ?? Thu, 10 Jun 2021 11:41:52 +0100
> > Steven Price  :
> > 
> >> The subject should have the prefix "drm/panfrost" and should
> >> mention what the patch is changing (not just the filename).
> >>
> >> On 09/06/2021 07:38, ChunyouTang wrote:

> >>> From: tangchunyou 
> >>>
> >>> The GPU exception fault status register(0x3C),the low 8 bit is the
> >>> EXCEPTION_TYPE.We can see the description at P3-78 in spec.
> >>
> >> Nit: When referring to a spec it's always good to mention the name
> >> - I'm not sure which specification you found this in.
> >>
> >>>
> >>> Signed-off-by: tangchunyou 
> >>> ---
> >>>  drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> >>> b/drivers/gpu/drm/panfrost/panfrost_gpu.c index
> >>> 2aae636f1cf5..1fffb6a0b24f 100644 ---
> >>> a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++
> >>> b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -33,7 +33,7 @@ static
> >>> irqreturn_t panfrost_gpu_irq_handler(int irq, void *data) address
> >>> |= gpu_read(pfdev, GPU_FAULT_ADDRESS_LO); 
> >>>   dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at
> >>> 0x%016llx\n",
> >>> -  fault_status & 0xFF,
> >>> panfrost_exception_name(pfdev, fault_status),
> >>> +  fault_status & 0xFF,
> >>> panfrost_exception_name(pfdev, fault_status & 0xFF),
> >>
> >> However this change is correct - panfrost_exception_name() should
> >> be taking only the lower 8 bits. Even better though would be to to
> >> report the full raw fault information as well as the high bits can
> >> contain useful information:
> >>
> >>dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at
> >> 0x%016llx\n", fault_status,
> >> panfrost_exception_name(pfdev, fault_status &
> >> 0xFF), address);
> >>
> >> Thanks,
> >>
> >> Steve
> >>
> >>>address);
> >>>  
> >>>   if (state & GPU_IRQ_MULTIPLE_FAULT)
> >>>
> > 
> > 




Re: [PATCH v3] Documentation: gpu: Mention the requirements for new properties

2021-06-15 Thread Pekka Paalanen
On Mon, 14 Jun 2021 19:33:47 +0300
Laurent Pinchart  wrote:

> On Mon, Jun 14, 2021 at 04:24:13PM +0100, Liviu Dudau wrote:
> > On Mon, Jun 14, 2021 at 05:49:12PM +0300, Pekka Paalanen wrote:  
> > > On Fri, 11 Jun 2021 13:03:09 +0100
> > > Liviu Dudau  wrote:
> > >   
> > > > On Fri, Jun 11, 2021 at 08:14:59AM +, Simon Ser wrote:  
> > > > > On Thursday, June 10th, 2021 at 23:00, Daniel Vetter 
> > > > >  wrote:
> > > > > 
> > > > > > If there's a strong consensus that we really need this then I'm not
> > > > > > going to nack this, but this really needs a pile of acks from
> > > > > > compositor folks that they're willing to live with the resulting
> > > > > > fallout this will likely bring. Your cc list seems to have an 
> > > > > > absence
> > > > > > of compositor folks, but instead every driver maintainer. That's
> > > > > > backwards. We make uapi for userspace, not for kernel driver
> > > > > > maintainers!
> > > > > 
> > > > > In wlroots we have a policy of only allowing standard KMS properties 
> > > > > to
> > > > > be used. Any vendor-specific property is going to be less 
> > > > > well-defined,
> > > > > less widely useful, potentially have more design issues, potentially
> > > > > overlap in functionality with other vendor-specific properties, likely
> > > > > have some hardware-specific assumptions, etc.
> > > > > 
> > > > > What matters here is discussing with other driver & user-space folks 
> > > > > to
> > > > > make sure the new property's design is sound. Designing uAPI is hard.
> > > > > 
> > > > > If kernel folks are struggling with a user-space implementation, they
> > > > > should discuss with user-space folks to see which project would be
> > > > > interested. There's a chance a compositor will be interested in the 
> > > > > new
> > > > > property and will just do the user-space part for you, if not we can
> > > > > suggest candidate projects.
> > > > > 
> > > > > tl;dr strong agree with Daniel here.
> > > > 
> > > > I think the assumption you and Daniel are making is that the first 
> > > > implementation of
> > > > a new KMS property can be made standard from day one and that it will 
> > > > work for any
> > > > late comer driver as is, without having to make changes to its 
> > > > behaviour in a
> > > > significant way. In my experience that is not the case.
> > > > 
> > > > I think we have moved from the times when we were trying to implement 
> > > > in the Linux
> > > > world features that were available in the hardware but needed a kernel 
> > > > and userspace
> > > > API. The set of properties that exist in KMS cover a lot of needed 
> > > > functionality and
> > > > I don't expect to see new properties for stuff that is already 
> > > > supported by hardware.
> > > > 
> > > > What I'm expected to see in the future is new functionality that gets 
> > > > implemented by
> > > > one hardware vendor and the kernel developers trying to enable that for 
> > > > userspace. It
> > > > could be that the new property is generic, but there is no way of 
> > > > testing that on
> > > > more than one implementation yet, so I'd say we are generous calling it 
> > > > "standard
> > > > property". When the second or third hardware vendor comes along and 
> > > > starts supporting
> > > > that property with their own set of extra requirements, then we can 
> > > > call it
> > > > "standard".  
> > > 
> > > I agree that is a problem with trying to make generic anything. But it
> > > does not mean you should not even try. Maybe trying really hard saves a
> > > couple revisions.  
> > 
> > Agree.
> >   
> > > What I think should be planned for is revisions. How to add new
> > > properties that do the same thing but better, while documenting that a
> > > userspace KMS client can use only one revision at a time. You never
> > > remove old revisions, unless maybe with a DRM client cap they
> > > could disappear from that file description if that is necessary for
> > > seeing the new revision.
> > > 
> > > While designing this, one also needs to take into account that KMS
> > > clients need to be able to save and restore properties *they do not
> > > understand*. So exposing two revisions of the same feature
> > > simultaneously would break save/restore is that's an error.  
> > 
> > I quite like the idea of having versions for properties.  
> 
> It's an interesting idea. We'll need to consider bundles of properties
> in that case I think, as in a v1 a feature could be implemented by
> properties A, B and C, while in v2 C could be replaced by D and E
> (beside A and B themselves also being changed in v2).
> 
> > > > Then comes the effort cost: would it be easier to start with a vendor
> > > > property that only the vendor needs to support (and can submit patches 
> > > > into the
> > > > compositors to do so) and when the standard property gets added moves 
> > > > to that, or  
> > > 
> > > But you can't move, you can only add? You can't delete the old property
> > > in 

Re: [PATCH v4] drm/i915: Invoke another _DSM to enable MUX on HP Workstation laptops

2021-06-15 Thread Kai-Heng Feng
On Fri, Jun 4, 2021 at 11:57 PM Kai-Heng Feng
 wrote:
>
> On Thu, May 20, 2021 at 2:58 PM Kai-Heng Feng
>  wrote:
> >
> > On HP Fury G7 Workstations, graphics output is re-routed from Intel GFX
> > to discrete GFX after S3. This is not desirable, because userspace will
> > treat connected display as a new one, losing display settings.
> >
> > The expected behavior is to let discrete GFX drives all external
> > displays.
> >
> > The platform in question uses ACPI method \_SB.PCI0.HGME to enable MUX.
> > The method is inside the another _DSM, so add the _DSM and call it
> > accordingly.
> >
> > I also tested some MUX-less and iGPU only laptops with that _DSM, no
> > regression was found.
> >
> > v4:
> >  - Rebase.
> >  - Change the DSM name to avoid confusion.
> >  - Move the function call to intel_opregion.
> >
> > v3:
> >  - Remove BXT from names.
> >  - Change the parameter type.
> >  - Fold the function into intel_modeset_init_hw().
> >
> > v2:
> >  - Forward declare struct pci_dev.
> >
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3113
> > References: 
> > https://lore.kernel.org/intel-gfx/1460040732-31417-4-git-send-email-animesh.ma...@intel.com/
> > Signed-off-by: Kai-Heng Feng 
>
> A gentle ping...

Another gentle ping...

>
> > ---
> >  drivers/gpu/drm/i915/display/intel_acpi.c | 19 +++
> >  drivers/gpu/drm/i915/display/intel_acpi.h |  3 +++
> >  drivers/gpu/drm/i915/display/intel_opregion.c |  3 +++
> >  3 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
> > b/drivers/gpu/drm/i915/display/intel_acpi.c
> > index 833d0c1be4f1..7cfe91fc05f2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > @@ -19,6 +19,12 @@ static const guid_t intel_dsm_guid =
> > GUID_INIT(0x7ed873d3, 0xc2d0, 0x4e4f,
> >   0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c);
> >
> > +#define INTEL_DSM_FN_GET_BIOS_DATA_FUNCS_SUPPORTED 0 /* No args */
> > +
> > +static const guid_t intel_dsm_guid2 =
> > +   GUID_INIT(0x3e5b41c6, 0xeb1d, 0x4260,
> > + 0x9d, 0x15, 0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14);
> > +
> >  static char *intel_dsm_port_name(u8 id)
> >  {
> > switch (id) {
> > @@ -176,6 +182,19 @@ void intel_unregister_dsm_handler(void)
> >  {
> >  }
> >
> > +void intel_dsm_get_bios_data_funcs_supported(struct drm_i915_private *i915)
> > +{
> > +   struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > +   acpi_handle dhandle;
> > +
> > +   dhandle = ACPI_HANDLE(>dev);
> > +   if (!dhandle)
> > +   return;
> > +
> > +   acpi_evaluate_dsm(dhandle, _dsm_guid2, INTEL_DSM_REVISION_ID,
> > + INTEL_DSM_FN_GET_BIOS_DATA_FUNCS_SUPPORTED, NULL);
> > +}
> > +
> >  /*
> >   * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All 
> > Devices
> >   * Attached to the Display Adapter).
> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h 
> > b/drivers/gpu/drm/i915/display/intel_acpi.h
> > index e8b068661d22..9f197401c313 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.h
> > @@ -11,11 +11,14 @@ struct drm_i915_private;
> >  #ifdef CONFIG_ACPI
> >  void intel_register_dsm_handler(void);
> >  void intel_unregister_dsm_handler(void);
> > +void intel_dsm_get_bios_data_funcs_supported(struct drm_i915_private 
> > *i915);
> >  void intel_acpi_device_id_update(struct drm_i915_private *i915);
> >  #else
> >  static inline void intel_register_dsm_handler(void) { return; }
> >  static inline void intel_unregister_dsm_handler(void) { return; }
> >  static inline
> > +void intel_dsm_get_bios_data_funcs_supported(struct drm_i915_private 
> > *i915) { return; }
> > +static inline
> >  void intel_acpi_device_id_update(struct drm_i915_private *i915) { return; }
> >  #endif /* CONFIG_ACPI */
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c 
> > b/drivers/gpu/drm/i915/display/intel_opregion.c
> > index dfd724e506b5..3855fba70980 100644
> > --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> > @@ -1078,6 +1078,9 @@ void intel_opregion_resume(struct drm_i915_private 
> > *i915)
> > opregion->asle->ardy = ASLE_ARDY_READY;
> > }
> >
> > +   /* Some platforms abuse the _DSM to enable MUX */
> > +   intel_dsm_get_bios_data_funcs_supported(i915);
> > +
> > intel_opregion_notify_adapter(i915, PCI_D0);
> >  }
> >
> > --
> > 2.31.1
> >


[PATCH 2/2] drm/panfrost:report the full raw fault information instead

2021-06-15 Thread ChunyouTang
From: tangchunyou 

of the low 8 bits.

Signed-off-by: tangchunyou 
---
 drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c 
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 1fffb6a0b24f..d2d287bbf4e7 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -33,7 +33,7 @@ static irqreturn_t panfrost_gpu_irq_handler(int irq, void 
*data)
address |= gpu_read(pfdev, GPU_FAULT_ADDRESS_LO);
 
dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at 0x%016llx\n",
-fault_status & 0xFF, panfrost_exception_name(pfdev, 
fault_status & 0xFF),
+fault_status, panfrost_exception_name(pfdev, 
fault_status & 0xFF),
 address);
 
if (state & GPU_IRQ_MULTIPLE_FAULT)
-- 
2.25.1




[PATCH 2/2] drm/panfrost:report the full raw fault information instead

2021-06-15 Thread ChunyouTang
From: tangchunyou 

of the low 8 bits.

Signed-off-by: tangchunyou 
---
 drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c 
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 1fffb6a0b24f..d2d287bbf4e7 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -33,7 +33,7 @@ static irqreturn_t panfrost_gpu_irq_handler(int irq, void 
*data)
address |= gpu_read(pfdev, GPU_FAULT_ADDRESS_LO);
 
dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at 0x%016llx\n",
-fault_status & 0xFF, panfrost_exception_name(pfdev, 
fault_status & 0xFF),
+fault_status, panfrost_exception_name(pfdev, 
fault_status & 0xFF),
 address);
 
if (state & GPU_IRQ_MULTIPLE_FAULT)
-- 
2.25.1




Re: [PATCH -next] drm/nouveau: Remove set but not used variable 'dev'

2021-06-15 Thread Christian König

Am 25.05.21 um 10:25 schrieb Baokun Li:

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/nouveau/nouveau_bo.c: In function 'nouveau_ttm_tt_populate':
drivers/gpu/drm/nouveau/nouveau_bo.c:1258:17: warning:
  variable ‘dev’ set but not used [-Wunused-but-set-variable]

drivers/gpu/drm/nouveau/nouveau_bo.c: In function 'nouveau_ttm_tt_unpopulate':
drivers/gpu/drm/nouveau/nouveau_bo.c:1281:17: warning:
  variable ‘dev’ set but not used [-Wunused-but-set-variable]

It never used since introduction.

Signed-off-by: Baokun Li 


Acked-by: Christian König 


---
  drivers/gpu/drm/nouveau/nouveau_bo.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 7a2624c0ba4c..51f9a2e6532e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1254,7 +1254,6 @@ nouveau_ttm_tt_populate(struct ttm_device *bdev,
  {
struct ttm_tt *ttm_dma = (void *)ttm;
struct nouveau_drm *drm;
-   struct device *dev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
  
  	if (ttm_tt_is_populated(ttm))

@@ -1267,7 +1266,6 @@ nouveau_ttm_tt_populate(struct ttm_device *bdev,
}
  
  	drm = nouveau_bdev(bdev);

-   dev = drm->dev->dev;
  
  	return ttm_pool_alloc(>ttm.bdev.pool, ttm, ctx);

  }
@@ -1277,14 +1275,12 @@ nouveau_ttm_tt_unpopulate(struct ttm_device *bdev,
  struct ttm_tt *ttm)
  {
struct nouveau_drm *drm;
-   struct device *dev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
  
  	if (slave)

return;
  
  	drm = nouveau_bdev(bdev);

-   dev = drm->dev->dev;
  
  	return ttm_pool_free(>ttm.bdev.pool, ttm);

  }




<    1   2