Re: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()
On Tue, Feb 20, 2024 at 11:53:47AM -0800, Abhinav Kumar wrote:
> Currently the size parameter of drm_dp_vsc_sdp_pack() is always
> the size of struct dp_sdp. Hence lets drop this parameter and
> use sizeof() directly.
>
> Suggested-by: Dmitry Baryshkov
> Signed-off-by: Abhinav Kumar
it looks indeed an unecessary check.
you can convert my ack to a
Reviewed-by: Rodrigo Vivi
and the ack to take this through drm-misc if needed
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 8 ++--
> drivers/gpu/drm/i915/display/intel_dp.c | 3 +--
> include/drm/display/drm_dp_helper.h | 3 +--
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 6c91f400ecb1..10ee82e34de7 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2918,19 +2918,15 @@ EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
> * @vsc: vsc sdp initialized according to its purpose as defined in
> * table 2-118 - table 2-120 in DP 1.4a specification
> * @sdp: valid handle to the generic dp_sdp which will be packed
> - * @size: valid size of the passed sdp handle
> *
> * Returns length of sdp on success and error code on failure
> */
> ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size)
> + struct dp_sdp *sdp)
> {
> size_t length = sizeof(struct dp_sdp);
>
> - if (size < length)
> - return -ENOSPC;
> -
> - memset(sdp, 0, size);
> + memset(sdp, 0, sizeof(struct dp_sdp));
>
> /*
>* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index a9458df475e2..e13121dc3a03 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4181,8 +4181,7 @@ static void intel_write_dp_sdp(struct intel_encoder
> *encoder,
>
> switch (type) {
> case DP_SDP_VSC:
> - len = drm_dp_vsc_sdp_pack(_state->infoframes.vsc, ,
> - sizeof(sdp));
> + len = drm_dp_vsc_sdp_pack(_state->infoframes.vsc, );
> break;
> case HDMI_PACKET_TYPE_GAMUT_METADATA:
> len = intel_dp_hdr_metadata_infoframe_sdp_pack(dev_priv,
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 8474504d4c88..1f41994796d3 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -812,7 +812,6 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
> int bpp_x16, unsigned long flags);
> int drm_dp_bw_channel_coding_efficiency(bool is_uhbr);
>
> -ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size);
> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp
> *sdp);
>
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 2.34.1
>
Re: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()
On Tue, Feb 20, 2024 at 11:53:47AM -0800, Abhinav Kumar wrote:
> Currently the size parameter of drm_dp_vsc_sdp_pack() is always
> the size of struct dp_sdp. Hence lets drop this parameter and
> use sizeof() directly.
>
> Suggested-by: Dmitry Baryshkov
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 8 ++--
> drivers/gpu/drm/i915/display/intel_dp.c | 3 +--
Acked-by: Rodrigo Vivi
> include/drm/display/drm_dp_helper.h | 3 +--
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 6c91f400ecb1..10ee82e34de7 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2918,19 +2918,15 @@ EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
> * @vsc: vsc sdp initialized according to its purpose as defined in
> * table 2-118 - table 2-120 in DP 1.4a specification
> * @sdp: valid handle to the generic dp_sdp which will be packed
> - * @size: valid size of the passed sdp handle
> *
> * Returns length of sdp on success and error code on failure
> */
> ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size)
> + struct dp_sdp *sdp)
> {
> size_t length = sizeof(struct dp_sdp);
>
> - if (size < length)
> - return -ENOSPC;
> -
> - memset(sdp, 0, size);
> + memset(sdp, 0, sizeof(struct dp_sdp));
>
> /*
>* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index a9458df475e2..e13121dc3a03 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4181,8 +4181,7 @@ static void intel_write_dp_sdp(struct intel_encoder
> *encoder,
>
> switch (type) {
> case DP_SDP_VSC:
> - len = drm_dp_vsc_sdp_pack(_state->infoframes.vsc, ,
> - sizeof(sdp));
> + len = drm_dp_vsc_sdp_pack(_state->infoframes.vsc, );
> break;
> case HDMI_PACKET_TYPE_GAMUT_METADATA:
> len = intel_dp_hdr_metadata_infoframe_sdp_pack(dev_priv,
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 8474504d4c88..1f41994796d3 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -812,7 +812,6 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
> int bpp_x16, unsigned long flags);
> int drm_dp_bw_channel_coding_efficiency(bool is_uhbr);
>
> -ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size);
> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp
> *sdp);
>
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 2.34.1
>
[PATCH 11/13] drm/sched: Reverse run-queue priority enumeration
From: Luben Tuikov
Reverse run-queue priority enumeration such that the higest priority is now 0,
and for each consecutive integer the prioirty diminishes.
Run-queues correspond to priorities. To an external observer a scheduler
created with a single run-queue, and another created with
DRM_SCHED_PRIORITY_COUNT number of run-queues, should always schedule
sched->sched_rq[0] with the same "priority", as that index run-queue exists in
both schedulers, i.e. a scheduler with one run-queue or many. This patch makes
it so.
In other words, the "priority" of sched->sched_rq[n], n >= 0, is the same for
any scheduler created with any allowable number of run-queues (priorities), 0
to DRM_SCHED_PRIORITY_COUNT.
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Danilo Krummrich
Cc: Alex Deucher
Cc: Christian König
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Luben Tuikov
Reviewed-by: Christian König
Link:
https://patchwork.freedesktop.org/patch/msgid/20231124052752.6915-6-ltuiko...@gmail.com
(cherry picked from commit 38f922a563aac3148ac73e73689805917f034cb5)
---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
drivers/gpu/drm/msm/msm_gpu.h| 2 +-
drivers/gpu/drm/scheduler/sched_entity.c | 5 +++--
drivers/gpu/drm/scheduler/sched_main.c | 15 +++
include/drm/gpu_scheduler.h | 6 +++---
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 1a25931607c5..71a5cf37b472 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -325,7 +325,7 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
drm_gpu_scheduler *sched)
int i;
/* Signal all jobs not yet scheduled */
- for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_LOW; i--) {
+ for (i = DRM_SCHED_PRIORITY_KERNEL; i < sched->num_rqs; i++) {
struct drm_sched_rq *rq = sched->sched_rq[i];
spin_lock(>lock);
list_for_each_entry(s_entity, >entities, list) {
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index eb0c97433e5f..2bfcb222e353 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -347,7 +347,7 @@ struct msm_gpu_perfcntr {
* DRM_SCHED_PRIORITY_KERNEL priority level is treated specially in some
* cases, so we don't use it (no need for kernel generated jobs).
*/
-#define NR_SCHED_PRIORITIES (1 + DRM_SCHED_PRIORITY_HIGH -
DRM_SCHED_PRIORITY_LOW)
+#define NR_SCHED_PRIORITIES (1 + DRM_SCHED_PRIORITY_LOW -
DRM_SCHED_PRIORITY_HIGH)
/**
* struct msm_file_private - per-drm_file context
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index dd2b8f777f51..3c4f5a392b06 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -82,13 +82,14 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
pr_warn("%s: called with uninitialized scheduler\n", __func__);
} else if (num_sched_list) {
/* The "priority" of an entity cannot exceed the number of
run-queues of a
-* scheduler. Protect against num_rqs being 0, by converting to
signed.
+* scheduler. Protect against num_rqs being 0, by converting to
signed. Choose
+* the lowest priority available.
*/
if (entity->priority >= sched_list[0]->num_rqs) {
drm_err(sched_list[0], "entity with out-of-bounds
priority:%u num_rqs:%u\n",
entity->priority, sched_list[0]->num_rqs);
entity->priority = max_t(s32, (s32)
sched_list[0]->num_rqs - 1,
-(s32) DRM_SCHED_PRIORITY_LOW);
+(s32)
DRM_SCHED_PRIORITY_KERNEL);
}
entity->rq = sched_list[0]->sched_rq[entity->priority];
}
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index b6d7bc49ff6e..682aebe96db7 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1051,8 +1051,9 @@ drm_sched_select_entity(struct drm_gpu_scheduler *sched)
struct drm_sched_entity *entity;
int i;
- /* Kernel run queue has higher priority than normal run queue*/
- for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_LOW; i--) {
+ /* Start with the highest priority.
+*/
+ for (i = DRM_SCHED_PRIORITY_KERNEL; i < sched->num_rqs; i++) {
entity = drm_sched_policy == DRM_SCHED_POLICY_FIFO ?
drm_sched_rq_select_entity_fifo(sched,
sched->sched_rq[i]) :
[PATCH 10/13] drm/sched: Rename priority MIN to LOW
From: Luben Tuikov
Rename DRM_SCHED_PRIORITY_MIN to DRM_SCHED_PRIORITY_LOW.
This mirrors DRM_SCHED_PRIORITY_HIGH, for a list of DRM scheduler priorities
in ascending order,
DRM_SCHED_PRIORITY_LOW,
DRM_SCHED_PRIORITY_NORMAL,
DRM_SCHED_PRIORITY_HIGH,
DRM_SCHED_PRIORITY_KERNEL.
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Danilo Krummrich
Cc: Alex Deucher
Cc: Christian König
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Luben Tuikov
Reviewed-by: Christian König
Link:
https://patchwork.freedesktop.org/patch/msgid/20231124052752.6915-5-ltuiko...@gmail.com
(cherry picked from commit fe375c74806dbd30b00ec038a80a5b7bf4653ab7)
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
drivers/gpu/drm/msm/msm_gpu.h| 2 +-
drivers/gpu/drm/scheduler/sched_entity.c | 2 +-
drivers/gpu/drm/scheduler/sched_main.c | 10 +-
include/drm/gpu_scheduler.h | 2 +-
6 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index fb050345b9f2..d46786618061 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -68,10 +68,10 @@ amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
return DRM_SCHED_PRIORITY_NORMAL;
case AMDGPU_CTX_PRIORITY_VERY_LOW:
- return DRM_SCHED_PRIORITY_MIN;
+ return DRM_SCHED_PRIORITY_LOW;
case AMDGPU_CTX_PRIORITY_LOW:
- return DRM_SCHED_PRIORITY_MIN;
+ return DRM_SCHED_PRIORITY_LOW;
case AMDGPU_CTX_PRIORITY_NORMAL:
return DRM_SCHED_PRIORITY_NORMAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 62bb7fc7448a..1a25931607c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -325,7 +325,7 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
drm_gpu_scheduler *sched)
int i;
/* Signal all jobs not yet scheduled */
- for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) {
+ for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_LOW; i--) {
struct drm_sched_rq *rq = sched->sched_rq[i];
spin_lock(>lock);
list_for_each_entry(s_entity, >entities, list) {
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 4252e3839fbc..eb0c97433e5f 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -347,7 +347,7 @@ struct msm_gpu_perfcntr {
* DRM_SCHED_PRIORITY_KERNEL priority level is treated specially in some
* cases, so we don't use it (no need for kernel generated jobs).
*/
-#define NR_SCHED_PRIORITIES (1 + DRM_SCHED_PRIORITY_HIGH -
DRM_SCHED_PRIORITY_MIN)
+#define NR_SCHED_PRIORITIES (1 + DRM_SCHED_PRIORITY_HIGH -
DRM_SCHED_PRIORITY_LOW)
/**
* struct msm_file_private - per-drm_file context
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index ee645d38e98d..dd2b8f777f51 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -88,7 +88,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
drm_err(sched_list[0], "entity with out-of-bounds
priority:%u num_rqs:%u\n",
entity->priority, sched_list[0]->num_rqs);
entity->priority = max_t(s32, (s32)
sched_list[0]->num_rqs - 1,
-(s32) DRM_SCHED_PRIORITY_MIN);
+(s32) DRM_SCHED_PRIORITY_LOW);
}
entity->rq = sched_list[0]->sched_rq[entity->priority];
}
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 044a8c4875ba..b6d7bc49ff6e 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1052,7 +1052,7 @@ drm_sched_select_entity(struct drm_gpu_scheduler *sched)
int i;
/* Kernel run queue has higher priority than normal run queue*/
- for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) {
+ for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_LOW; i--) {
entity = drm_sched_policy == DRM_SCHED_POLICY_FIFO ?
drm_sched_rq_select_entity_fifo(sched,
sched->sched_rq[i]) :
drm_sched_rq_select_entity_rr(sched,
sched->sched_rq[i]);
@@ -1291,7 +1291,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
if (!sched->sched_rq)
goto Out_free;
sched->num_rqs = num_rqs;
- for (i = DRM_SCHED_PRIORITY_MIN; i < sched->num_rqs; i++) {
+ for (i =
[Freedreno] [PATCH 11/20] drm/sched: Convert the GPU scheduler to variable number of run-queues
From: Luben Tuikov
The GPU scheduler has now a variable number of run-queues, which are set up at
drm_sched_init() time. This way, each driver announces how many run-queues it
requires (supports) per each GPU scheduler it creates. Note, that run-queues
correspond to scheduler "priorities", thus if the number of run-queues is set
to 1 at drm_sched_init(), then that scheduler supports a single run-queue,
i.e. single "priority". If a driver further sets a single entity per
run-queue, then this creates a 1-to-1 correspondence between a scheduler and
a scheduled entity.
Cc: Lucas Stach
Cc: Russell King
Cc: Qiang Yu
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Danilo Krummrich
Cc: Matthew Brost
Cc: Boris Brezillon
Cc: Alex Deucher
Cc: Christian König
Cc: Emma Anholt
Cc: etna...@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Luben Tuikov
Acked-by: Christian König
Link: https://lore.kernel.org/r/20231023032251.164775-1-luben.tui...@amd.com
(cherry picked from commit 56e449603f0ac580700621a356d35d5716a62ce5)
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4 +-
drivers/gpu/drm/etnaviv/etnaviv_sched.c| 1 +
drivers/gpu/drm/lima/lima_sched.c | 4 +-
drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +-
drivers/gpu/drm/nouveau/nouveau_sched.c| 1 +
drivers/gpu/drm/panfrost/panfrost_job.c| 1 +
drivers/gpu/drm/scheduler/sched_entity.c | 18 +-
drivers/gpu/drm/scheduler/sched_main.c | 74 ++
drivers/gpu/drm/v3d/v3d_sched.c| 5 ++
include/drm/gpu_scheduler.h| 9 ++-
11 files changed, 98 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 30c4f5cca02c..15074232fbbd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct
amdgpu_device *adev)
}
r = drm_sched_init(>sched, _sched_ops,
+ DRM_SCHED_PRIORITY_COUNT,
ring->num_hw_submission, 0,
timeout, adev->reset_domain->wq,
ring->sched_score, ring->name,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 78476bc75b4e..1f357198533f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -325,8 +325,8 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
drm_gpu_scheduler *sched)
int i;
/* Signal all jobs not yet scheduled */
- for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >= DRM_SCHED_PRIORITY_MIN;
i--) {
- struct drm_sched_rq *rq = >sched_rq[i];
+ for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) {
+ struct drm_sched_rq *rq = sched->sched_rq[i];
spin_lock(>lock);
list_for_each_entry(s_entity, >entities, list) {
while ((s_job =
to_drm_sched_job(spsc_queue_pop(_entity->job_queue {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 345fec6cb1a4..9b79f218e21a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -135,6 +135,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
int ret;
ret = drm_sched_init(>sched, _sched_ops,
+DRM_SCHED_PRIORITY_COUNT,
etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
msecs_to_jiffies(500), NULL, NULL,
dev_name(gpu->dev), gpu->dev);
diff --git a/drivers/gpu/drm/lima/lima_sched.c
b/drivers/gpu/drm/lima/lima_sched.c
index ffd91a5ee299..295f0353a02e 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -488,7 +488,9 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe,
const char *name)
INIT_WORK(>recover_work, lima_sched_recover_work);
- return drm_sched_init(>base, _sched_ops, 1,
+ return drm_sched_init(>base, _sched_ops,
+ DRM_SCHED_PRIORITY_COUNT,
+ 1,
lima_job_hang_limit,
msecs_to_jiffies(timeout), NULL,
NULL, name, pipe->ldev->dev);
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c
b/drivers/gpu/drm/msm/msm_ringbuffer.c
index 40c0bc35a44c..95257ab0185d 100644
--- a/drivers/gpu/drm/ms
Re: [Freedreno] [PATCH v10 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers
On Sat, May 20, 2023 at 02:07:51AM +0300, Dmitry Baryshkov wrote: > On 20/05/2023 00:16, Rodrigo Vivi wrote: > > On Fri, May 19, 2023 at 07:55:47PM +0300, Dmitry Baryshkov wrote: > > > On 19/04/2023 18:43, Mark Yacoub wrote: > > > > Hi all, > > > > This is v10 of the HDCP patches. The patches are authored by Sean Paul. > > > > I rebased and addressed the review comments in v6-v10. > > > > > > > > Main change in v10 is handling the kernel test bot warnings. > > > > > > > > Patches 1-4 focus on moving the common HDCP helpers to common DRM. > > > > This introduces a slight change in the original intel flow > > > > as it splits the unique driver protocol from the generic implementation. > > > > > > > > Patches 5-7 split the HDCP flow on the i915 driver to make use of the > > > > common DRM helpers. > > > > > > > > Patches 8-10 implement HDCP on MSM driver. > > > > > > > > Thanks, > > > > -Mark Yacoub > > > > > > > > Sean Paul (10): > > > > drm/hdcp: Add drm_hdcp_atomic_check() > > > > drm/hdcp: Avoid changing crtc state in hdcp atomic check > > > > drm/hdcp: Update property value on content type and user changes > > > > drm/hdcp: Expand HDCP helper library for enable/disable/check > > > > drm/i915/hdcp: Consolidate HDCP setup/state cache > > > > drm/i915/hdcp: Retain hdcp_capable return codes > > > > drm/i915/hdcp: Use HDCP helpers for i915 > > > > dt-bindings: msm/dp: Add bindings for HDCP registers > > > > arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller > > > > > > Dear i915 maintainers, > > > > > > I wanted to ping you regarding this patch series. If there are no comments > > > for the series from you side, would it be possible to land Intel-specific > > > and generic patches into drm-intel tree? We will continue working on the > > > msm > > > specific parts and merge them through the msm tree. > > > > pushed to drm-intel-next. > > > > should be propagated in a few weeks to drm-next on our next pull request. > > Probably there is some kind of confusion here. You've pushed the DSC > patches, while the response was sent to the HDCP series. I'm sorry, my confusion for replying to the wrong thread. So, on this one here I believe it would be helpful if there's a split in the series with the already reviewed ones related to i915 are resent separated from the rest, send with --subject-prefix="CI" so when that pass CI we just merge it through drm-intel-next > > -- > With best wishes > Dmitry >
Re: [Freedreno] [PATCH v10 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers
On Fri, May 19, 2023 at 07:55:47PM +0300, Dmitry Baryshkov wrote: > On 19/04/2023 18:43, Mark Yacoub wrote: > > Hi all, > > This is v10 of the HDCP patches. The patches are authored by Sean Paul. > > I rebased and addressed the review comments in v6-v10. > > > > Main change in v10 is handling the kernel test bot warnings. > > > > Patches 1-4 focus on moving the common HDCP helpers to common DRM. > > This introduces a slight change in the original intel flow > > as it splits the unique driver protocol from the generic implementation. > > > > Patches 5-7 split the HDCP flow on the i915 driver to make use of the > > common DRM helpers. > > > > Patches 8-10 implement HDCP on MSM driver. > > > > Thanks, > > -Mark Yacoub > > > > Sean Paul (10): > >drm/hdcp: Add drm_hdcp_atomic_check() > >drm/hdcp: Avoid changing crtc state in hdcp atomic check > >drm/hdcp: Update property value on content type and user changes > >drm/hdcp: Expand HDCP helper library for enable/disable/check > >drm/i915/hdcp: Consolidate HDCP setup/state cache > >drm/i915/hdcp: Retain hdcp_capable return codes > >drm/i915/hdcp: Use HDCP helpers for i915 > >dt-bindings: msm/dp: Add bindings for HDCP registers > >arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller > > Dear i915 maintainers, > > I wanted to ping you regarding this patch series. If there are no comments > for the series from you side, would it be possible to land Intel-specific > and generic patches into drm-intel tree? We will continue working on the msm > specific parts and merge them through the msm tree. pushed to drm-intel-next. should be propagated in a few weeks to drm-next on our next pull request. > > >drm/msm: Implement HDCP 1.x using the new drm HDCP helpers > > > > .../bindings/display/msm/dp-controller.yaml |7 +- > > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi |8 + > > drivers/gpu/drm/display/drm_hdcp_helper.c | 1224 + > > drivers/gpu/drm/i915/display/intel_atomic.c |8 +- > > drivers/gpu/drm/i915/display/intel_ddi.c | 32 +- > > .../drm/i915/display/intel_display_debugfs.c | 12 +- > > .../drm/i915/display/intel_display_types.h| 51 +- > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 352 ++--- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 1060 +++--- > > drivers/gpu/drm/i915/display/intel_hdcp.h | 48 +- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 267 ++-- > > drivers/gpu/drm/msm/Kconfig |1 + > > drivers/gpu/drm/msm/Makefile |1 + > > drivers/gpu/drm/msm/dp/dp_catalog.c | 156 +++ > > drivers/gpu/drm/msm/dp/dp_catalog.h | 18 + > > drivers/gpu/drm/msm/dp/dp_debug.c | 46 +- > > drivers/gpu/drm/msm/dp/dp_debug.h | 11 +- > > drivers/gpu/drm/msm/dp/dp_display.c | 39 +- > > drivers/gpu/drm/msm/dp/dp_display.h |5 + > > drivers/gpu/drm/msm/dp/dp_drm.c | 39 +- > > drivers/gpu/drm/msm/dp/dp_drm.h |7 + > > drivers/gpu/drm/msm/dp/dp_hdcp.c | 389 ++ > > drivers/gpu/drm/msm/dp/dp_hdcp.h | 33 + > > drivers/gpu/drm/msm/dp/dp_parser.c| 14 + > > drivers/gpu/drm/msm/dp/dp_parser.h|4 + > > drivers/gpu/drm/msm/dp/dp_reg.h | 30 +- > > drivers/gpu/drm/msm/msm_atomic.c | 19 + > > include/drm/display/drm_hdcp.h| 296 > > include/drm/display/drm_hdcp_helper.h | 23 + > > 30 files changed, 2867 insertions(+), 1349 deletions(-) > > create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.c > > create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.h > > > > -- > With best wishes > Dmitry >
Re: [Freedreno] [PATCH v2 0/2] drm: fdinfo memory stats
On Wed, Apr 12, 2023 at 10:11:32AM +0200, Daniel Vetter wrote: > On Wed, Apr 12, 2023 at 01:36:52AM +0300, Dmitry Baryshkov wrote: > > On 11/04/2023 21:28, Rob Clark wrote: > > > On Tue, Apr 11, 2023 at 10:36 AM Dmitry Baryshkov > > > wrote: > > > > > > > > On Tue, 11 Apr 2023 at 20:13, Rob Clark wrote: > > > > > > > > > > On Tue, Apr 11, 2023 at 9:53 AM Daniel Vetter wrote: > > > > > > > > > > > > On Tue, Apr 11, 2023 at 09:47:32AM -0700, Rob Clark wrote: > > > > > > > On Mon, Apr 10, 2023 at 2:06 PM Rob Clark > > > > > > > wrote: > > > > > > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > > > > > Similar motivation to other similar recent attempt[1]. But > > > > > > > > with an > > > > > > > > attempt to have some shared code for this. As well as > > > > > > > > documentation. > > > > > > > > > > > > > > > > It is probably a bit UMA-centric, I guess devices with VRAM > > > > > > > > might want > > > > > > > > some placement stats as well. But this seems like a reasonable > > > > > > > > start. > > > > > > > > > > > > > > > > Basic gputop support: > > > > > > > > https://patchwork.freedesktop.org/series/116236/ > > > > > > > > And already nvtop support: > > > > > > > > https://github.com/Syllo/nvtop/pull/204 > > > > > > > > > > > > > > On a related topic, I'm wondering if it would make sense to report > > > > > > > some more global things (temp, freq, etc) via fdinfo? Some of > > > > > > > this, > > > > > > > tools like nvtop could get by trawling sysfs or other driver > > > > > > > specific > > > > > > > ways. But maybe it makes sense to have these sort of things > > > > > > > reported > > > > > > > in a standardized way (even though they aren't really > > > > > > > per-drm_file) > > > > > > > > > > > > I think that's a bit much layering violation, we'd essentially have > > > > > > to > > > > > > reinvent the hwmon sysfs uapi in fdinfo. Not really a business I > > > > > > want to > > > > > > be in :-) > > > > > > > > > > I guess this is true for temp (where there are thermal zones with > > > > > potentially multiple temp sensors.. but I'm still digging my way thru > > > > > the thermal_cooling_device stuff) > > > > > > > > It is slightly ugly. All thermal zones and cooling devices are virtual > > > > devices (so, even no connection to the particular tsens device). One > > > > can either enumerate them by checking > > > > /sys/class/thermal/thermal_zoneN/type or enumerate them through > > > > /sys/class/hwmon. For cooling devices again the only enumeration is > > > > through /sys/class/thermal/cooling_deviceN/type. > > > > > > > > Probably it should be possible to push cooling devices and thermal > > > > zones under corresponding providers. However I do not know if there is > > > > a good way to correlate cooling device (ideally a part of GPU) to the > > > > thermal_zone (which in our case is provided by tsens / temp_alarm > > > > rather than GPU itself). > > > > > > > > > > > > > > But what about freq? I think, esp for cases where some "fw thing" is > > > > > controlling the freq we end up needing to use gpu counters to measure > > > > > the freq. > > > > > > > > For the freq it is slightly easier: /sys/class/devfreq/*, devices are > > > > registered under proper parent (IOW, GPU). So one can read > > > > /sys/class/devfreq/3d0.gpu/cur_freq or > > > > /sys/bus/platform/devices/3d0.gpu/devfreq/3d0.gpu/cur_freq. > > > > > > > > However because of the components usage, there is no link from > > > > /sys/class/drm/card0 > > > > (/sys/devices/platform/soc@0/ae0.display-subsystem/ae01000.display-controller/drm/card0) > > > > to /sys/devices/platform/soc@0/3d0.gpu, the GPU unit. > > > > > > > > Getting all these items together in a platform-independent way would > > > > be definitely an important but complex topic. > > > > > > But I don't believe any of the pci gpu's use devfreq ;-) > > > > > > And also, you can't expect the CPU to actually know the freq when fw > > > is the one controlling freq. We can, currently, have a reasonable > > > approximation from devfreq but that stops if IFPC is implemented. And > > > other GPUs have even less direct control. So freq is a thing that I > > > don't think we should try to get from "common frameworks" > > > > I think it might be useful to add another passive devfreq governor type for > > external frequencies. This way we can use the same interface to export > > non-CPU-controlled frequencies. > > Yeah this sounds like a decent idea to me too. It might also solve the fun > of various pci devices having very non-standard freq controls in sysfs > (looking at least at i915 here ...) I also like the idea of having some common infrastructure for the GPU freq. hwmon have a good infrastructure, but they are more focused on individual monitoring devices and not very welcomed to embedded monitoring and control. I still want to check the opportunity to see if at least some freq control could be
Re: [Freedreno] [PATCH v9 15/15] drm/i915: Add deadline based boost support
On Thu, Mar 02, 2023 at 03:53:37PM -0800, Rob Clark wrote:
> From: Rob Clark
>
missing some wording here...
> v2: rebase
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/i915/i915_request.c | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c
> b/drivers/gpu/drm/i915/i915_request.c
> index 7503dcb9043b..44491e7e214c 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -97,6 +97,25 @@ static bool i915_fence_enable_signaling(struct dma_fence
> *fence)
> return i915_request_enable_breadcrumb(to_request(fence));
> }
>
> +static void i915_fence_set_deadline(struct dma_fence *fence, ktime_t
> deadline)
> +{
> + struct i915_request *rq = to_request(fence);
> +
> + if (i915_request_completed(rq))
> + return;
> +
> + if (i915_request_started(rq))
> + return;
why do we skip the boost if already started?
don't we want to boost the freq anyway?
> +
> + /*
> + * TODO something more clever for deadlines that are in the
> + * future. I think probably track the nearest deadline in
> + * rq->timeline and set timer to trigger boost accordingly?
> + */
I'm afraid it will be very hard to find some heuristics of what's
late enough for the boost no?
I mean, how early to boost the freq on an upcoming deadline for the
timer?
> +
> + intel_rps_boost(rq);
> +}
> +
> static signed long i915_fence_wait(struct dma_fence *fence,
> bool interruptible,
> signed long timeout)
> @@ -182,6 +201,7 @@ const struct dma_fence_ops i915_fence_ops = {
> .signaled = i915_fence_signaled,
> .wait = i915_fence_wait,
> .release = i915_fence_release,
> + .set_deadline = i915_fence_set_deadline,
> };
>
> static void irq_execute_cb(struct irq_work *wrk)
> --
> 2.39.1
>
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 27, 2023 at 02:20:04PM -0800, Rob Clark wrote: > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi wrote: > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > >> Tvrtko Ursulin wrote: > > > > >> > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > >>>> On Fri, 24 Feb 2023 09:41:46 + > > > > >>>> Tvrtko Ursulin wrote: > > > > >>>> > > > > >>>>> On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > >>>>>> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > >>>>>> Rob Clark wrote: > > > > >>>>>> > > > > >>>>>>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > >>>>>>> wrote: > > > > >>>>>>>> > > > > >>>>>>>> On Wed, 22 Feb 2023 07:37:26 -0800 > > > > >>>>>>>> Rob Clark wrote: > > > > >>>>>>>> > > > > >>>>>>>>> On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > >>>>>>>>> wrote: > > > > >>>>>> > > > > >>>>>> ... > > > > >>>>>> > > > > >>>>>>>>>> On another matter, if the application uses SET_DEADLINE with > > > > >>>>>>>>>> one > > > > >>>>>>>>>> timestamp, and the compositor uses SET_DEADLINE on the same > > > > >>>>>>>>>> thing with > > > > >>>>>>>>>> another timestamp, what should happen? > > > > >>>>>>>>> > > > > >>>>>>>>> The expectation is that many deadline hints can be set on a > > > > >>>>>>>>> fence. > > > > >>>>>>>>> The fence signaller should track the soonest deadline. > > > > >>>>>>>> > > > > >>>>>>>> You need to document that as UAPI, since it is observable to > > > > >>>>>>>> userspace. > > > > >>>>>>>> It would be bad if drivers or subsystems would differ in > > > > >>>>>>>> behaviour. > > > > >>>>>>>> > > > > >>>>>>> > > > > >>>>>>> It is in the end a hint. It is about giving the driver more > > > > >>>>>>> information so that it can make better choices. But the driver > > > > >>>>>>> is > > > > >>>>>>> even free to ignore it. So maybe "expectation" is too strong > > > > >>>>>>> of a > > > > >>>>>>> word. Rather, any other behavior doesn't really make sense. > > > > >>>>>>> But it > > > > >>>>>>> could end up being dictated by how the hw and/or fw works. > > > > >>>>>> > > > > >>>>>> It will stop being a hint once it has been implemented and used > > > > >>>>>> in the > > > > >>>>>> wild long enough. The kernel userspace regression rules make > > > > >>>>>> sure of > > > > >>>>>> that. > > > > >>>>> > > > > >>>>> Yeah, tricky and maybe a gray area in this case. I think we eluded > > > > >>>>> elsewhere in the thread that renaming the thing might be an > > > > >>>>> option. > > > > >>>>> > > > > >>>>> So maybe instead of deadline, which is a very strong word, use > > > > >>>>> something > > > > >>>>> along the lines of "present time hint", or "signalled time hint"? > > > > >>>>> Mayb
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > >> On Fri, 24 Feb 2023 10:50:51 + > > >> Tvrtko Ursulin wrote: > > >> > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > On Fri, 24 Feb 2023 09:41:46 + > > Tvrtko Ursulin wrote: > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > >> Rob Clark wrote: > > >> > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > >>> wrote: > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > Rob Clark wrote: > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > wrote: > > >> > > >> ... > > >> > > >> On another matter, if the application uses SET_DEADLINE with one > > >> timestamp, and the compositor uses SET_DEADLINE on the same > > >> thing with > > >> another timestamp, what should happen? > > > > > > The expectation is that many deadline hints can be set on a fence. > > > The fence signaller should track the soonest deadline. > > > > You need to document that as UAPI, since it is observable to > > userspace. > > It would be bad if drivers or subsystems would differ in behaviour. > > > > >>> > > >>> It is in the end a hint. It is about giving the driver more > > >>> information so that it can make better choices. But the driver is > > >>> even free to ignore it. So maybe "expectation" is too strong of a > > >>> word. Rather, any other behavior doesn't really make sense. But it > > >>> could end up being dictated by how the hw and/or fw works. > > >> > > >> It will stop being a hint once it has been implemented and used in > > >> the > > >> wild long enough. The kernel userspace regression rules make sure of > > >> that. > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > elsewhere in the thread that renaming the thing might be an option. > > > > > > So maybe instead of deadline, which is a very strong word, use > > > something > > > along the lines of "present time hint", or "signalled time hint"? > > > Maybe > > > reads clumsy. Just throwing some ideas for a start. > > > > You can try, but I fear that if it ever changes behaviour and > > someone notices that, it's labelled as a kernel regression. I don't > > think documentation has ever been the authoritative definition of UABI > > in Linux, it just guides drivers and userspace towards a common > > understanding and common usage patterns. > > > > So even if the UABI contract is not documented (ugh), you need to be > > prepared to set the UABI contract through kernel implementation. > > >>> > > >>> To be the devil's advocate it probably wouldn't be an ABI regression but > > >>> just an regression. Same way as what nice(2) priorities mean hasn't > > >>> always been the same over the years, I don't think there is a strict > > >>> contract. > > >>> > > >>> Having said that, it may be different with latency sensitive stuff such > > >>> as UIs though since it is very observable and can be very painful to > > >>> users. > > >>> > > If you do not document the UABI contract, then different drivers are > > likely to implement it differently, leading to differing behaviour. > > Also userspace will invent wild ways to abuse the UABI if there is no > > documentation guiding it on proper use. If userspace or end users > > observe different behaviour, that's bad even if it's not a regression. > > > > I don't like the situation either, but it is what it is. UABI stability > > trumps everything regardless of whether it was documented or not. > > > > I bet userspace is going to use this as a "make it faster, make it > > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > > library that stamps any and all fences with an expired deadline to > > just squeeze out a little more through some weird side-effect. > > > > Well, that's hopefully overboard in scaring, but in the end, I would > > like to see UABI documented so I can have a feeling of what it is for > > and how it was intended to be used. That's all. > > >>> > > >>> We share the same concern. If you read elsewhere in these threads you > > >>> will notice I have been calling this an "arms race". If the ability to > > >>> make yourself go faster does not required additional privilege I also > > >>> worry everyone will do it at which point it becomes pointless. So yes, I > > >>> do share this concern about exposing any of this as an unprivileged > > >>> uapi. > > >>> > > >>>
Re: [Freedreno] [PATCH v5 09/14] drm/syncobj: Add deadline support for syncobj waits
On Wed, Feb 22, 2023 at 12:09:04PM +0200, Pekka Paalanen wrote:
> On Tue, 21 Feb 2023 09:25:18 -0800
> Rob Clark wrote:
>
> > On Tue, Feb 21, 2023 at 12:53 AM Pekka Paalanen wrote:
> > >
> > > On Mon, 20 Feb 2023 12:18:56 -0800
> > > Rob Clark wrote:
> > >
> > > > From: Rob Clark
> > > >
> > > > Add a new flag to let userspace provide a deadline as a hint for syncobj
> > > > and timeline waits. This gives a hint to the driver signaling the
> > > > backing fences about how soon userspace needs it to compete work, so it
> > > > can addjust GPU frequency accordingly. An immediate deadline can be
> > > > given to provide something equivalent to i915 "wait boost".
> > > >
> > > > v2: Use absolute u64 ns value for deadline hint, drop cap and driver
> > > > feature flag in favor of allowing count_handles==0 as a way for
> > > > userspace to probe kernel for support of new flag
> > > >
> > > > Signed-off-by: Rob Clark
> > > > ---
> > > > drivers/gpu/drm/drm_syncobj.c | 59 +++
> > > > include/uapi/drm/drm.h| 5 +++
> > > > 2 files changed, 51 insertions(+), 13 deletions(-)
> > >
> > > ...
> > >
> > > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > > > index 642808520d92..aefc8cc743e0 100644
> > > > --- a/include/uapi/drm/drm.h
> > > > +++ b/include/uapi/drm/drm.h
> > > > @@ -887,6 +887,7 @@ struct drm_syncobj_transfer {
> > > > #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
> > > > #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
> > > > #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) /* wait for
> > > > time point to become available */
> > > > +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE (1 << 3) /* set fence
> > > > deadline based to deadline_nsec/sec */
> > >
> > > Hi,
> > >
> > > where is the UAPI documentation explaining what is a "fence deadline"
> > > and what setting it does here?
> >
> > It's with the rest of the drm_syncobj UAPI docs ;-)
>
> Is that
> https://www.kernel.org/doc/html/latest/driver-api/dma-buf.html#dma-fence-uabi-sync-file
> ?
>
> That whole page never mentions e.g. WAIT_AVAILABLE, so at least the
> flags are not there. Does not mention syncobj_wait either.
probably this:
https://docs.kernel.org/gpu/drm-mm.html
the new one needs to be added there as well.
>
> I could ask where the real non-IGT userspace is or the plan for it,
> too, since this is new DRM UAPI.
yeap, it looks like we need to close on this...
https://gitlab.freedesktop.org/drm/intel/-/issues/8014
I confess I got lost on the many discussions and on how this will
be used. Is mesa going to set the deadline based on the vk priority?
Will this continue to be internal? I didn't get the broader picture
I'm afraid...
>
>
> Thanks,
> pq
>
> >
> > BR,
> > -R
> >
> > > btw. no nsec/sec anymore.
> > >
> > >
> > > Thanks,
> > > pq
> > >
> > >
> > > > struct drm_syncobj_wait {
> > > > __u64 handles;
> > > > /* absolute timeout */
> > > > @@ -895,6 +896,8 @@ struct drm_syncobj_wait {
> > > > __u32 flags;
> > > > __u32 first_signaled; /* only valid when not waiting all */
> > > > __u32 pad;
> > > > + /* Deadline hint to set on backing fence(s) in CLOCK_MONOTONIC: */
> > > > + __u64 deadline_ns;
> > > > };
> > > >
> > > > struct drm_syncobj_timeline_wait {
> > > > @@ -907,6 +910,8 @@ struct drm_syncobj_timeline_wait {
> > > > __u32 flags;
> > > > __u32 first_signaled; /* only valid when not waiting all */
> > > > __u32 pad;
> > > > + /* Deadline hint to set on backing fence(s) in CLOCK_MONOTONIC: */
> > > > + __u64 deadline_ns;
> > > > };
> > > >
> > > >
> > >
>
Re: [Freedreno] [Intel-gfx] [PATCH v6 07/10] drm/i915/hdcp: Use HDCP helpers for i915
+Suraj
On Tue, Jan 31, 2023 at 12:16:44PM -0500, Rodrigo Vivi wrote:
>
> +Suraj who is also working on HDCP related code that even can conflict
> wit this.
>
> On Wed, Jan 18, 2023 at 07:30:12PM +, Mark Yacoub wrote:
> > From: Sean Paul
>
> First of all, Sean, please accept my public apologies here. I just noticed
> now that you had pinged me 9 *months* ago!
> I noticed while taking a look to the history to refresh my mind around
> this series.
>
> >
> > Now that all of the HDCP 1.x logic has been migrated to the central HDCP
> > helpers, use it in the i915 driver.
> >
> > The majority of the driver code for HDCP 1.x will live in intel_hdcp.c,
> > however there are a few helper hooks which are connector-specific and
> > need to be partially or fully implemented in the intel_dp_hdcp.c or
> > intel_hdmi.c.
>
> so far so good! we need to do this soon.
>
> >
> > We'll leave most of the HDCP 2.x code alone since we don't have another
> > implementation of HDCP 2.x to use as reference for what should and
> > should not live in the drm helpers. The helper will call the overly
> > general enable/disable/is_capable HDCP 2.x callbacks and leave the
> > interesting stuff for the driver. Once we have another HDCP 2.x
> > implementation, we should do a similar migration.
>
> I believe this part is the part that I start to get lost when
> trying to review it.
>
> Mark told me in irc that it is really hard to split this patch,
> but do we really need to have the hdcp 1.x changes along with the
> 2.x? I start to get lost in the review when I see the changes around
> the hdcp2_capable...
>
> So, it would be really really good if we can split this patch.
>
> >
> > Acked-by: Jani Nikula
> > Signed-off-by: Sean Paul
> > Signed-off-by: Mark Yacoub
>
> Cc: Suraj Kandpal
>
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-8-s...@poorly.run
> > #v1
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-8-s...@poorly.run
> > #v2
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-8-s...@poorly.run
> > #v3
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-8-s...@poorly.run
> > #v4
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-8-s...@poorly.run
> > #v5
> >
> > Changes in v2:
> > -Fix mst helper function pointer reported by 0-day
> > Changes in v3:
> > -Add forward declaration for drm_atomic_state in intel_hdcp.h identified
> > by 0-day
> > Changes in v4:
> > -None
> > Changes in v5:
> > -None
> > Changes in v6:
> > -Rebased.
> >
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 32 +-
> > .../drm/i915/display/intel_display_debugfs.c | 6 +-
> > .../drm/i915/display/intel_display_types.h| 60 +-
> > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 348 +++
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +-
> > drivers/gpu/drm/i915/display/intel_hdcp.c | 952 +++---
> > drivers/gpu/drm/i915/display/intel_hdcp.h | 31 +-
> > drivers/gpu/drm/i915/display/intel_hdmi.c | 270 ++---
> > 8 files changed, 445 insertions(+), 1270 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 69ecf2a3d6c6..a4397f066a3e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -28,6 +28,7 @@
> > #include
> >
> > #include
> > +#include
> > #include
> >
> > #include "i915_drv.h"
> > @@ -2909,6 +2910,10 @@ static void intel_enable_ddi(struct
> > intel_atomic_state *state,
> > const struct intel_crtc_state *crtc_state,
> > const struct drm_connector_state *conn_state)
> > {
> > + struct intel_connector *connector =
> > + to_intel_connector(conn_state->connector);
> > + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > +
> > drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
> >
> > if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> > @@ -2925,12 +2930,10 @@ static void intel_enable_ddi(struct
> > intel_atomic_state *state,
> > else
> > intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
> >
> > -
Re: [Freedreno] [Intel-gfx] [PATCH v6 07/10] drm/i915/hdcp: Use HDCP helpers for i915
DP_AUX_HDCP_V_PRIME(i), part,
> -DRM_HDCP_V_PRIME_PART_LEN);
> - if (ret != DRM_HDCP_V_PRIME_PART_LEN) {
> - drm_dbg_kms(>drm,
> - "Read v'[%d] from DP/AUX failed (%zd)\n", i, ret);
> - return ret >= 0 ? -EIO : ret;
> - }
> - return 0;
> -}
> -
> static
> int intel_dp_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
> enum transcoder cpu_transcoder,
> @@ -252,40 +107,6 @@ int intel_dp_hdcp_toggle_signalling(struct
> intel_digital_port *dig_port,
> return 0;
> }
>
> -static
> -bool intel_dp_hdcp_check_link(struct intel_digital_port *dig_port,
> - struct intel_connector *connector)
> -{
> - struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> - ssize_t ret;
> - u8 bstatus;
> -
> - ret = drm_dp_dpcd_read(_port->dp.aux, DP_AUX_HDCP_BSTATUS,
> -, 1);
> - if (ret != 1) {
> - drm_dbg_kms(>drm,
> - "Read bstatus from DP/AUX failed (%zd)\n", ret);
> - return false;
> - }
> -
> - return !(bstatus & (DP_BSTATUS_LINK_FAILURE | DP_BSTATUS_REAUTH_REQ));
> -}
> -
> -static
> -int intel_dp_hdcp_capable(struct intel_digital_port *dig_port,
> - bool *hdcp_capable)
> -{
> - ssize_t ret;
> - u8 bcaps;
> -
> - ret = intel_dp_hdcp_read_bcaps(dig_port, );
> - if (ret)
> - return ret;
> -
> - *hdcp_capable = bcaps & DP_BCAPS_HDCP_CAPABLE;
> - return 0;
> -}
> -
> struct hdcp2_dp_errata_stream_type {
> u8 msg_id;
> u8 stream_type;
> @@ -628,13 +449,19 @@ int intel_dp_hdcp2_check_link(struct intel_digital_port
> *dig_port,
> return ret;
> }
>
> -static
> -int intel_dp_hdcp2_capable(struct intel_digital_port *dig_port,
> -bool *capable)
> +static int intel_dp_hdcp2_capable(struct drm_connector *drm_connector,
> + bool *capable)
> {
> + struct intel_connector *connector = to_intel_connector(drm_connector);
> + struct intel_digital_port *dig_port =
> + intel_attached_dig_port(connector);
> u8 rx_caps[3];
> int ret;
>
> + ret = intel_hdcp2_capable(drm_connector, capable);
> + if (ret || !capable)
> + return ret;
> +
> *capable = false;
> ret = drm_dp_dpcd_read(_port->dp.aux,
> DP_HDCP_2_2_REG_RX_CAPS_OFFSET,
> @@ -650,22 +477,11 @@ int intel_dp_hdcp2_capable(struct intel_digital_port
> *dig_port,
> }
>
> static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
> - .write_an_aksv = intel_dp_hdcp_write_an_aksv,
> - .read_bksv = intel_dp_hdcp_read_bksv,
Here is the part that I get fully stuck and don't know how to proceed in the
review.
I'd like to get someone with a stronger background in HDCP here, so I hope
Suraj can help here.
When looking to the removed functions I end up in the enable and auth functions
and I see that the sequence in the new drm ones introduced in the patch 04
are totally different from the ones we had in i915. Hence I'm not comfortable
with adding a review on a flow that I'm not totally familiar with.
But let me be clear: I really don't see any blocker in this patch. Even as is.
I believe the smaller patch would help the review, but it is not mandatory.
That sad:
Acked-by: Rodrigo Vivi
> - .read_bstatus = intel_dp_hdcp_read_bstatus,
> - .repeater_present = intel_dp_hdcp_repeater_present,
> - .read_ri_prime = intel_dp_hdcp_read_ri_prime,
> - .read_ksv_ready = intel_dp_hdcp_read_ksv_ready,
> - .read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
> - .read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
> .toggle_signalling = intel_dp_hdcp_toggle_signalling,
> - .check_link = intel_dp_hdcp_check_link,
> - .hdcp_capable = intel_dp_hdcp_capable,
> .write_2_2_msg = intel_dp_hdcp2_write_msg,
> .read_2_2_msg = intel_dp_hdcp2_read_msg,
> .config_stream_type = intel_dp_hdcp2_config_stream_type,
> .check_2_2_link = intel_dp_hdcp2_check_link,
> - .hdcp_2_2_capable = intel_dp_hdcp2_capable,
> .protocol = HDCP_PROTOCOL_DP,
> };
>
> @@ -721,6 +537,46 @@ intel_dp_mst_hdcp_stream_encryption(struct
> intel_connector *connector,
> return 0;
> }
>
> +static int
> +intel_dp_mst_hdcp1_post_encryption(struct drm_connector *drm_connector)
> +{
> + struct intel_connector *connector = to_intel_connector(drm_conn
Re: [Freedreno] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
On Thu, Apr 14, 2022 at 03:58:02PM +, Sean Paul wrote:
> On Tue, Apr 12, 2022 at 09:25:59AM -0400, Rodrigo Vivi wrote:
> > On Mon, Apr 11, 2022 at 08:47:32PM +, Sean Paul wrote:
> > > From: Sean Paul
> > >
> > > This patch updates the connector's property value in 2 cases which were
> > > previously missed:
> > >
> > > 1- Content type changes. The value should revert back to DESIRED from
> > >ENABLED in case the driver must re-authenticate the link due to the
> > >new content type.
> > >
> > > 2- Userspace sets value to DESIRED while ENABLED. In this case, the
> > >value should be reset immediately to ENABLED since the link is
> > >actively being encrypted.
> > >
> > > To accommodate these changes, I've split up the conditionals to make
> > > things a bit more clear (as much as one can with this mess of state).
> > >
> > > Acked-by: Jani Nikula
> > > Reviewed-by: Abhinav Kumar
> > > Signed-off-by: Sean Paul
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
> > > #v1
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
> > > #v2
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
> > > #v3
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
> > > #v4
> > >
> > > Changes in v2:
> > > -None
> > > Changes in v3:
> > > -Fixed indentation issue identified by 0-day
> > > Changes in v4:
> > > -None
> > > Changes in v5:
> > > -None
> > > ---
> > > drivers/gpu/drm/drm_hdcp.c | 26 +-
> > > 1 file changed, 17 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> > > index dd8fa91c51d6..8c851d40cd45 100644
> > > --- a/drivers/gpu/drm/drm_hdcp.c
> > > +++ b/drivers/gpu/drm/drm_hdcp.c
> > > @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
> > > *connector,
> > > return true;
> > >
> > > /*
> > > - * Nothing to do if content type is unchanged and one of:
> > > - * - state didn't change
> > > + * Content type changes require an HDCP disable/enable cycle.
> > > + */
> > > + if (new_conn_state->hdcp_content_type !=
> > > old_conn_state->hdcp_content_type) {
> >
> > shouldn't we add some && ( old_hdcp ==
> > DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
> > here?
>
> Thanks for your reviews Rodrigo.
>
> I don't think so since the content type is changing the current state of old
> content protection is immaterial (ie: if we need to enable HDCP 2.x, the state
> of HDCP 1.x doesn't really matter), we need to re-evaluate whether the current
> level of HDCP is sufficient.
>
> Hopefully that makes sense, but I could be missing something :-)
nope, I think I was just missing that the important is the content type change
as you pointed out in the item number 1 in the commit msg.
Reviewed-by: Rodrigo Vivi
>
> Sean
>
> >
> > > + new_conn_state->content_protection =
> > > + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > > + return true;
> > > + }
> > > +
> > > + /*
> > > + * Ignore meaningless state changes:
> > >* - HDCP was activated since the last commit
> > > - * - attempting to set to desired while already enabled
> > > + * - Attempting to set to desired while already enabled
> > >*/
> > > - if (old_hdcp == new_hdcp ||
> > > - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > > + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > >new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> > > (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > >new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> > > - if (old_conn_state->hdcp_content_type ==
> > > - new_conn_state->hdcp_content_type)
> > > - return false;
> > > + new_conn_state->content_protection =
> > > + DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > > + return false;
> > > }
> > >
> > > - return true;
> > > + /* Finally, if state changes, we need action */
> > > + return old_hdcp != new_hdcp;
> > > }
> > > EXPORT_SYMBOL(drm_hdcp_atomic_check);
> > > --
> > > Sean Paul, Software Engineer, Google / Chromium OS
> > >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
Re: [Freedreno] [PATCH v5 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers
On Mon, Apr 11, 2022 at 08:47:29PM +, Sean Paul wrote: > From: Sean Paul > > Rebased set from November. Fixed a nit from Stephen in the msm patch and > moved hdcp registers into the trogdor dtsi file to avoid differences > with sc7180-based windows devices. The set is 4 patches lighter since > some of the changes were accepted into msm. > > I'm still waiting for Intel review of the first 7 patches. Rodrigo/Jani, > would you please provide your input so we can move forward with this > set? I'm a bit concerned with patches 4 and 7. It is hard to map the removals and additions and there are some changes that looks like changing behaviors, but end up not being clear in the big patch. Also with big patch it is prune to the rebasing and backport conflicts. Would be possible to split some work in moving individual functions from i915 to drm little by little with smaller patches? But thank you for this great work. It is also good to align our drm drivers. Thanks, Rodrigo. > > Thanks, > > Sean > > Link: https://patchwork.freedesktop.org/series/94623/ #v1 > Link: https://patchwork.freedesktop.org/series/94713/ #v2 > Link: https://patchwork.freedesktop.org/series/94712/ #v3 > Link: https://patchwork.freedesktop.org/series/94712/ #v4 > > Sean Paul (10): > drm/hdcp: Add drm_hdcp_atomic_check() > drm/hdcp: Avoid changing crtc state in hdcp atomic check > drm/hdcp: Update property value on content type and user changes > drm/hdcp: Expand HDCP helper library for enable/disable/check > drm/i915/hdcp: Consolidate HDCP setup/state cache > drm/i915/hdcp: Retain hdcp_capable return codes > drm/i915/hdcp: Use HDCP helpers for i915 > dt-bindings: msm/dp: Add bindings for HDCP registers > arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller > drm/msm: Implement HDCP 1.x using the new drm HDCP helpers > > .../bindings/display/msm/dp-controller.yaml |7 +- > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi |8 + > arch/arm64/boot/dts/qcom/sc7180.dtsi |6 +- > drivers/gpu/drm/drm_hdcp.c| 1197 - > drivers/gpu/drm/i915/display/intel_atomic.c |7 +- > drivers/gpu/drm/i915/display/intel_ddi.c | 29 +- > .../drm/i915/display/intel_display_debugfs.c | 11 +- > .../drm/i915/display/intel_display_types.h| 58 +- > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 345 ++--- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 17 +- > drivers/gpu/drm/i915/display/intel_hdcp.c | 1011 +++--- > drivers/gpu/drm/i915/display/intel_hdcp.h | 36 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 256 ++-- > drivers/gpu/drm/msm/Makefile |1 + > drivers/gpu/drm/msm/dp/dp_debug.c | 46 +- > drivers/gpu/drm/msm/dp/dp_debug.h |6 +- > drivers/gpu/drm/msm/dp/dp_display.c | 46 +- > drivers/gpu/drm/msm/dp/dp_display.h |5 + > drivers/gpu/drm/msm/dp/dp_drm.c | 68 +- > drivers/gpu/drm/msm/dp/dp_drm.h |5 + > drivers/gpu/drm/msm/dp/dp_hdcp.c | 453 +++ > drivers/gpu/drm/msm/dp/dp_hdcp.h | 27 + > drivers/gpu/drm/msm/dp/dp_parser.c| 20 +- > drivers/gpu/drm/msm/dp/dp_parser.h|4 + > drivers/gpu/drm/msm/dp/dp_reg.h | 32 +- > drivers/gpu/drm/msm/msm_atomic.c | 15 + > include/drm/drm_hdcp.h| 194 +++ > 27 files changed, 2582 insertions(+), 1328 deletions(-) > create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.h > > -- > Sean Paul, Software Engineer, Google / Chromium OS >
Re: [Freedreno] [PATCH v5 05/10] drm/i915/hdcp: Consolidate HDCP setup/state cache
On Mon, Apr 11, 2022 at 08:47:34PM +, Sean Paul wrote:
> From: Sean Paul
>
> Stick all of the setup for HDCP into a dedicated function. No functional
> change, but this will facilitate moving HDCP logic into helpers.
Reviewed-by: Rodrigo Vivi
>
> Acked-by: Jani Nikula
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-6-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-6-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-6-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-6-s...@poorly.run
> #v4
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
> drivers/gpu/drm/i915/display/intel_hdcp.c | 52 +++
> 1 file changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 861c550b5bd6..6bb5a3971ed9 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2167,6 +2167,37 @@ static enum mei_fw_tc intel_get_mei_fw_tc(enum
> transcoder cpu_transcoder)
> }
> }
>
> +static int
> +_intel_hdcp_setup(struct intel_connector *connector,
> + const struct intel_crtc_state *pipe_config, u8 content_type)
> +{
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> + struct intel_hdcp *hdcp = >hdcp;
> + int ret = 0;
> +
> + if (!connector->encoder) {
> + drm_err(_priv->drm, "[%s:%d] encoder is not initialized\n",
> + connector->base.name, connector->base.base.id);
> + return -ENODEV;
> + }
> +
> + hdcp->content_type = content_type;
> +
> + if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) {
> + hdcp->cpu_transcoder = pipe_config->mst_master_transcoder;
> + hdcp->stream_transcoder = pipe_config->cpu_transcoder;
> + } else {
> + hdcp->cpu_transcoder = pipe_config->cpu_transcoder;
> + hdcp->stream_transcoder = INVALID_TRANSCODER;
> + }
> +
> + if (DISPLAY_VER(dev_priv) >= 12)
> + dig_port->hdcp_port_data.fw_tc =
> intel_get_mei_fw_tc(hdcp->cpu_transcoder);
> +
> + return ret;
> +}
> +
> static int initialize_hdcp_port_data(struct intel_connector *connector,
>struct intel_digital_port *dig_port,
>const struct intel_hdcp_shim *shim)
> @@ -2306,28 +2337,14 @@ int intel_hdcp_enable(struct intel_connector
> *connector,
> if (!hdcp->shim)
> return -ENOENT;
>
> - if (!connector->encoder) {
> - drm_err(_priv->drm, "[%s:%d] encoder is not initialized\n",
> - connector->base.name, connector->base.base.id);
> - return -ENODEV;
> - }
> -
> mutex_lock(>mutex);
> mutex_lock(_port->hdcp_mutex);
> drm_WARN_ON(_priv->drm,
> hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
> - hdcp->content_type = content_type;
> -
> - if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) {
> - hdcp->cpu_transcoder = pipe_config->mst_master_transcoder;
> - hdcp->stream_transcoder = pipe_config->cpu_transcoder;
> - } else {
> - hdcp->cpu_transcoder = pipe_config->cpu_transcoder;
> - hdcp->stream_transcoder = INVALID_TRANSCODER;
> - }
>
> - if (DISPLAY_VER(dev_priv) >= 12)
> - dig_port->hdcp_port_data.fw_tc =
> intel_get_mei_fw_tc(hdcp->cpu_transcoder);
> + ret = _intel_hdcp_setup(connector, pipe_config, content_type);
> + if (ret)
> + goto out;
>
> /*
>* Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
> @@ -2355,6 +2372,7 @@ int intel_hdcp_enable(struct intel_connector *connector,
> true);
> }
>
> +out:
> mutex_unlock(_port->hdcp_mutex);
> mutex_unlock(>mutex);
> return ret;
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>
Re: [Freedreno] [PATCH v5 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
On Mon, Apr 11, 2022 at 08:47:35PM +, Sean Paul wrote:
> From: Sean Paul
>
> The shim functions return error codes, but they are discarded in
> intel_hdcp.c. This patch plumbs the return codes through so they are
> properly handled.
Reviewed-by: Rodrigo Vivi
>
> Acked-by: Jani Nikula
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-7-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-7-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-7-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-7-s...@poorly.run
> #v4
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
> .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++-
> drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> 3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 452d773fd4e3..f18b4bec4dd4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -489,6 +489,7 @@ static void intel_panel_info(struct seq_file *m,
> static void intel_hdcp_info(struct seq_file *m,
> struct intel_connector *intel_connector)
> {
> + int ret;
> bool hdcp_cap, hdcp2_cap;
>
> if (!intel_connector->hdcp.shim) {
> @@ -496,8 +497,12 @@ static void intel_hdcp_info(struct seq_file *m,
> goto out;
> }
>
> - hdcp_cap = intel_hdcp_capable(intel_connector);
> - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> + ret = intel_hdcp_capable(intel_connector, _cap);
> + if (ret)
> + hdcp_cap = false;
> + ret = intel_hdcp2_capable(intel_connector, _cap);
> + if (ret)
> + hdcp2_cap = false;
>
> if (hdcp_cap)
> seq_puts(m, "HDCP1.4 ");
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 6bb5a3971ed9..771e94fa8dff 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -154,50 +154,49 @@ int intel_hdcp_read_valid_bksv(struct
> intel_digital_port *dig_port,
> }
>
> /* Is HDCP1.4 capable on Platform and Sink */
> -bool intel_hdcp_capable(struct intel_connector *connector)
> +int intel_hdcp_capable(struct intel_connector *connector, bool *capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> - bool capable = false;
> u8 bksv[5];
>
> + *capable = false;
> +
> if (!shim)
> - return capable;
> + return 0;
>
> - if (shim->hdcp_capable) {
> - shim->hdcp_capable(dig_port, );
> - } else {
> - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> - capable = true;
> - }
> + if (shim->hdcp_capable)
> + return shim->hdcp_capable(dig_port, capable);
> +
> + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> + *capable = true;
>
> - return capable;
> + return 0;
> }
>
> /* Is HDCP2.2 capable on Platform and Sink */
> -bool intel_hdcp2_capable(struct intel_connector *connector)
> +int intel_hdcp2_capable(struct intel_connector *connector, bool *capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = >hdcp;
> - bool capable = false;
> +
> + *capable = false;
>
> /* I915 support for HDCP2.2 */
> if (!hdcp->hdcp2_supported)
> - return false;
> + return 0;
>
> /* MEI interface is solid */
> mutex_lock(_priv->hdcp_comp_mutex);
> if (!dev_priv->hdcp_comp_added || !dev_priv->hdcp_master) {
> mutex_unlock(_priv->hdcp_comp_mutex);
> - return false;
> + return 0;
> }
> mutex_unlock(_priv->hdcp_comp_mutex);
>
> /* Sink's capability for HDCP2.2 */
> - hdcp->shim->hdcp_2_2_capable(dig_port, );
> -
>
Re: [Freedreno] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
On Mon, Apr 11, 2022 at 08:47:32PM +, Sean Paul wrote:
> From: Sean Paul
>
> This patch updates the connector's property value in 2 cases which were
> previously missed:
>
> 1- Content type changes. The value should revert back to DESIRED from
>ENABLED in case the driver must re-authenticate the link due to the
>new content type.
>
> 2- Userspace sets value to DESIRED while ENABLED. In this case, the
>value should be reset immediately to ENABLED since the link is
>actively being encrypted.
>
> To accommodate these changes, I've split up the conditionals to make
> things a bit more clear (as much as one can with this mess of state).
>
> Acked-by: Jani Nikula
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
> #v4
>
> Changes in v2:
> -None
> Changes in v3:
> -Fixed indentation issue identified by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
> drivers/gpu/drm/drm_hdcp.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index dd8fa91c51d6..8c851d40cd45 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
> *connector,
> return true;
>
> /*
> - * Nothing to do if content type is unchanged and one of:
> - * - state didn't change
> + * Content type changes require an HDCP disable/enable cycle.
> + */
> + if (new_conn_state->hdcp_content_type !=
> old_conn_state->hdcp_content_type) {
shouldn't we add some && ( old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
here?
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return true;
> + }
> +
> + /*
> + * Ignore meaningless state changes:
>* - HDCP was activated since the last commit
> - * - attempting to set to desired while already enabled
> + * - Attempting to set to desired while already enabled
>*/
> - if (old_hdcp == new_hdcp ||
> - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
>new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
>new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> - if (old_conn_state->hdcp_content_type ==
> - new_conn_state->hdcp_content_type)
> - return false;
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_ENABLED;
> + return false;
> }
>
> - return true;
> + /* Finally, if state changes, we need action */
> + return old_hdcp != new_hdcp;
> }
> EXPORT_SYMBOL(drm_hdcp_atomic_check);
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>
Re: [Freedreno] [PATCH v5 02/10] drm/hdcp: Avoid changing crtc state in hdcp atomic check
On Mon, Apr 11, 2022 at 08:47:31PM +, Sean Paul wrote:
> From: Sean Paul
>
> Instead of forcing a modeset in the hdcp atomic check, simply return
> true if the content protection value is changing and let the driver
> decide whether a modeset is required or not.
>
> Acked-by: Jani Nikula
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-3-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-3-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-3-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-3-s...@poorly.run
> #v4
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
> drivers/gpu/drm/drm_hdcp.c | 33 +++--
> drivers/gpu/drm/i915/display/intel_atomic.c | 5 ++--
> include/drm/drm_hdcp.h | 2 +-
> 3 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index 522326b03e66..dd8fa91c51d6 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -430,11 +430,14 @@ EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> * @connector: drm_connector on which content protection state needs an
> update
> *
> * This function can be used by display drivers to perform an atomic check
> on the
> - * hdcp state elements. If hdcp state has changed, this function will set
> - * mode_changed on the crtc driving the connector so it can update its
> hardware
> - * to match the hdcp state.
> + * hdcp state elements. If hdcp state has changed in a manner which requires
> the
> + * driver to enable or disable content protection, this function will return
> + * true.
> + *
> + * Returns:
> + * true if the driver must enable/disable hdcp, false otherwise
> */
> -void drm_hdcp_atomic_check(struct drm_connector *connector,
> +bool drm_hdcp_atomic_check(struct drm_connector *connector,
> struct drm_atomic_state *state)
> {
> struct drm_connector_state *new_conn_state, *old_conn_state;
> @@ -452,10 +455,12 @@ void drm_hdcp_atomic_check(struct drm_connector
> *connector,
>* If the connector is being disabled with CP enabled, mark it
>* desired so it's re-enabled when the connector is brought back
>*/
> - if (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + if (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> new_conn_state->content_protection =
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> - return;
> + return true;
> + }
> + return false;
> }
>
> new_crtc_state = drm_atomic_get_new_crtc_state(state,
> @@ -467,9 +472,19 @@ void drm_hdcp_atomic_check(struct drm_connector
> *connector,
> */
> if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> - new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> + new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED)) {
> new_conn_state->content_protection =
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return true;
> + }
> +
> + /*
> + * Coming back from disable or changing CRTC with DESIRED state requires
> + * that the driver try CP enable.
> + */
> + if (new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + new_conn_state->crtc != old_conn_state->crtc)
> + return true;
I'm with the feeling that this chunk should deserve a separated patch.
But the reason looks correct so anyway
Reviewed-by: Rodrigo Vivi
>
> /*
>* Nothing to do if content type is unchanged and one of:
> @@ -484,9 +499,9 @@ void drm_hdcp_atomic_check(struct drm_connector
> *connector,
>new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> if (old_conn_state->hdcp_content_type ==
> new_conn_state->hdcp_content_type)
> - return;
> + return false;
> }
>
> - new_crtc_state->mode_changed = true;
> + return true;
> }
> EXPORT_SYMBOL(drm_hdcp_atomic_check);
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
Re: [Freedreno] [PATCH v5 01/10] drm/hdcp: Add drm_hdcp_atomic_check()
On Mon, Apr 11, 2022 at 08:47:30PM +, Sean Paul wrote:
> From: Sean Paul
>
> This patch moves the hdcp atomic check from i915 to drm_hdcp so other
> drivers can use it. No functional changes, just cleaned up some of the
> code when moving it over.
Reviewed-by: Rodrigo Vivi
>
> Acked-by: Jani Nikula
> Acked-by: Jani Nikula
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-2-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-2-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-2-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-2-s...@poorly.run
> #v4
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
> drivers/gpu/drm/drm_hdcp.c | 71 -
> drivers/gpu/drm/i915/display/intel_atomic.c | 4 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 47 --
> drivers/gpu/drm/i915/display/intel_hdcp.h | 3 -
> include/drm/drm_hdcp.h | 3 +
> 5 files changed, 75 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index ca9b8f697202..522326b03e66 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -13,13 +13,14 @@
> #include
> #include
>
> +#include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> -#include
>
> #include "drm_internal.h"
>
> @@ -421,3 +422,71 @@ void drm_hdcp_update_content_protection(struct
> drm_connector *connector,
>dev->mode_config.content_protection_property);
> }
> EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> +
> +/**
> + * drm_hdcp_atomic_check - Helper for drivers to call during
> connector->atomic_check
> + *
> + * @state: pointer to the atomic state being checked
> + * @connector: drm_connector on which content protection state needs an
> update
> + *
> + * This function can be used by display drivers to perform an atomic check
> on the
> + * hdcp state elements. If hdcp state has changed, this function will set
> + * mode_changed on the crtc driving the connector so it can update its
> hardware
> + * to match the hdcp state.
> + */
> +void drm_hdcp_atomic_check(struct drm_connector *connector,
> +struct drm_atomic_state *state)
> +{
> + struct drm_connector_state *new_conn_state, *old_conn_state;
> + struct drm_crtc_state *new_crtc_state;
> + u64 old_hdcp, new_hdcp;
> +
> + old_conn_state = drm_atomic_get_old_connector_state(state, connector);
> + old_hdcp = old_conn_state->content_protection;
> +
> + new_conn_state = drm_atomic_get_new_connector_state(state, connector);
> + new_hdcp = new_conn_state->content_protection;
> +
> + if (!new_conn_state->crtc) {
> + /*
> + * If the connector is being disabled with CP enabled, mark it
> + * desired so it's re-enabled when the connector is brought back
> + */
> + if (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return;
> + }
> +
> + new_crtc_state = drm_atomic_get_new_crtc_state(state,
> +new_conn_state->crtc);
> + /*
> + * Fix the HDCP uapi content protection state in case of modeset.
> + * FIXME: As per HDCP content protection property uapi doc, an uevent()
> + * need to be sent if there is transition from ENABLED->DESIRED.
> + */
> + if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> + new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +
> + /*
> + * Nothing to do if content type is unchanged and one of:
> + * - state didn't change
> + * - HDCP was activated since the last commit
> + * - attempting to set to desired while already enabled
> + */
> + if (old_hdcp == new_hdcp ||
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + new_hdcp == DRM_MODE_CONTENT_
Re: [Freedreno] [Intel-gfx] [PATCH 19/30] drm/dp: Pass drm_dp_aux to drm_dp_link_train_clock_recovery_delay()
On Fri, Feb 19, 2021 at 04:53:15PM -0500, Lyude Paul wrote:
> So that we can start using drm_dbg_*() in
> drm_dp_link_train_clock_recovery_delay().
>
> Signed-off-by: Lyude Paul
I wonder if we could have a drm_dp so we encapsulate both aux and dpcd
related information...
But this one already solves the issue...
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +-
> drivers/gpu/drm/drm_dp_helper.c | 3 ++-
> drivers/gpu/drm/i915/display/intel_dp_link_training.c | 2 +-
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
> drivers/gpu/drm/msm/edp/edp_ctrl.c| 2 +-
> drivers/gpu/drm/radeon/atombios_dp.c | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> include/drm/drm_dp_helper.h | 4 +++-
> 8 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> index 6d35da65e09f..4468f9d6b4dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> @@ -611,7 +611,7 @@ amdgpu_atombios_dp_link_train_cr(struct
> amdgpu_atombios_dp_link_train_info *dp_i
> dp_info->tries = 0;
> voltage = 0xff;
> while (1) {
> - drm_dp_link_train_clock_recovery_delay(dp_info->dpcd);
> + drm_dp_link_train_clock_recovery_delay(dp_info->aux,
> dp_info->dpcd);
>
> if (drm_dp_dpcd_read_link_status(dp_info->aux,
>dp_info->link_status) <= 0) {
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 194e0c273809..ce08eb3bface 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -132,7 +132,8 @@ u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZ
> }
> EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
>
> -void drm_dp_link_train_clock_recovery_delay(const u8
> dpcd[DP_RECEIVER_CAP_SIZE])
> +void drm_dp_link_train_clock_recovery_delay(const struct drm_dp_aux *aux,
> + const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> {
> unsigned long rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
>DP_TRAINING_AUX_RD_MASK;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 892d7db7d94f..222073d46bdb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -441,7 +441,7 @@ static void
> intel_dp_link_training_clock_recovery_delay(struct intel_dp *intel_d
> enum drm_dp_phy dp_phy)
> {
> if (dp_phy == DP_PHY_DPRX)
> - drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd);
> + drm_dp_link_train_clock_recovery_delay(_dp->aux,
> intel_dp->dpcd);
> else
> drm_dp_lttpr_link_train_clock_recovery_delay();
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 36b39c381b3f..2501a6b326a3 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1103,7 +1103,7 @@ static int dp_ctrl_link_train_1(struct dp_ctrl_private
> *ctrl,
> tries = 0;
> old_v_level = ctrl->link->phy_params.v_level;
> for (tries = 0; tries < maximum_retries; tries++) {
> - drm_dp_link_train_clock_recovery_delay(ctrl->panel->dpcd);
> + drm_dp_link_train_clock_recovery_delay(ctrl->aux,
> ctrl->panel->dpcd);
>
> ret = dp_ctrl_read_link_status(ctrl, link_status);
> if (ret)
> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> index 57af3d8b6699..6501598448b4 100644
> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> @@ -608,7 +608,7 @@ static int edp_start_link_train_1(struct edp_ctrl *ctrl)
> tries = 0;
> old_v_level = ctrl->v_level;
> while (1) {
> - drm_dp_link_train_clock_recovery_delay(ctrl->dpcd);
> + drm_dp_link_train_clock_recovery_delay(ctrl->drm_aux,
> ctrl->dpcd);
>
> rlen = drm_dp_dpcd_read_link_status(ctrl->drm_aux, link_status);
> if (rlen < DP_LINK_STATUS_SIZE) {
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index
Re: [Freedreno] [PATCH 06/14] drm: extract drm_atomic_uapi.c
On Mon, Sep 03, 2018 at 06:54:31PM +0200, Daniel Vetter wrote:
> This leaves all the commit/check and state handling in drm_atomic.c,
> while pulling all the uapi glue and the huge ioctl itself into a
> seprate file.
>
> This seems to almost perfectly split the rather big drm_atomic.c file
> into 2 equal sizes.
>
> Also adjust the kerneldoc and type a very terse overview text.
>
> v2: Rebase.
>
> v3: Fix tiny typo.
>
> Signed-off-by: Daniel Vetter
> Cc: David Airlie
> Cc: Gustavo Padovan
> Cc: Maarten Lankhorst
> Cc: Sean Paul
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Rob Clark
> Cc: Eric Anholt
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> ---
> Documentation/gpu/drm-kms.rst| 11 +-
> drivers/gpu/drm/Makefile |3 +-
> drivers/gpu/drm/drm_atomic.c | 1359 +
> drivers/gpu/drm/drm_atomic_helper.c |1 +
> drivers/gpu/drm/drm_atomic_uapi.c| 1393 ++
> drivers/gpu/drm/drm_crtc_helper.c|1 +
> drivers/gpu/drm/drm_crtc_internal.h |5 +
> drivers/gpu/drm/drm_framebuffer.c|1 +
> drivers/gpu/drm/drm_gem_framebuffer_helper.c |1 +
> drivers/gpu/drm/drm_plane_helper.c |1 +
> drivers/gpu/drm/i915/intel_display.c |1 +
> drivers/gpu/drm/msm/msm_atomic.c |2 +
> drivers/gpu/drm/vc4/vc4_crtc.c |1 +
> drivers/gpu/drm/vc4/vc4_plane.c |1 +
> include/drm/drm_atomic.h | 16 -
> include/drm/drm_atomic_uapi.h| 58 +
> 16 files changed, 1480 insertions(+), 1375 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_atomic_uapi.c
> create mode 100644 include/drm/drm_atomic_uapi.h
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 3a9dd68b97c9..4b1501b4835b 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -287,6 +287,15 @@ Atomic Mode Setting Function Reference
> .. kernel-doc:: drivers/gpu/drm/drm_atomic.c
> :export:
>
> +Atomic Mode Setting IOCTL and UAPI Functions
> +
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
> + :doc: overview
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
> + :export:
> +
> CRTC Abstraction
>
>
> @@ -563,7 +572,7 @@ Tile Group Property
> Explicit Fencing Properties
> ---
>
> -.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
> +.. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
> :doc: explicit fencing properties
>
> Existing KMS Properties
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index a6771cef85e2..bc6a16a3c36e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -18,7 +18,8 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
> drm_encoder.o drm_mode_object.o drm_property.o \
> drm_plane.o drm_color_mgmt.o drm_print.o \
> drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> - drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o
> + drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> + drm_atomic_uapi.o
>
> drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 29a4e6959358..19634e03b78e 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -28,6 +28,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -309,285 +310,6 @@ drm_atomic_get_crtc_state(struct drm_atomic_state
> *state,
> }
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> -static void set_out_fence_for_crtc(struct drm_atomic_state *state,
> -struct drm_crtc *crtc, s32 __user *fence_ptr)
> -{
> - state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
> -}
> -
> -static s32 __user *get_out_fence_for_crtc(struct drm_atomic_state *state,
> - struct drm_crtc *crtc)
> -{
> - s32 __user *fence_ptr;
> -
> - fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
> - state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
> -
> - return fence_ptr;
> -}
> -
> -static int set_out_fence_for_connector(struct drm_atomic_state *state,
> -
