Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Enabling WD Transcoder
On Thu, 04 Aug 2022, Suraj Kandpal wrote:
> Adding support for writeback transcoder to start capturing frames using
> interrupt mechanism
Basically this changes the assumption that all drm_connectors in i915
are embedded within intel_connectors. That needs to be a separate prep
patch, actually several patches, without the actual writeback
implementation.
Some random comments inline. It's not so much about the writeback
implementation details, but about the overall stuff, with a bunch of
nitpicking about other details.
BR,
Jani.
> Signed-off-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/display/intel_acpi.c | 1 +
> drivers/gpu/drm/i915/display/intel_crtc.c | 3 +
> .../drm/i915/display/intel_crtc_state_dump.c | 1 +
> drivers/gpu/drm/i915/display/intel_ddi.c | 6 +
> drivers/gpu/drm/i915/display/intel_display.c | 63 +-
> drivers/gpu/drm/i915/display/intel_display.h | 15 +-
> .../drm/i915/display/intel_display_debugfs.c | 14 +-
> .../drm/i915/display/intel_display_types.h| 29 +
> drivers/gpu/drm/i915/display/intel_dpll.c | 6 +
> .../drm/i915/display/intel_modeset_setup.c| 67 +-
> .../drm/i915/display/intel_modeset_verify.c | 18 +-
> drivers/gpu/drm/i915/display/intel_opregion.c | 3 +
> .../gpu/drm/i915/display/intel_wb_connector.h | 20 +
> drivers/gpu/drm/i915/display/intel_wd.c | 733 ++
> drivers/gpu/drm/i915/display/intel_wd.h | 76 ++
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 8 +-
> drivers/gpu/drm/i915/i915_pci.c | 7 +-
> 19 files changed, 1046 insertions(+), 27 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h
> create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 522ef9b4aff3..ec63ed16c250 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -302,6 +302,7 @@ i915-y += \
> display/intel_tv.o \
> display/intel_vdsc.o \
> display/intel_vrr.o \
> + display/intel_wd.o \
> display/vlv_dsi.o \
> display/vlv_dsi_pll.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
> b/drivers/gpu/drm/i915/display/intel_acpi.c
> index e78430001f07..ae08db164f73 100644
> --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> @@ -247,6 +247,7 @@ static u32 acpi_display_type(struct intel_connector
> *connector)
> case DRM_MODE_CONNECTOR_LVDS:
> case DRM_MODE_CONNECTOR_eDP:
> case DRM_MODE_CONNECTOR_DSI:
> + case DRM_MODE_CONNECTOR_WRITEBACK:
> display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
> break;
> case DRM_MODE_CONNECTOR_Unknown:
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
> b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 4442aa355f86..f9fa612ac991 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -512,6 +512,9 @@ void intel_pipe_update_start(struct intel_crtc_state
> *new_crtc_state)
> if (min <= 0 || max <= 0)
> goto irq_disable;
>
> + if (new_crtc_state->output_types & BIT(INTEL_OUTPUT_WD))
> + goto irq_disable;
> +
> if (drm_WARN_ON(_priv->drm, drm_crtc_vblank_get(>base)))
> goto irq_disable;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index e9212f69c360..f49630d95d6a 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -71,6 +71,7 @@ static const char * const output_type_str[] = {
> OUTPUT_TYPE(DSI),
> OUTPUT_TYPE(DDI),
> OUTPUT_TYPE(DP_MST),
> + OUTPUT_TYPE(WD)
Please keep the comma at the end so whatever gets added next doesn't
need to add it.
> };
>
> #undef OUTPUT_TYPE
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a4c8493f3ce7..1360406ca531 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1974,6 +1974,12 @@ void intel_ddi_sanitize_encoder_pll_mapping(struct
> intel_encoder *encoder)
>*/
> if (encoder->type == INTEL_OUTPUT_DP_MST)
> return;
> + /*
> + * WD transcoder is a virtual encoder hence sanization
> + * is not required for it
> + */
> + if (encoder->type == INTEL_OUTPUT_WD)
> + return;
>
> if (!encoder->base.crtc && intel_encoder_is_dp(encoder)) {
> u8 pipe_mask;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>
[Intel-gfx] [PATCH v3 2/2] drm/i915: Enabling WD Transcoder
Adding support for writeback transcoder to start capturing frames using
interrupt mechanism
Signed-off-by: Suraj Kandpal
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/display/intel_acpi.c | 1 +
drivers/gpu/drm/i915/display/intel_crtc.c | 3 +
.../drm/i915/display/intel_crtc_state_dump.c | 1 +
drivers/gpu/drm/i915/display/intel_ddi.c | 6 +
drivers/gpu/drm/i915/display/intel_display.c | 63 +-
drivers/gpu/drm/i915/display/intel_display.h | 15 +-
.../drm/i915/display/intel_display_debugfs.c | 14 +-
.../drm/i915/display/intel_display_types.h| 29 +
drivers/gpu/drm/i915/display/intel_dpll.c | 6 +
.../drm/i915/display/intel_modeset_setup.c| 67 +-
.../drm/i915/display/intel_modeset_verify.c | 18 +-
drivers/gpu/drm/i915/display/intel_opregion.c | 3 +
.../gpu/drm/i915/display/intel_wb_connector.h | 20 +
drivers/gpu/drm/i915/display/intel_wd.c | 733 ++
drivers/gpu/drm/i915/display/intel_wd.h | 76 ++
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_irq.c | 8 +-
drivers/gpu/drm/i915/i915_pci.c | 7 +-
19 files changed, 1046 insertions(+), 27 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h
create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c
create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 522ef9b4aff3..ec63ed16c250 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -302,6 +302,7 @@ i915-y += \
display/intel_tv.o \
display/intel_vdsc.o \
display/intel_vrr.o \
+ display/intel_wd.o \
display/vlv_dsi.o \
display/vlv_dsi_pll.o
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index e78430001f07..ae08db164f73 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -247,6 +247,7 @@ static u32 acpi_display_type(struct intel_connector
*connector)
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
case DRM_MODE_CONNECTOR_DSI:
+ case DRM_MODE_CONNECTOR_WRITEBACK:
display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
break;
case DRM_MODE_CONNECTOR_Unknown:
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
b/drivers/gpu/drm/i915/display/intel_crtc.c
index 4442aa355f86..f9fa612ac991 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -512,6 +512,9 @@ void intel_pipe_update_start(struct intel_crtc_state
*new_crtc_state)
if (min <= 0 || max <= 0)
goto irq_disable;
+ if (new_crtc_state->output_types & BIT(INTEL_OUTPUT_WD))
+ goto irq_disable;
+
if (drm_WARN_ON(_priv->drm, drm_crtc_vblank_get(>base)))
goto irq_disable;
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index e9212f69c360..f49630d95d6a 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -71,6 +71,7 @@ static const char * const output_type_str[] = {
OUTPUT_TYPE(DSI),
OUTPUT_TYPE(DDI),
OUTPUT_TYPE(DP_MST),
+ OUTPUT_TYPE(WD)
};
#undef OUTPUT_TYPE
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index a4c8493f3ce7..1360406ca531 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1974,6 +1974,12 @@ void intel_ddi_sanitize_encoder_pll_mapping(struct
intel_encoder *encoder)
*/
if (encoder->type == INTEL_OUTPUT_DP_MST)
return;
+ /*
+* WD transcoder is a virtual encoder hence sanization
+* is not required for it
+*/
+ if (encoder->type == INTEL_OUTPUT_WD)
+ return;
if (!encoder->base.crtc && intel_encoder_is_dp(encoder)) {
u8 pipe_mask;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index a0f84cbe974f..90b41b49e1d7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -116,6 +116,7 @@
#include "intel_sprite.h"
#include "intel_tc.h"
#include "intel_vga.h"
+#include "intel_wd.h"
#include "i9xx_plane.h"
#include "skl_scaler.h"
#include "skl_universal_plane.h"
@@ -1507,6 +1508,9 @@ static void intel_encoders_update_prepare(struct
intel_atomic_state *state)
struct intel_encoder *encoder;
struct intel_crtc *crtc;
+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
+ continue;
+
