Re: [PATCH v14 4/9] drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
On 2/29/2024 10:09 PM, Mitul Golani wrote:
Add the necessary structures and functions to handle reading and
unpacking Adaptive Sync Secondary Data Packets. Also add support
to write and pack AS SDP.
--v2:
- Correct use of REG_BIT and REG_GENMASK. [Jani]
- Use as_sdp instead of async. [Jani]
- Remove unrelated comments and changes. [Jani]
- Correct code indent. [Jani]
--v3:
- Update definition names for AS SDP which are starting from
HSW, as these defines are applicable for ADLP+.(Ankit)
--v4:
- Remove as_sdp_mode from crtc_state.
- Drop metadata keyword.
- For consistency, update ADL_ prefix or post fix as required.
--v5:
- Check if AS_SDP bit is set in crtc_state->infoframes.enable. If not
return.
- Check for HAS_AS_SDP() before setting VIDEO_DIP_ENABLE_AS_ADL mask.
Signed-off-by: Mitul Golani
---
.../drm/i915/display/intel_display_device.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 91 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 12 ++-
drivers/gpu/drm/i915/i915_reg.h | 8 ++
include/drm/display/drm_dp_helper.h | 2 +-
5 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
b/drivers/gpu/drm/i915/display/intel_display_device.h
index fe4268813786..6399fbc6c738 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -68,6 +68,7 @@ struct drm_printer;
#define HAS_TRANSCODER(i915, trans)
((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
BIT(trans)) != 0)
#define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
+#define HAS_AS_SDP(i915) (DISPLAY_VER(i915) >= 13)
#define INTEL_NUM_PIPES(i915)
(hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
#define I915_HAS_HOTPLUG(i915)
(DISPLAY_INFO(i915)->has_hotplug)
#define OVERLAY_NEEDS_PHYSICAL(i915)
(DISPLAY_INFO(i915)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index e13121dc3a03..7cf849015797 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4089,6 +4089,32 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state
*crtc_state,
return false;
}
+static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *as_sdp,
+ struct dp_sdp *sdp, size_t size)
+{
+ size_t length = sizeof(struct dp_sdp);
+
+ if (size < length)
+ return -ENOSPC;
+
+ memset(sdp, 0, size);
+
+ /* Prepare AS (Adaptive Sync) SDP Header */
+ sdp->sdp_header.HB0 = 0;
+ sdp->sdp_header.HB1 = as_sdp->sdp_type;
+ sdp->sdp_header.HB2 = 0x02;
+ sdp->sdp_header.HB3 = as_sdp->length;
+
+ /* Fill AS (Adaptive Sync) SDP Payload */
+ sdp->db[0] = as_sdp->mode;
+ sdp->db[1] = as_sdp->vtotal & 0xFF;
+ sdp->db[2] = (as_sdp->vtotal >> 8) & 0xFF;
+ sdp->db[3] = as_sdp->target_rr;
+ sdp->db[4] = (as_sdp->target_rr >> 8) & 0x3;
+
+ return length;
+}
+
static ssize_t
intel_dp_hdr_metadata_infoframe_sdp_pack(struct drm_i915_private *i915,
const struct hdmi_drm_infoframe
*drm_infoframe,
@@ -4188,6 +4214,10 @@ static void intel_write_dp_sdp(struct intel_encoder
*encoder,
_state->infoframes.drm.drm,
,
sizeof(sdp));
break;
+ case DP_SDP_ADAPTIVE_SYNC:
+ len = intel_dp_as_sdp_pack(_state->infoframes.as_sdp, ,
+ sizeof(sdp));
+ break;
default:
MISSING_CASE(type);
return;
@@ -4209,6 +4239,10 @@ void intel_dp_set_infoframes(struct intel_encoder
*encoder,
u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW |
VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK;
+
+ if (HAS_AS_SDP(dev_priv))
+ dip_enable |= VIDEO_DIP_ENABLE_AS_ADL;
+
u32 val = intel_de_read(dev_priv, reg) & ~dip_enable;
/* TODO: Sanitize DSC enabling wrt. intel_dsc_dp_pps_write(). */
@@ -4230,6 +4264,36 @@ void intel_dp_set_infoframes(struct intel_encoder
*encoder,
intel_write_dp_sdp(encoder, crtc_state,
HDMI_PACKET_TYPE_GAMUT_METADATA);
}
+static
+int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *as_sdp,
+ const void *buffer, size_t size)
+{
+ const struct dp_sdp *sdp = buffer;
+
+ if (size < sizeof(struct dp_sdp))
+ return -EINVAL;
+
+ memset(as_sdp, 0, sizeof(*as_sdp));
+
+ if (sdp->sdp_header.HB0 != 0)
+ return
Re: [PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height
[PATCH v6 3/3] Revert "drm/i915: Wait for active retire before i915_active_fini()"
This reverts commit 7a2280e8dcd2f1f436db9631287c0b21cf6a92b0, obsoleted by "drm/i915/vma: Fix UAF on destroy against retire race". Signed-off-by: Janusz Krzysztofik Cc: Nirmoy Das --- drivers/gpu/drm/i915/i915_vma.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index ffe81fe338f7e..00ce9e20deb2d 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1752,8 +1752,6 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, if (vm_ddestroy) i915_vm_resv_put(vma->vm); - /* Wait for async active retire */ - i915_active_wait(>active); i915_active_fini(>active); GEM_WARN_ON(vma->resource); i915_vma_free(vma); -- 2.43.0
[PATCH v6 2/3] drm/i915: Remove extra multi-gt pm-references
There was an attempt to fix an issue of illegal attempts to free a still
active i915 VMA object when parking a GT believed to be idle, reported by
CI on 2-GT Meteor Lake. As a solution, an extra wakeref for a Primary GT
was acquired from i915_gem_do_execbuffer() -- see commit f56fe3e91787
("drm/i915: Fix a VMA UAF for multi-gt platform").
However, that fix occurred insufficient -- the issue was still reported by
CI. That wakeref was released on exit from i915_gem_do_execbuffer(), then
potentially before completion of the request and deactivation of its
associated VMAs. Moreover, CI reports indicated that single-GT platforms
also suffered sporadically from the same race.
Since the issue has now been fixed by a preceding patch "drm/i915/vma: Fix
UAF on destroy against retire race", drop the no longer useful changes
introduced by that insufficient fix.
v3: Also drop the no longer used .wakeref_gt0 field from struct
i915_execbuffer.
v2: Avoid the word "revert" in commit message (Rodrigo),
- update commit description reusing relevant chunks dropped from the
description of the proper fix (Rodrigo).
Signed-off-by: Janusz Krzysztofik
Cc: Nirmoy Das
Cc: Rodrigo Vivi
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d3a771afb083e..3f20fe3811999 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -255,7 +255,6 @@ struct i915_execbuffer {
struct intel_context *context; /* logical state for the request */
struct i915_gem_context *gem_context; /** caller's context */
intel_wakeref_t wakeref;
- intel_wakeref_t wakeref_gt0;
/** our requests to build */
struct i915_request *requests[MAX_ENGINE_INSTANCE + 1];
@@ -2686,7 +2685,6 @@ static int
eb_select_engine(struct i915_execbuffer *eb)
{
struct intel_context *ce, *child;
- struct intel_gt *gt;
unsigned int idx;
int err;
@@ -2710,17 +2708,10 @@ eb_select_engine(struct i915_execbuffer *eb)
}
}
eb->num_batches = ce->parallel.number_children + 1;
- gt = ce->engine->gt;
for_each_child(ce, child)
intel_context_get(child);
eb->wakeref = intel_gt_pm_get(ce->engine->gt);
- /*
-* Keep GT0 active on MTL so that i915_vma_parked() doesn't
-* free VMAs while execbuf ioctl is validating VMAs.
-*/
- if (gt->info.id)
- eb->wakeref_gt0 = intel_gt_pm_get(to_gt(gt->i915));
if (!test_bit(CONTEXT_ALLOC_BIT, >flags)) {
err = intel_context_alloc_state(ce);
@@ -2759,9 +2750,6 @@ eb_select_engine(struct i915_execbuffer *eb)
return err;
err:
- if (gt->info.id)
- intel_gt_pm_put(to_gt(gt->i915), eb->wakeref_gt0);
-
intel_gt_pm_put(ce->engine->gt, eb->wakeref);
for_each_child(ce, child)
intel_context_put(child);
@@ -2775,12 +2763,6 @@ eb_put_engine(struct i915_execbuffer *eb)
struct intel_context *child;
i915_vm_put(eb->context->vm);
- /*
-* This works in conjunction with eb_select_engine() to prevent
-* i915_vma_parked() from interfering while execbuf validates vmas.
-*/
- if (eb->gt->info.id)
- intel_gt_pm_put(to_gt(eb->gt->i915), eb->wakeref_gt0);
intel_gt_pm_put(eb->context->engine->gt, eb->wakeref);
for_each_child(eb->context, child)
intel_context_put(child);
--
2.43.0
[PATCH v6 1/3] drm/i915/vma: Fix UAF on destroy against retire race
Object debugging tools were sporadically reporting illegal attempts to
free a still active i915 VMA object when parking a GT believed to be idle.
[161.359441] ODEBUG: free active (active state 0) object: 88811643b958
object type: i915_active hint: __i915_vma_active+0x0/0x50 [i915]
[161.360082] WARNING: CPU: 5 PID: 276 at lib/debugobjects.c:514
debug_print_object+0x80/0xb0
...
[161.360304] CPU: 5 PID: 276 Comm: kworker/5:2 Not tainted
6.5.0-rc1-CI_DRM_13375-g003f860e5577+ #1
[161.360314] Hardware name: Intel Corporation Rocket Lake Client
Platform/RocketLake S UDIMM 6L RVP, BIOS RKLSFWI1.R00.3173.A03.2204210138
04/21/2022
[161.360322] Workqueue: i915-unordered __intel_wakeref_put_work [i915]
[161.360592] RIP: 0010:debug_print_object+0x80/0xb0
...
[161.361347] debug_object_free+0xeb/0x110
[161.361362] i915_active_fini+0x14/0x130 [i915]
[161.361866] release_references+0xfe/0x1f0 [i915]
[161.362543] i915_vma_parked+0x1db/0x380 [i915]
[161.363129] __gt_park+0x121/0x230 [i915]
[161.363515] intel_wakeref_put_last+0x1f/0x70 [i915]
That has been tracked down to be happening when another thread is
deactivating the VMA inside __active_retire() helper, after the VMA's
active counter has been already decremented to 0, but before deactivation
of the VMA's object is reported to the object debugging tool.
We could prevent from that race by serializing i915_active_fini() with
__active_retire() via ref->tree_lock, but that wouldn't stop the VMA from
being used, e.g. from __i915_vma_retire() called at the end of
__active_retire(), after that VMA has been already freed by a concurrent
i915_vma_destroy() on return from the i915_active_fini(). Then, we should
rather fix the issue at the VMA level, not in i915_active.
Since __i915_vma_parked() is called from __gt_park() on last put of the
GT's wakeref, the issue could be addressed by holding the GT wakeref long
enough for __active_retire() to complete before that wakeref is released
and the GT parked.
I believe the issue was introduced by commit d93939730347 ("drm/i915:
Remove the vma refcount") which moved a call to i915_active_fini() from
a dropped i915_vma_release(), called on last put of the removed VMA kref,
to i915_vma_parked() processing path called on last put of a GT wakeref.
However, its visibility to the object debugging tool was suppressed by a
bug in i915_active that was fixed two weeks later with commit e92eb246feb9
("drm/i915/active: Fix missing debug object activation").
A VMA associated with a request doesn't acquire a GT wakeref by itself.
Instead, it depends on a wakeref held directly by the request's active
intel_context for a GT associated with its VM, and indirectly on that
intel_context's engine wakeref if the engine belongs to the same GT as the
VMA's VM. Those wakerefs are released asynchronously to VMA deactivation.
Fix the issue by getting a wakeref for the VMA's GT when activating it,
and putting that wakeref only after the VMA is deactivated. However,
exclude global GTT from that processing path, otherwise the GPU never goes
idle. Since __i915_vma_retire() may be called from atomic contexts, use
async variant of wakeref put. Also, to avoid circular locking dependency,
take care of acquiring the wakeref before VM mutex when both are needed.
v6: Since __i915_vma_active/retire() callbacks are not serialized, storing
a wakeref tracking handle inside struct i915_vma is not safe, and
there is no other good place for that. Use untracked variants of
intel_gt_pm_get/put_async().
v5: Replace "tile" with "GT" across commit description (Rodrigo),
- avoid mentioning multi-GT case in commit description (Rodrigo),
- explain why we need to take a temporary wakeref unconditionally inside
i915_vma_pin_ww() (Rodrigo).
v4: Refresh on top of commit 5e4e06e4087e ("drm/i915: Track gt pm
wakerefs") (Andi),
- for more easy backporting, split out removal of former insufficient
workarounds and move them to separate patches (Nirmoy).
- clean up commit message and description a bit.
v3: Identify root cause more precisely, and a commit to blame,
- identify and drop former workarounds,
- update commit message and description.
v2: Get the wakeref before VM mutex to avoid circular locking dependency,
- drop questionable Fixes: tag.
Fixes: d93939730347 ("drm/i915: Remove the vma refcount")
Closes: https://gitlab.freedesktop.org/drm/intel/issues/8875
Signed-off-by: Janusz Krzysztofik
Cc: Thomas Hellström
Cc: Nirmoy Das
Cc: Andi Shyti
Cc: Rodrigo Vivi
Cc: sta...@vger.kernel.org # v5.19+
---
drivers/gpu/drm/i915/i915_vma.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index d09aad34ba37f..ffe81fe338f7e 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -34,6 +34,7 @@
#include "gt/intel_engine.h"
#include "gt/intel_engine_heartbeat.h"
#include "gt/intel_gt.h"
[PATCH v6 0/3] drm/i915: Fix VMA UAF on destroy against deactivate race
Object debugging tools were sporadically reporting illegal attempts to
free a still active i915 VMA object when parking a GT believed to be idle.
[161.359441] ODEBUG: free active (active state 0) object: 88811643b958
object type: i915_active hint: __i915_vma_active+0x0/0x50 [i915]
[161.360082] WARNING: CPU: 5 PID: 276 at lib/debugobjects.c:514
debug_print_object+0x80/0xb0
...
[161.360304] CPU: 5 PID: 276 Comm: kworker/5:2 Not tainted
6.5.0-rc1-CI_DRM_13375-g003f860e5577+ #1
[161.360314] Hardware name: Intel Corporation Rocket Lake Client
Platform/RocketLake S UDIMM 6L RVP, BIOS RKLSFWI1.R00.3173.A03.2204210138
04/21/2022
[161.360322] Workqueue: i915-unordered __intel_wakeref_put_work [i915]
[161.360592] RIP: 0010:debug_print_object+0x80/0xb0
...
[161.361347] debug_object_free+0xeb/0x110
[161.361362] i915_active_fini+0x14/0x130 [i915]
[161.361866] release_references+0xfe/0x1f0 [i915]
[161.362543] i915_vma_parked+0x1db/0x380 [i915]
[161.363129] __gt_park+0x121/0x230 [i915]
[161.363515] intel_wakeref_put_last+0x1f/0x70 [i915]
That has been tracked down to be happening when another thread is
deactivating the VMA inside __active_retire() helper, after the VMA's
active counter has been already decremented to 0, but before deactivation
of the VMA's object is reported to the object debugging tool.
We could prevent from that race by serializing i915_active_fini() with
__active_retire() via ref->tree_lock, but that wouldn't stop the VMA from
being used, e.g. from __i915_vma_retire() called at the end of
__active_retire(), after that VMA has been already freed by a concurrent
i915_vma_destroy() on return from the i915_active_fini(). Then, we should
rather fix the issue at the VMA level, not in i915_active.
Since __i915_vma_parked() is called from __gt_park() on last put of the
GT's wakeref, the issue could be addressed by holding the GT wakeref long
enough for __active_retire() to complete before that wakeref is released
and the GT parked.
A VMA associated with a request doesn't acquire a GT wakeref by itself.
Instead, it depends on a wakeref held directly by the request's active
intel_context for a GT associated with its VM, and indirectly on that
intel_context's engine wakeref if the engine belongs to the same GT as the
VMA's VM. Those wakerefs are released asynchronously to VMA deactivation.
In case of single-GT platforms, at least one of those wakerefs is usually
held long enough for the request's VMA to be deactivated on time, before
it is destroyed on last put of its VM GT wakeref. However, on multi-GT
platforms, a request may use a VMA from a GT other than the one that hosts
the request's engine, then it is protected only with the intel_context's
VM GT wakeref.
There was an attempt to fix the issue on 2-GT Meteor Lake by acquiring an
extra wakeref for a Primary GT from i915_gem_do_execbuffer() -- see commit
f56fe3e91787 ("drm/i915: Fix a VMA UAF for multi-gt platform"). However,
that fix occurred insufficient -- the issue was still reported by CI.
That wakeref was released on exit from i915_gem_do_execbuffer(), then
potentially before completion of the request and deactivation of its
associated VMAs. Moreover, CI reports indicated that single-GT platforms
also suffered sporadically from the same race.
I believe the issue was introduced by commit d93939730347 ("drm/i915:
Remove the vma refcount") which moved a call to i915_active_fini() from
a dropped i915_vma_release(), called on last put of the removed VMA kref,
to i915_vma_parked() processing path called on last put of a GT wakeref.
However, its visibility to the object debugging tool was suppressed by a
bug in i915_active that was fixed two weeks later with commit e92eb246feb9
("drm/i915/active: Fix missing debug object activation").
Fix the issue by getting a wakeref for the VMA's GT when activating it,
and putting that wakeref only after the VMA is deactivated. However,
exclude global GTT from that processing path, otherwise the GPU never goes
idle. Since __i915_vma_retire() may be called from atomic contexts, use
async variant of wakeref put. Also, to avoid circular locking dependency,
take care of acquiring the wakeref before VM mutex when both are needed.
Having that fixed, stop explicitly acquiring the extra GT0 wakeref from
inside i915_gem_do_execbuffer(), and also drop an extra call to
i915_active_wait(), introduced by commit 7a2280e8dcd2 ("drm/i915: Wait for
active retire before i915_active_fini()") as another insufficient fix for
this UAF race.
v6: Since __i915_vma_active/retire() callbacks are not serialized, storing
a wakeref tracking handle inside struct i915_vma is not safe, and
there is no other good place for that. Use untracked variants of
intel_gt_pm_get/put_async(),
- drop no longer used .wakeref_gt0 field from struct i915_execbuffer.
v5: Replace "tile" with "GT" across commit descriptions (Rodrigo),
- reword commit message and description of patch 2 reusing relevant
chunks moved there
Re: [PATCH v5 05/13] drm/mediatek: Set DRM mode configs accordingly
Re: linux-next: build failure after merge of the kunit-next tree
On Thu, 29 Feb 2024 at 23:07, Shuah Khan wrote:
>
> Hi Stephen,
>
> On 2/28/24 21:26, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the kunit-next tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > In file included from drivers/gpu/drm/tests/drm_buddy_test.c:7:
> > drivers/gpu/drm/tests/drm_buddy_test.c: In function
> > 'drm_test_buddy_alloc_range_bias':
> > drivers/gpu/drm/tests/drm_buddy_test.c:191:40: error: format '%u' expects a
> > matching 'unsigned int' argument [-Werror=format=]
> >191 |"buddy_alloc failed with
> > bias(%x-%x), size=%u, ps=%u\n",
> >|
> > ^~~
> > include/kunit/test.h:597:37: note: in definition of macro '_KUNIT_FAILED'
> >597 | fmt,
> > \
> >| ^~~
> > include/kunit/test.h:662:9: note: in expansion of macro
> > 'KUNIT_UNARY_ASSERTION'
> >662 | KUNIT_UNARY_ASSERTION(test,
> > \
> >| ^
> > include/kunit/test.h:1233:9: note: in expansion of macro
> > 'KUNIT_FALSE_MSG_ASSERTION'
> > 1233 | KUNIT_FALSE_MSG_ASSERTION(test,
> > \
> >| ^
> > drivers/gpu/drm/tests/drm_buddy_test.c:186:17: note: in expansion of macro
> > 'KUNIT_ASSERT_FALSE_MSG'
> >186 | KUNIT_ASSERT_FALSE_MSG(test,
> >| ^~
> > drivers/gpu/drm/tests/drm_buddy_test.c:191:91: note: format string is
> > defined here
> >191 |"buddy_alloc failed with
> > bias(%x-%x), size=%u, ps=%u\n",
> >|
> > ~^
> >|
> >|
> >|
> >unsigned int
> > cc1: all warnings being treated as errors
> >
> > Caused by commit
> >
> >806cb2270237 ("kunit: Annotate _MSG assertion variants with gnu printf
> > specifiers")
> >
>
> Thank you. I did allmodconfig build on kselftest kunit branch to make
> sure all is well, before I pushed the commits.
>
> > interacting with commit
> >
> >c70703320e55 ("drm/tests/drm_buddy: add alloc_range_bias test")
> >
> > from the drm-misc-fixes tree.
> >
> > I have applied the following patch for today (this should probably
> > actually be fixed in the drm-misc-fixes tree).
> >
>
> Danial, David,
>
> I can carry the fix through kselftest kunit if it works
> for all.
I'm happy for this to go in with the KUnit changes if that's the best
way to keep all of the printk formatting fixes together.
-- David
>
> > From: Stephen Rothwell
> > Date: Thu, 29 Feb 2024 15:18:36 +1100
> > Subject: [PATCH] fix up for "drm/tests/drm_buddy: add alloc_range_bias test"
> >
> > Signed-off-by: Stephen Rothwell
> > ---
> > drivers/gpu/drm/tests/drm_buddy_test.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
> > b/drivers/gpu/drm/tests/drm_buddy_test.c
> > index 1e73e3f0d278..369edf587b44 100644
> > --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> > +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> > @@ -188,7 +188,7 @@ static void drm_test_buddy_alloc_range_bias(struct
> > kunit *test)
> > bias_end, size,
> > ps,
> > ,
> >
> > DRM_BUDDY_RANGE_ALLOCATION),
> > -"buddy_alloc failed with bias(%x-%x),
> > size=%u, ps=%u\n",
> > +"buddy_alloc failed with bias(%x-%x),
> > size=%u\n",
> > bias_start, bias_end, size);
> > bias_rem -= size;
> >
>
> thanks,
> -- Shuah
smime.p7s
Description: S/MIME Cryptographic Signature
Re: Making drm_gpuvm work across gpu devices
Hi Thomas, Am 29.02.24 um 18:12 schrieb Thomas Hellström: Hi, Christian. On Thu, 2024-02-29 at 10:41 +0100, Christian König wrote: Am 28.02.24 um 20:51 schrieb Zeng, Oak: The mail wasn’t indent/preface correctly. Manually format it. *From:*Christian König *Sent:* Tuesday, February 27, 2024 1:54 AM *To:* Zeng, Oak ; Danilo Krummrich ; Dave Airlie ; Daniel Vetter ; Felix Kuehling ; jgli...@redhat.com *Cc:* Welty, Brian ; dri-devel@lists.freedesktop.org; intel...@lists.freedesktop.org; Bommu, Krishnaiah ; Ghimiray, Himal Prasad ; thomas.hellst...@linux.intel.com; Vishwanathapura, Niranjana ; Brost, Matthew ; Gupta, saurabhg *Subject:* Re: Making drm_gpuvm work across gpu devices Hi Oak, Am 23.02.24 um 21:12 schrieb Zeng, Oak: Hi Christian, I go back this old email to ask a question. sorry totally missed that one. Quote from your email: “Those ranges can then be used to implement the SVM feature required for higher level APIs and not something you need at the UAPI or even inside the low level kernel memory management.” “SVM is a high level concept of OpenCL, Cuda, ROCm etc.. This should not have any influence on the design of the kernel UAPI.” There are two category of SVM: 1.driver svm allocator: this is implemented in user space, i.g., cudaMallocManaged (cuda) or zeMemAllocShared (L0) or clSVMAlloc(openCL). Intel already have gem_create/vm_bind in xekmd and our umd implemented clSVMAlloc and zeMemAllocShared on top of gem_create/vm_bind. Range A..B of the process address space is mapped into a range C..D of the GPU address space, exactly as you said. 2.system svm allocator: This doesn’t introduce extra driver API for memory allocation. Any valid CPU virtual address can be used directly transparently in a GPU program without any extra driver API call. Quote from kernel Documentation/vm/hmm.hst: “Any application memory region (private anonymous, shared memory, or regular file backed memory) can be used by a device transparently” and “to share the address space by duplicating the CPU page table in the device page table so the same address points to the same physical memory for any valid main memory address in the process address space”. In system svm allocator, we don’t need that A..B C..D mapping. It looks like you were talking of 1). Were you? No, even when you fully mirror the whole address space from a process into the GPU you still need to enable this somehow with an IOCTL. And while enabling this you absolutely should specify to which part of the address space this mirroring applies and where it maps to. */[Zeng, Oak] /* Lets say we have a hardware platform where both CPU and GPU support 57bit(use it for example. The statement apply to any address range) virtual address range, how do you decide “which part of the address space this mirroring applies”? You have to mirror the whole address space [0~2^57-1], do you? As you designed it, the gigantic vm_bind/mirroring happens at the process initialization time, and at that time, you don’t know which part of the address space will be used for gpu program. Remember for system allocator, *any* valid CPU address can be used for GPU program. If you add an offset to [0~2^57-1], you get an address out of 57bit address range. Is this a valid concern? Well you can perfectly mirror on demand. You just need something similar to userfaultfd() for the GPU. This way you don't need to mirror the full address space, but can rather work with large chunks created on demand, let's say 1GiB or something like that. What we're looking at as the current design is an augmented userptr (A..B -> C..D mapping) which is internally sparsely populated in chunks. KMD manages the population using gpu pagefaults. We acknowledge that some parts of this mirror will not have a valid CPU mapping. That is, no vma so a gpu page-fault that resolves to such a mirror address will cause an error. Would you have any concerns / objections against such an approach? Nope, as far as I can see that sounds like a perfectly valid design to me. Regards, Christian. Thanks, Thomas
Re: [PATCH v5 04/13] drm/mediatek: Fix errors when reporting rotation capability
Re: [PATCH v2 0/9] drm: Switch from dev_err to dev_err_probe for missing DSI host error path
Hi Nícolas,
On Thu, Feb 29, 2024 at 07:12:06PM -0500, Nícolas F. R. A. Prado wrote:
> This series changes every occurence of the following pattern:
>
> dsi_host = of_find_mipi_dsi_host_by_node(dsi);
> if (!dsi_host) {
> dev_err(dev, "failed to find dsi host\n");
> return -EPROBE_DEFER;
> }
>
> into
>
> dsi_host = of_find_mipi_dsi_host_by_node(dsi);
> if (!dsi_host)
> return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
> host\n");
>
> This registers the defer probe reason (so it can later be printed by the
> driver core or checked on demand through the devices_deferred file in
> debugfs) and prevents errors to be spammed in the kernel log every time
> the driver retries to probe, unnecessarily alerting userspace about
> something that is a normal part of the boot process.
The idea is good, but I have a small issue with patches 1/9 to 7/9. They
all patch a function that is called by the probe function. Calling
dev_err_probe() in such functions is error-prone. I had to manually
check when reviewing the patches that those functions were indeed called
at probe time, and not through other code paths, and I also had to check
that no callers were using dev_err_probe() in the error handling path,
as that would have overridden the error message.
Would there be a way to move the dev_err_probe() to the top-level ? I
understand it's not always possible or convenient, but if it was doable
in at least some of the drivers, I think it would be better. I'll let
you be the judge.
> I have omitted a Fixes: tag in the last patch, for the truly-nt35597
> panel, because it predates the dev_err_probe() helper.
>
> Changes in v2:
> - Added patches 2 onwards to fix all occurences of this pattern instead
> of just for the anx7625 driver
> - Link to v1:
> https://lore.kernel.org/r/20240226-anx7625-defer-log-no-dsi-host-v1-1-242b1af31...@collabora.com
>
> ---
> Nícolas F. R. A. Prado (9):
> drm/bridge: anx7625: Don't log an error when DSI host can't be found
> drm/bridge: icn6211: Don't log an error when DSI host can't be found
> drm/bridge: lt8912b: Don't log an error when DSI host can't be found
> drm/bridge: lt9611: Don't log an error when DSI host can't be found
> drm/bridge: lt9611uxc: Don't log an error when DSI host can't be found
> drm/bridge: tc358775: Don't log an error when DSI host can't be found
> drm/bridge: dpc3433: Don't log an error when DSI host can't be found
> drm/panel: novatek-nt35950: Don't log an error when DSI host can't be
> found
> drm/panel: truly-nt35597: Don't log an error when DSI host can't be
> found
>
> drivers/gpu/drm/bridge/analogix/anx7625.c | 6 ++
> drivers/gpu/drm/bridge/chipone-icn6211.c | 6 ++
> drivers/gpu/drm/bridge/lontium-lt8912b.c | 6 ++
> drivers/gpu/drm/bridge/lontium-lt9611.c | 6 ++
> drivers/gpu/drm/bridge/lontium-lt9611uxc.c| 6 ++
> drivers/gpu/drm/bridge/tc358775.c | 6 ++
> drivers/gpu/drm/bridge/ti-dlpc3433.c | 17 +
> drivers/gpu/drm/panel/panel-novatek-nt35950.c | 6 ++
> drivers/gpu/drm/panel/panel-truly-nt35597.c | 6 ++
> 9 files changed, 25 insertions(+), 40 deletions(-)
> ---
> base-commit: 2ae0a045e6814c8c1d676d6153c605a65746aa29
> change-id: 20240226-anx7625-defer-log-no-dsi-host-c3f9ccbcb287
--
Regards,
Laurent Pinchart
Re: [PATCH v2 9/9] drm/panel: truly-nt35597: Don't log an error when DSI host can't be found
Hi Nícolas,
Thank you for the patch.
On Thu, Feb 29, 2024 at 07:12:15PM -0500, Nícolas F. R. A. Prado wrote:
> Given that failing to find a DSI host causes the driver to defer probe,
> make use of dev_err_probe() to log the reason. This makes the defer
> probe reason available and avoids alerting userspace about something
> that is not necessarily an error.
>
> Suggested-by: AngeloGioacchino Del Regno
>
> Signed-off-by: Nícolas F. R. A. Prado
> ---
> drivers/gpu/drm/panel/panel-truly-nt35597.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c
> b/drivers/gpu/drm/panel/panel-truly-nt35597.c
> index b73448cf349d..d447db912a61 100644
> --- a/drivers/gpu/drm/panel/panel-truly-nt35597.c
> +++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c
> @@ -550,10 +550,8 @@ static int truly_nt35597_probe(struct mipi_dsi_device
> *dsi)
>
> dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
> of_node_put(dsi1);
> - if (!dsi1_host) {
> - dev_err(dev, "failed to find dsi host\n");
> - return -EPROBE_DEFER;
> - }
> + if (!dsi1_host)
> + return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
> host\n");
return dev_err_probe(dev, -EPROBE_DEFER,
"failed to find dsi host\n");
With this addressed,
Reviewed-by: Laurent Pinchart
>
> /* register the second DSI device */
> dsi1_device = mipi_dsi_device_register_full(dsi1_host, );
>
--
Regards,
Laurent Pinchart
Re: [PATCH v2 8/9] drm/panel: novatek-nt35950: Don't log an error when DSI host can't be found
Hi Nicolas,
Thank you for the patch.
On Thu, Feb 29, 2024 at 07:12:14PM -0500, Nícolas F. R. A. Prado wrote:
> Given that failing to find a DSI host causes the driver to defer probe,
> make use of dev_err_probe() to log the reason. This makes the defer
> probe reason available and avoids alerting userspace about something
> that is not necessarily an error.
>
> Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC
> panels")
> Suggested-by: AngeloGioacchino Del Regno
>
> Signed-off-by: Nícolas F. R. A. Prado
> ---
> drivers/gpu/drm/panel/panel-novatek-nt35950.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> index 648ce9201426..028fdac293f7 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> @@ -556,10 +556,8 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
> }
> dsi_r_host = of_find_mipi_dsi_host_by_node(dsi_r);
> of_node_put(dsi_r);
> - if (!dsi_r_host) {
> - dev_err(dev, "Cannot get secondary DSI host\n");
> - return -EPROBE_DEFER;
> - }
> + if (!dsi_r_host)
> + return dev_err_probe(dev, -EPROBE_DEFER, "Cannot get
> secondary DSI host\n");
return dev_err_probe(dev, -EPROBE_DEFER,
"Cannot get secondary DSI host\n");
With this,
Reviewed-by: Laurent Pinchart
>
> nt->dsi[1] = mipi_dsi_device_register_full(dsi_r_host, info);
> if (!nt->dsi[1]) {
>
--
Regards,
Laurent Pinchart
[PATCH] drm/panel: boe-tv101wum-nl6: Fine tune Himax83102-j02 panel HFP and HBP
The current measured frame rate is 59.95Hz, which does not meet the
requirements of touch-stylus and stylus cannot work normally. After
adjustment, the actual measurement is 60.001Hz. Now this panel looks
like it's only used by me on the MTK platform, so let's change this
set of parameters.
Fixes: cea7008190ad ("drm/panel: Fine tune Himax83102-j02 panel HFP and HBP")
Signed-off-by: Cong Yang
---
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index bc08814954f9..0ffe8f8c01de 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1768,11 +1768,11 @@ static const struct panel_desc
starry_qfh032011_53g_desc = {
};
static const struct drm_display_mode starry_himax83102_j02_default_mode = {
- .clock = 162850,
+ .clock = 162680,
.hdisplay = 1200,
- .hsync_start = 1200 + 50,
- .hsync_end = 1200 + 50 + 20,
- .htotal = 1200 + 50 + 20 + 50,
+ .hsync_start = 1200 + 60,
+ .hsync_end = 1200 + 60 + 20,
+ .htotal = 1200 + 60 + 20 + 40,
.vdisplay = 1920,
.vsync_start = 1920 + 116,
.vsync_end = 1920 + 116 + 8,
--
2.25.1
[git pull] drm fixes for 6.8-rc7
Hi Linus, Regular weekly fixes pull, I'll be travelling for a few days but I don't think it should interfere with anything apart from my responsiveness if things go wrong :-P. Bunch of fixes, xe, amdgpu, nouveau and tegra all have a few. Then drm/bridge including some drivers/soc fallout fixes. The biggest thing in here is a new unit test for some buddy allocator fixes, otherwise a misc fbcon, ttm unit test and one msm revert. Seems to pretty normal for this stage. Regards, Dave. drm-fixes-2024-03-01: drm fixes for 6.8-rc7 buddy: - two allocation fixes + unit test fbcon: - font restore syzkaller fix ttm: - kunit test fix bridge: - fix aux-hpd leaks - fix aux-hpd registration - fix use after free in soc/qcom - fix boot on soc/qcom xe: - A couple of tracepoint updates from Priyanka and Lucas. - Make sure BINDs are completed before accepting UNBINDs on LR vms. - Don't arbitrarily restrict max number of batched binds. - Add uapi for dumpable bos (agreed on IRC). - Remove unused uapi flags and a leftover comment. - A couple of fixes related to the execlist backend. msm: - DP: Revert a change which was causing a HDP regression amdgpu: - Fix potential buffer overflow - Fix power min cap - Suspend/resume fix - SI PM fix - eDP fix nouveau: - fix a misreported VRAM sizing - fix a regression in suspend/resume due to freeing tegra: - host1x reset fix - only remove existing driver if display is possible The following changes since commit d206a76d7d2726f3b096037f2079ce0bd3ba329b: Linux 6.8-rc6 (2024-02-25 15:46:06 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/drm/kernel.git tags/drm-fixes-2024-03-01 for you to fetch changes up to f6ecfdad359a01c7fd8a3bcfde3ef0acdf107e6e: drm/nouveau: keep DMA buffers required for suspend/resume (2024-03-01 15:27:04 +1000) drm fixes for 6.8-rc7 buddy: - two allocation fixes + unit test fbcon: - font restore syzkaller fix ttm: - kunit test fix bridge: - fix aux-hpd leaks - fix aux-hpd registration - fix use after free in soc/qcom - fix boot on soc/qcom xe: - A couple of tracepoint updates from Priyanka and Lucas. - Make sure BINDs are completed before accepting UNBINDs on LR vms. - Don't arbitrarily restrict max number of batched binds. - Add uapi for dumpable bos (agreed on IRC). - Remove unused uapi flags and a leftover comment. - A couple of fixes related to the execlist backend. msm: - DP: Revert a change which was causing a HDP regression amdgpu: - Fix potential buffer overflow - Fix power min cap - Suspend/resume fix - SI PM fix - eDP fix nouveau: - fix a misreported VRAM sizing - fix a regression in suspend/resume due to freeing tegra: - host1x reset fix - only remove existing driver if display is possible Alex Deucher (1): Revert "drm/amd/pm: resolve reboot exception for si oland" Arnd Bergmann (1): drm/xe/mmio: fix build warning for BAR resize on 32-bit Christian König (1): drm/ttm/tests: depend on UML || COMPILE_TEST Dave Airlie (5): Merge tag 'drm-misc-fixes-2024-02-29' of https://anongit.freedesktop.org/git/drm/drm-misc into drm-fixes Merge tag 'drm-xe-fixes-2024-02-29' of https://gitlab.freedesktop.org/drm/xe/kernel into drm-fixes Merge tag 'drm-msm-fixes-2024-02-28' of https://gitlab.freedesktop.org/drm/msm into drm-fixes Merge tag 'amd-drm-fixes-6.8-2024-02-29' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes nouveau: report byte usage in VRAM usage. Dmitry Baryshkov (1): Revert "drm/msm/dp: use drm_bridge_hpd_notify() to report HPD status changes" Francois Dugast (1): drm/xe/uapi: Remove unused flags Jiri Slaby (SUSE) (1): fbcon: always restore the old font data in fbcon_do_set_font() Johan Hovold (3): drm/bridge: aux-hpd: fix OF node leaks drm/bridge: aux-hpd: separate allocation and registration soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free José Roberto de Souza (1): drm/xe/uapi: Remove DRM_XE_VM_BIND_FLAG_ASYNC comment left over Lucas De Marchi (1): drm/xe: Use pointers in trace events Ma Jun (1): drm/amdgpu/pm: Fix the power1_min_cap value Maarten Lankhorst (1): drm/xe: Add uapi for dumpable bos Matthew Auld (3): drm/buddy: fix range bias drm/buddy: check range allocation matches alignment drm/tests/drm_buddy: add alloc_range_bias test Matthew Brost (3): drm/xe: Fix execlist splat drm/xe: Don't support execlists in xe_gt_tlb_invalidation layer drm/xe: Use vmalloc for array of bind allocation in bind IOCTL Maxime Ripard (1): Merge drm/drm-fixes into drm-misc-fixes Mika Kuoppala (2): drm/xe: Expose user fence from xe_sync_entry drm/xe: Deny unbinds if uapi ufence pending Mikko Perttunen (1): gpu: host1x: Skip reset assert on Tegra186 Paulo Zanoni (1): drm/xe:
[PATCH v9 8/8] drm: atmel-hlcdc: add LCD controller layer definition for sam9x75
Add the LCD controller layer definition and descriptor structure for
sam9x75 for the following layers:
- Base Layer
- Overlay1 Layer
- Overlay2 Layer
- High End Overlay
Signed-off-by: Manikandan Muralidharan
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 100 +++
1 file changed, 100 insertions(+)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index b09df821cbc0..9ce429f889ca 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -467,6 +467,102 @@ static const struct atmel_hlcdc_dc_desc
atmel_hlcdc_dc_sam9x60 = {
.ops = _hlcdc_ops,
};
+static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x75_layers[] = {
+ {
+ .name = "base",
+ .formats = _hlcdc_plane_rgb_formats,
+ .regs_offset = 0x60,
+ .id = 0,
+ .type = ATMEL_HLCDC_BASE_LAYER,
+ .cfgs_offset = 0x1c,
+ .layout = {
+ .xstride = { 2 },
+ .default_color = 3,
+ .general_config = 4,
+ .disc_pos = 5,
+ .disc_size = 6,
+ },
+ .clut_offset = 0x700,
+ },
+ {
+ .name = "overlay1",
+ .formats = _hlcdc_plane_rgb_formats,
+ .regs_offset = 0x160,
+ .id = 1,
+ .type = ATMEL_HLCDC_OVERLAY_LAYER,
+ .cfgs_offset = 0x1c,
+ .layout = {
+ .pos = 2,
+ .size = 3,
+ .xstride = { 4 },
+ .pstride = { 5 },
+ .default_color = 6,
+ .chroma_key = 7,
+ .chroma_key_mask = 8,
+ .general_config = 9,
+ },
+ .clut_offset = 0xb00,
+ },
+ {
+ .name = "overlay2",
+ .formats = _hlcdc_plane_rgb_formats,
+ .regs_offset = 0x260,
+ .id = 2,
+ .type = ATMEL_HLCDC_OVERLAY_LAYER,
+ .cfgs_offset = 0x1c,
+ .layout = {
+ .pos = 2,
+ .size = 3,
+ .xstride = { 4 },
+ .pstride = { 5 },
+ .default_color = 6,
+ .chroma_key = 7,
+ .chroma_key_mask = 8,
+ .general_config = 9,
+ },
+ .clut_offset = 0xf00,
+ },
+ {
+ .name = "high-end-overlay",
+ .formats = _hlcdc_plane_rgb_and_yuv_formats,
+ .regs_offset = 0x360,
+ .id = 3,
+ .type = ATMEL_HLCDC_OVERLAY_LAYER,
+ .cfgs_offset = 0x30,
+ .layout = {
+ .pos = 2,
+ .size = 3,
+ .memsize = 4,
+ .xstride = { 5, 7 },
+ .pstride = { 6, 8 },
+ .default_color = 9,
+ .chroma_key = 10,
+ .chroma_key_mask = 11,
+ .general_config = 12,
+ .csc = 16,
+ .scaler_config = 23,
+ .vxs_config = 30,
+ .hxs_config = 31,
+ },
+ .clut_offset = 0x1300,
+ },
+};
+
+static const struct atmel_hlcdc_dc_desc atmel_xlcdc_dc_sam9x75 = {
+ .min_width = 0,
+ .min_height = 0,
+ .max_width = 2048,
+ .max_height = 2048,
+ .max_spw = 0x3ff,
+ .max_vpw = 0x3ff,
+ .max_hpw = 0x3ff,
+ .fixed_clksrc = true,
+ .is_xlcdc = true,
+ .nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x75_layers),
+ .layers = atmel_xlcdc_sam9x75_layers,
+ .ops = _xlcdc_ops,
+};
+
static const struct of_device_id atmel_hlcdc_of_match[] = {
{
.compatible = "atmel,at91sam9n12-hlcdc",
@@ -492,6 +588,10 @@ static const struct of_device_id atmel_hlcdc_of_match[] = {
.compatible = "microchip,sam9x60-hlcdc",
.data = _hlcdc_dc_sam9x60,
},
+ {
+ .compatible = "microchip,sam9x75-xlcdc",
+ .data = _xlcdc_dc_sam9x75,
+ },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, atmel_hlcdc_of_match);
--
2.25.1
[PATCH v9 7/8] drm: atmel-hlcdc: add support for DSI output formats
Add support for the following DPI mode if the encoder type
is DSI as per the XLCDC IP datasheet:
- 16BPPCFG1
- 16BPPCFG2
- 16BPPCFG3
- 18BPPCFG1
- 18BPPCFG2
- 24BPP
Signed-off-by: Manikandan Muralidharan
[durai.manicka...@microchip.com: update output format using is_xlcdc flag]
Signed-off-by: Durai Manickam KR
---
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 70 +--
1 file changed, 64 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index fdd3a6bc0f79..0f7ffb3ced20 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -301,11 +301,60 @@ static void atmel_hlcdc_crtc_atomic_enable(struct
drm_crtc *c,
}
-#define ATMEL_HLCDC_RGB444_OUTPUT BIT(0)
-#define ATMEL_HLCDC_RGB565_OUTPUT BIT(1)
-#define ATMEL_HLCDC_RGB666_OUTPUT BIT(2)
-#define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
-#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
+#define ATMEL_HLCDC_RGB444_OUTPUT BIT(0)
+#define ATMEL_HLCDC_RGB565_OUTPUT BIT(1)
+#define ATMEL_HLCDC_RGB666_OUTPUT BIT(2)
+#define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
+#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUTBIT(4)
+#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUTBIT(5)
+#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUTBIT(6)
+#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUTBIT(7)
+#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUTBIT(8)
+#define ATMEL_HLCDC_DPI_RGB888_OUTPUT BIT(9)
+#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
+#define ATMEL_XLCDC_OUTPUT_MODE_MASK GENMASK(9, 0)
+
+static int atmel_xlcdc_connector_output_dsi(struct drm_encoder *encoder,
+ struct drm_display_info *info)
+{
+ int j;
+ unsigned int supported_fmts = 0;
+
+ switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+ case 0:
+ break;
+ case MEDIA_BUS_FMT_RGB565_1X16:
+ return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
+ case MEDIA_BUS_FMT_RGB666_1X18:
+ return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
+ case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+ return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
+ default:
+ return -EINVAL;
+ }
+
+ for (j = 0; j < info->num_bus_formats; j++) {
+ switch (info->bus_formats[j]) {
+ case MEDIA_BUS_FMT_RGB565_1X16:
+ supported_fmts |= ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
+ break;
+ case MEDIA_BUS_FMT_RGB666_1X18:
+ supported_fmts |= ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
+ break;
+ case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+ supported_fmts |= ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ supported_fmts |= ATMEL_HLCDC_DPI_RGB888_OUTPUT;
+ break;
+ default:
+ break;
+ }
+ }
+ return supported_fmts;
+}
static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
{
@@ -318,6 +367,13 @@ static int atmel_hlcdc_connector_output_mode(struct
drm_connector_state *state)
encoder = state->best_encoder;
if (!encoder)
encoder = connector->encoder;
+ /*
+* atmel-hlcdc to support DSI formats with DSI video pipeline
+* when DRM_MODE_ENCODER_DSI type is set by
+* connector driver component.
+*/
+ if (encoder->encoder_type == DRM_MODE_ENCODER_DSI)
+ return atmel_xlcdc_connector_output_dsi(encoder, info);
switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
case 0:
@@ -358,7 +414,7 @@ static int atmel_hlcdc_connector_output_mode(struct
drm_connector_state *state)
static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
{
- unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
+ unsigned int output_fmts;
struct atmel_hlcdc_crtc_state *hstate;
struct drm_connector_state *cstate;
struct drm_connector *connector;
@@ -366,6 +422,8 @@ static int atmel_hlcdc_crtc_select_output_mode(struct
drm_crtc_state *state)
int i;
crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
+ output_fmts = crtc->dc->desc->is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
+ ATMEL_HLCDC_OUTPUT_MODE_MASK;
for_each_new_connector_in_state(state->state, connector, cstate, i) {
unsigned int supported_fmts = 0;
--
2.25.1
[PATCH v9 6/8] drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC
Update the vertical and horizontal scaler registers of XLCDC IP
with Bilinear and Bicubic co-efficients taps for Chroma and
Luma componenets of the Pixel.
Signed-off-by: Manikandan Muralidharan
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 4
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 20 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 1153d402f810..e1a0bb24b511 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -198,6 +198,8 @@
* @disc_pos: discard area position register
* @disc_size: discard area size register
* @csc: color space conversion register
+ * @vxs_config: vertical scalar filter taps control register
+ * @hxs_config: horizontal scalar filter taps control register
*/
struct atmel_hlcdc_layer_cfg_layout {
int xstride[ATMEL_HLCDC_LAYER_MAX_PLANES];
@@ -217,6 +219,8 @@ struct atmel_hlcdc_layer_cfg_layout {
int disc_pos;
int disc_size;
int csc;
+ int vxs_config;
+ int hxs_config;
};
/**
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 8a5419ccc343..55ce4b2fcb88 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -966,6 +966,26 @@ static void atmel_xlcdc_csc_init(struct atmel_hlcdc_plane
*plane,
desc->layout.csc + i,
xlcdc_csc_coeffs[i]);
}
+
+ if (desc->layout.vxs_config && desc->layout.hxs_config) {
+ /*
+* Updating vxs.config and hxs.config fixes the
+* Green Color Issue in SAM9X7 EGT Video Player App
+*/
+ atmel_hlcdc_layer_write_cfg(>layer,
+ desc->layout.vxs_config,
+ ATMEL_XLCDC_LAYER_VXSYCFG_ONE |
+ ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE |
+ ATMEL_XLCDC_LAYER_VXSCCFG_ONE |
+ ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE);
+
+ atmel_hlcdc_layer_write_cfg(>layer,
+ desc->layout.hxs_config,
+ ATMEL_XLCDC_LAYER_HXSYCFG_ONE |
+ ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE |
+ ATMEL_XLCDC_LAYER_HXSCCFG_ONE |
+ ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE);
+ }
}
static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
--
2.25.1
[PATCH v9 1/8] drm: atmel-hlcdc: add driver ops to differentiate HLCDC and XLCDC IP
Add LCD IP specific ops in driver data to differentiate
HLCDC and XLCDC code within the atmel-hlcdc driver files.
XLCDC in SAM9X7 has different sets of registers and additional
configuration bits when compared to previous HLCDC IP. Read/write
operation on the controller register and functionality is now
separated using the LCD IP specific ops.
Signed-off-by: Manikandan Muralidharan
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 5 +
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 83 ++---
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 167 +++---
3 files changed, 172 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 84c54e8622d1..b09df821cbc0 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -58,6 +58,7 @@ static const struct atmel_hlcdc_dc_desc
atmel_hlcdc_dc_at91sam9n12 = {
.conflicting_output_formats = true,
.nlayers = ARRAY_SIZE(atmel_hlcdc_at91sam9n12_layers),
.layers = atmel_hlcdc_at91sam9n12_layers,
+ .ops = _hlcdc_ops,
};
static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
@@ -151,6 +152,7 @@ static const struct atmel_hlcdc_dc_desc
atmel_hlcdc_dc_at91sam9x5 = {
.conflicting_output_formats = true,
.nlayers = ARRAY_SIZE(atmel_hlcdc_at91sam9x5_layers),
.layers = atmel_hlcdc_at91sam9x5_layers,
+ .ops = _hlcdc_ops,
};
static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
@@ -269,6 +271,7 @@ static const struct atmel_hlcdc_dc_desc
atmel_hlcdc_dc_sama5d3 = {
.conflicting_output_formats = true,
.nlayers = ARRAY_SIZE(atmel_hlcdc_sama5d3_layers),
.layers = atmel_hlcdc_sama5d3_layers,
+ .ops = _hlcdc_ops,
};
static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
@@ -364,6 +367,7 @@ static const struct atmel_hlcdc_dc_desc
atmel_hlcdc_dc_sama5d4 = {
.max_hpw = 0x3ff,
.nlayers = ARRAY_SIZE(atmel_hlcdc_sama5d4_layers),
.layers = atmel_hlcdc_sama5d4_layers,
+ .ops = _hlcdc_ops,
};
static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sam9x60_layers[] = {
@@ -460,6 +464,7 @@ static const struct atmel_hlcdc_dc_desc
atmel_hlcdc_dc_sam9x60 = {
.fixed_clksrc = true,
.nlayers = ARRAY_SIZE(atmel_hlcdc_sam9x60_layers),
.layers = atmel_hlcdc_sam9x60_layers,
+ .ops = _hlcdc_ops,
};
static const struct of_device_id atmel_hlcdc_of_match[] = {
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 5b5c774e0edf..d0ecf0f58cce 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -288,6 +288,63 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
return container_of(layer, struct atmel_hlcdc_plane, layer);
}
+/**
+ * struct atmel_hlcdc_dc - Atmel HLCDC Display Controller.
+ * @desc: HLCDC Display Controller description
+ * @dscrpool: DMA coherent pool used to allocate DMA descriptors
+ * @hlcdc: pointer to the atmel_hlcdc structure provided by the MFD device
+ * @crtc: CRTC provided by the display controller
+ * @layers: active HLCDC layers
+ * @suspend: used to store the HLCDC state when entering suspend
+ * @suspend.imr: used to read/write LCDC Interrupt Mask Register
+ * @suspend.state: Atomic commit structure
+ */
+struct atmel_hlcdc_dc {
+ const struct atmel_hlcdc_dc_desc *desc;
+ struct dma_pool *dscrpool;
+ struct atmel_hlcdc *hlcdc;
+ struct drm_crtc *crtc;
+ struct atmel_hlcdc_layer *layers[ATMEL_HLCDC_MAX_LAYERS];
+ struct {
+ u32 imr;
+ struct drm_atomic_state *state;
+ } suspend;
+};
+
+struct atmel_hlcdc_plane_state;
+
+/**
+ * struct atmel_lcdc_dc_ops - describes atmel_lcdc ops group
+ * to differentiate HLCDC and XLCDC IP code support
+ * @plane_setup_scaler: update the vertical and horizontal scaling factors
+ * @update_lcdc_buffers: update the each LCDC layers DMA registers
+ * @lcdc_atomic_disable: disable LCDC interrupts and layers
+ * @lcdc_update_general_settings: update each LCDC layers general
+ * configuration register
+ * @lcdc_atomic_update: enable the LCDC layers and interrupts
+ * @lcdc_csc_init: update the color space conversion co-efficient of
+ * High-end overlay register
+ * @lcdc_irq_dbg: to raise alert incase of interrupt overrun in any LCDC layer
+ */
+struct atmel_lcdc_dc_ops {
+ void (*plane_setup_scaler)(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_plane_state *state);
+ void (*lcdc_update_buffers)(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_plane_state *state,
+ u32 sr, int i);
+ void (*lcdc_atomic_disable)(struct
[PATCH v9 5/8] drm: atmel-hlcdc: add DPI mode support for XLCDC
Add support for Display Pixel Interface (DPI) Compatible Mode
support in atmel-hlcdc driver for XLCDC IP along with legacy
pixel mapping. DPI mode BIT is configured in LCDC_CFG5 register.
Signed-off-by: Manikandan Muralidharan
[durai.manicka...@microchip.com: update DPI mode bit using is_xlcdc flag]
Signed-off-by: Durai Manickam KR
---
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 21 +--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 98a98b5fca85..fdd3a6bc0f79 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -30,10 +30,12 @@
*
* @base: base CRTC state
* @output_mode: RGBXXX output mode
+ * @dpi: output DPI mode
*/
struct atmel_hlcdc_crtc_state {
struct drm_crtc_state base;
unsigned int output_mode;
+ u8 dpi;
};
static inline struct atmel_hlcdc_crtc_state *
@@ -170,6 +172,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc
*c)
if (adj->flags & DRM_MODE_FLAG_NHSYNC)
cfg |= ATMEL_HLCDC_HSPOL;
+ } else {
+ cfg |= state->dpi << 11;
}
regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
@@ -177,7 +181,9 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc
*c)
ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
ATMEL_HLCDC_DISPPOL | ATMEL_HLCDC_DISPDLY |
ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
- ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
+ ATMEL_HLCDC_GUARDTIME_MASK |
+ (crtc->dc->desc->is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
+ ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
cfg);
clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
@@ -380,7 +386,15 @@ static int atmel_hlcdc_crtc_select_output_mode(struct
drm_crtc_state *state)
hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
hstate->output_mode = fls(output_fmts) - 1;
-
+ if (crtc->dc->desc->is_xlcdc) {
+ /* check if MIPI DPI bit needs to be set */
+ if (fls(output_fmts) > 3) {
+ hstate->output_mode -= 4;
+ hstate->dpi = 1;
+ } else {
+ hstate->dpi = 0;
+ }
+ }
return 0;
}
@@ -484,6 +498,7 @@ static struct drm_crtc_state *
atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
{
struct atmel_hlcdc_crtc_state *state, *cur;
+ struct atmel_hlcdc_crtc *c = drm_crtc_to_atmel_hlcdc_crtc(crtc);
if (WARN_ON(!crtc->state))
return NULL;
@@ -495,6 +510,8 @@ atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
cur = drm_crtc_state_to_atmel_hlcdc_crtc_state(crtc->state);
state->output_mode = cur->output_mode;
+ if (c->dc->desc->is_xlcdc)
+ state->dpi = cur->dpi;
return >base;
}
--
2.25.1
[PATCH v9 4/8] drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops
Add XLCDC specific driver ops and is_xlcdc flag to separate the
functionality and to access the controller registers.
HEO scaling, window resampling, Alpha blending, YUV-to-RGB
conversion in XLCDC is derived and handled using additional
configuration bits and registers. Writing one to the Enable fields
of each layer in LCD_ATTRE is required to reflect the values set
in Configuration, FBA, Enable registers of each layer.
Signed-off-by: Manikandan Muralidharan
Co-developed-by: Hari Prasath Gujulan Elango
Signed-off-by: Hari Prasath Gujulan Elango
Co-developed-by: Durai Manickam KR
Signed-off-by: Durai Manickam KR
---
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 37 +++-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 3 +
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 182 +-
3 files changed, 217 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index b229692a27ca..98a98b5fca85 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -164,11 +164,13 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct
drm_crtc *c)
state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
cfg = state->output_mode << 8;
- if (adj->flags & DRM_MODE_FLAG_NVSYNC)
- cfg |= ATMEL_HLCDC_VSPOL;
+ if (!crtc->dc->desc->is_xlcdc) {
+ if (adj->flags & DRM_MODE_FLAG_NVSYNC)
+ cfg |= ATMEL_HLCDC_VSPOL;
- if (adj->flags & DRM_MODE_FLAG_NHSYNC)
- cfg |= ATMEL_HLCDC_HSPOL;
+ if (adj->flags & DRM_MODE_FLAG_NHSYNC)
+ cfg |= ATMEL_HLCDC_HSPOL;
+ }
regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
ATMEL_HLCDC_HSPOL | ATMEL_HLCDC_VSPOL |
@@ -202,6 +204,20 @@ static void atmel_hlcdc_crtc_atomic_disable(struct
drm_crtc *c,
pm_runtime_get_sync(dev->dev);
+ if (crtc->dc->desc->is_xlcdc) {
+ regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
+ if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+!(status & ATMEL_XLCDC_CM),
+10, 1000))
+ dev_warn(dev->dev, "Atmel LCDC status register CMSTS
timeout\n");
+
+ regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
+ if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+status & ATMEL_XLCDC_SD,
+10, 1000))
+ dev_warn(dev->dev, "Atmel LCDC status register SDSTS
timeout\n");
+ }
+
regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
!(status & ATMEL_HLCDC_DISP),
@@ -261,6 +277,19 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc
*c,
10, 1000))
dev_warn(dev->dev, "Atmel LCDC status register DISPSTS
timeout\n");
+ if (crtc->dc->desc->is_xlcdc) {
+ regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
+ if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+status & ATMEL_XLCDC_CM,
+10, 1000))
+ dev_warn(dev->dev, "Atmel LCDC status register CMSTS
timeout\n");
+
+ regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
+ if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+!(status & ATMEL_XLCDC_SD),
+10, 1000))
+ dev_warn(dev->dev, "Atmel LCDC status register SDSTS
timeout\n");
+ }
pm_runtime_put_sync(dev->dev);
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index c32e5c8809d7..1153d402f810 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -386,6 +386,7 @@ struct atmel_lcdc_dc_ops {
};
extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
+extern const struct atmel_lcdc_dc_ops atmel_xlcdc_ops;
/**
* Atmel HLCDC Display Controller description structure.
@@ -403,6 +404,7 @@ extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
* @conflicting_output_formats: true if RGBXXX output formats conflict with
* each other.
* @fixed_clksrc: true if clock source is fixed
+ * @is_xlcdc: true if XLCDC IP is supported
* @layers: a layer description table describing available layers
* @nlayers: layer description table size
* @ops: atmel lcdc dc ops
@@ -417,6 +419,7 @@ struct atmel_hlcdc_dc_desc
[PATCH v9 2/8] drm: atmel-hlcdc: Define XLCDC specific registers
From: Durai Manickam KR The register address of the XLCDC IP used in SAM9X7 SoC family are different from the previous HLCDC. Defining those address space with valid macros. Signed-off-by: Durai Manickam KR [manikanda...@microchip.com: Remove unused macro definitions] Signed-off-by: Manikandan Muralidharan --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 42 include/linux/mfd/atmel-hlcdc.h | 10 + 2 files changed, 52 insertions(+) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index d0ecf0f58cce..c32e5c8809d7 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -15,6 +15,7 @@ #include +/* LCD controller common registers */ #define ATMEL_HLCDC_LAYER_CHER 0x0 #define ATMEL_HLCDC_LAYER_CHDR 0x4 #define ATMEL_HLCDC_LAYER_CHSR 0x8 @@ -128,6 +129,47 @@ #define ATMEL_HLCDC_MAX_LAYERS 6 +/* XLCDC controller specific registers */ +#define ATMEL_XLCDC_LAYER_ENR 0x10 +#define ATMEL_XLCDC_LAYER_EN BIT(0) + +#define ATMEL_XLCDC_LAYER_IER 0x0 +#define ATMEL_XLCDC_LAYER_IDR 0x4 +#define ATMEL_XLCDC_LAYER_ISR 0xc +#define ATMEL_XLCDC_LAYER_OVR_IRQ(p) BIT(2 + (8 * (p))) + +#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)(((p) * 0x4) + 0x18) + +#define ATMEL_XLCDC_LAYER_DMA_CFG 0 + +#define ATMEL_XLCDC_LAYER_DMA BIT(0) +#define ATMEL_XLCDC_LAYER_REP BIT(1) +#define ATMEL_XLCDC_LAYER_DISCEN BIT(4) + +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS(4 << 6) +#define ATMEL_XLCDC_LAYER_SFACTA_ONE BIT(9) +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS (6 << 11) +#define ATMEL_XLCDC_LAYER_DFACTA_ONE BIT(14) + +#define ATMEL_XLCDC_LAYER_A0_SHIFT 16 +#define ATMEL_XLCDC_LAYER_A0(x)\ + ((x) << ATMEL_XLCDC_LAYER_A0_SHIFT) + +#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE BIT(0) +#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLEBIT(1) +#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE BIT(4) +#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLEBIT(5) + +#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE BIT(0) +#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE BIT(4) +#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE BIT(16) +#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE BIT(20) + +#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE BIT(0) +#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE BIT(4) +#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE BIT(16) +#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE BIT(20) + /** * Atmel HLCDC Layer registers layout structure * diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h index a186119a49b5..80d675a03b39 100644 --- a/include/linux/mfd/atmel-hlcdc.h +++ b/include/linux/mfd/atmel-hlcdc.h @@ -22,6 +22,8 @@ #define ATMEL_HLCDC_DITHER BIT(6) #define ATMEL_HLCDC_DISPDLYBIT(7) #define ATMEL_HLCDC_MODE_MASK GENMASK(9, 8) +#define ATMEL_XLCDC_MODE_MASK GENMASK(10, 8) +#define ATMEL_XLCDC_DPIBIT(11) #define ATMEL_HLCDC_PP BIT(10) #define ATMEL_HLCDC_VSPSU BIT(12) #define ATMEL_HLCDC_VSPHO BIT(13) @@ -34,6 +36,12 @@ #define ATMEL_HLCDC_IDR0x30 #define ATMEL_HLCDC_IMR0x34 #define ATMEL_HLCDC_ISR0x38 +#define ATMEL_XLCDC_ATTRE 0x3c + +#define ATMEL_XLCDC_BASE_UPDATEBIT(0) +#define ATMEL_XLCDC_OVR1_UPDATEBIT(1) +#define ATMEL_XLCDC_OVR3_UPDATEBIT(2) +#define ATMEL_XLCDC_HEO_UPDATE BIT(3) #define ATMEL_HLCDC_CLKPOL BIT(0) #define ATMEL_HLCDC_CLKSEL BIT(2) @@ -48,6 +56,8 @@ #define ATMEL_HLCDC_DISP BIT(2) #define ATMEL_HLCDC_PWMBIT(3) #define ATMEL_HLCDC_SIPBIT(4) +#define ATMEL_XLCDC_SD BIT(5) +#define ATMEL_XLCDC_CM BIT(6) #define ATMEL_HLCDC_SOFBIT(0) #define ATMEL_HLCDC_SYNCDISBIT(1) -- 2.25.1
[PATCH v9 3/8] drm: atmel_hlcdc: replace regmap_read with regmap_read_poll_timeout
Replace regmap_read with regmap_read_poll_timeout to neatly handle retries Signed-off-by: Manikandan Muralidharan --- .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 44 +++ 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index cc5cf4c2faf7..b229692a27ca 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -203,19 +203,22 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c, pm_runtime_get_sync(dev->dev); regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP); - while (!regmap_read(regmap, ATMEL_HLCDC_SR, ) && - (status & ATMEL_HLCDC_DISP)) - cpu_relax(); + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status, +!(status & ATMEL_HLCDC_DISP), + 10, 1000)) + dev_warn(dev->dev, "Atmel LCDC status register DISPSTS timeout\n"); regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_SYNC); - while (!regmap_read(regmap, ATMEL_HLCDC_SR, ) && - (status & ATMEL_HLCDC_SYNC)) - cpu_relax(); + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status, +!(status & ATMEL_HLCDC_SYNC), + 10, 1000)) + dev_warn(dev->dev, "Atmel LCDC status register LCDSTS timeout\n"); regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_PIXEL_CLK); - while (!regmap_read(regmap, ATMEL_HLCDC_SR, ) && - (status & ATMEL_HLCDC_PIXEL_CLK)) - cpu_relax(); + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status, +!(status & ATMEL_HLCDC_PIXEL_CLK), + 10, 1000)) + dev_warn(dev->dev, "Atmel LCDC status register CLKSTS timeout\n"); clk_disable_unprepare(crtc->dc->hlcdc->sys_clk); pinctrl_pm_select_sleep_state(dev->dev); @@ -241,20 +244,23 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c, clk_prepare_enable(crtc->dc->hlcdc->sys_clk); regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_PIXEL_CLK); - while (!regmap_read(regmap, ATMEL_HLCDC_SR, ) && - !(status & ATMEL_HLCDC_PIXEL_CLK)) - cpu_relax(); - + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status, +status & ATMEL_HLCDC_PIXEL_CLK, +10, 1000)) + dev_warn(dev->dev, "Atmel LCDC status register CLKSTS timeout\n"); regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_SYNC); - while (!regmap_read(regmap, ATMEL_HLCDC_SR, ) && - !(status & ATMEL_HLCDC_SYNC)) - cpu_relax(); + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status, +status & ATMEL_HLCDC_SYNC, +10, 1000)) + dev_warn(dev->dev, "Atmel LCDC status register LCDSTS timeout\n"); regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_DISP); - while (!regmap_read(regmap, ATMEL_HLCDC_SR, ) && - !(status & ATMEL_HLCDC_DISP)) - cpu_relax(); + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status, +status & ATMEL_HLCDC_DISP, +10, 1000)) + dev_warn(dev->dev, "Atmel LCDC status register DISPSTS timeout\n"); + pm_runtime_put_sync(dev->dev); -- 2.25.1
[PATCH v9 0/8] Add support for XLCDC to sam9x7 SoC family.
This patch series aims to add support for XLCDC IP of sam9x7 SoC family to the DRM subsystem.XLCDC IP has additional registers and new configuration bits compared to the existing register set of HLCDC IP. The new compatible string "microchip,sam9x75-xlcdc" is defined for sam9x75 variant of the sam9x7 SoC family.The is_xlcdc flag under driver data and IP specific driver ops helps to differentiate the XLCDC and existing HLCDC code within the same driver. changes in v9: * Fix struct comments as per kernel-doc format * Rename LCDC ops * Move regmap_read_poll_timeout change to separate commit * cosmetic fixes changes in v8: * Re-arrange the patch set to prepare and update the current HLCDC code base with the new LCDC IP based driver ops and then add support for XLCDC code changes. * Fix Cosmetic issues. changes in v7: * LCDC IP driver ops functions are declared static and its declaration are removed. changes in v6: * Fixed Cosmetic defects. * Added comments for readability. changes in v5: * return value of regmap_read_poll_timeout is checked in failure case. * HLCDC and XLCDC specific driver functions are now invoked using its IP specific driver ops w/o the need of checking is_xlcdc flag. * Removed empty spaces and blank lines. changes in v4: * fixed kernel warnings reported by kernel test robot. changes in v3: * Removed de-referencing the value of is_xlcdc flag multiple times in a single function. * Removed cpu_relax() call when using regmap_read_poll_timeout. * Updated xfactor and yfactor equations using shift operators * Defined CSC co-efficients in an array for code readability. changes in v2: * Change the driver compatible name from "microchip,sam9x7-xlcdc" to "microchip,sam9x75-xlcdc". * Move is_xlcdc flag to driver data. * Remove unsed Macro definitions. * Add co-developed-bys tags * Replace regmap_read() with regmap_read_poll_timeout() call * Split code into two helpers for code readablitity. --- Durai Manickam KR (1): drm: atmel-hlcdc: Define XLCDC specific registers Manikandan Muralidharan (7): drm: atmel-hlcdc: add driver ops to differentiate HLCDC and XLCDC IP drm: atmel_hlcdc: replace regmap_read with regmap_read_poll_timeout drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops drm: atmel-hlcdc: add DPI mode support for XLCDC drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC drm: atmel-hlcdc: add support for DSI output formats drm: atmel-hlcdc: add LCD controller layer definition for sam9x75 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 172 ++-- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 105 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 132 +-- .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 369 +++--- include/linux/mfd/atmel-hlcdc.h | 10 + 5 files changed, 673 insertions(+), 115 deletions(-) -- 2.25.1
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Thu, Feb 29, 2024 at 08:27:30PM +0200, Andy Shevchenko wrote: On Thu, Feb 29, 2024 at 12:21:34PM -0600, Lucas De Marchi wrote: On Thu, Feb 29, 2024 at 12:49:57PM +0200, Andy Shevchenko wrote: > On Wed, Feb 28, 2024 at 05:39:21PM -0600, Lucas De Marchi wrote: > > On Thu, Feb 22, 2024 at 06:49:59AM -0800, Yury Norov wrote: ... > > I build-tested this in x86-64, x86-32 and arm64. I didn't like much the > > need to fork the __GENMASK() implementation on the 2 sides of the ifdef > > since I think the GENMASK_INPUT_CHECK() should be the one covering the > > input checks. However to make it common we'd need to solve 2 problems: > > the casts and the sizeof. The sizeof can be passed as arg to > > __GENMASK(), however the casts I think would need a __CAST_U8(x) > > or the like and sprinkle it everywhere, which would hurt readability. > > Not pretty. Or go back to the original submission and make it less > > horrible :-/ > > I'm wondering if we can use _Generic() approach here. in assembly? Yes. I added a _Generic() in a random .S and to my surprise the build didn't break. Then I went to implement, and couldn't find where the _Generic() would actually be useful. What I came up with builds for me with gcc on x86-64, x86-32 and arm64. I'm also adding some additional tests in lib/test_bits.c to cover the expected values and types. Thoughts? 8< Subject: [PATCH] bits: introduce fixed-type genmasks Generalize __GENMASK() to support different types, and implement fixed-types versions of GENMASK() based on it. The fixed-type version allows more strict checks to the min/max values accepted, which is useful for defining registers like implemented by i915 and xe drivers with their REG_GENMASK*() macros. The strict checks rely on shift-count-overflow compiler check to fail the build if a number outside of the range allowed is passed. Example: #define FOO_MASK GENMASK_U32(33, 4) will generate a warning like: ../include/linux/bits.h:48:23: warning: right shift count is negative [-Wshift-count-negative] 48 | (~literal(0) >> ((w) - 1 - (h) | ^~ Some additional tests in lib/test_bits.c are added to cover the expected/non-expected values and also that the result value matches the expected type. Since those are known at build time, use static_assert() instead of normal kunit tests. Signed-off-by: Lucas De Marchi --- include/linux/bits.h | 33 +++-- lib/test_bits.c | 21 +++-- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/include/linux/bits.h b/include/linux/bits.h index 7c0cf5031abe8..6f089e71a195c 100644 --- a/include/linux/bits.h +++ b/include/linux/bits.h @@ -22,24 +22,37 @@ #define GENMASK_INPUT_CHECK(h, l) \ (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \ __is_constexpr((l) > (h)), (l) > (h), 0))) +#define __CAST_T(t, v) ((t)v) #else /* * BUILD_BUG_ON_ZERO is not available in h files included from asm files, * disable the input check if that is the case. */ #define GENMASK_INPUT_CHECK(h, l) 0 +#define __CAST_T(t, v) v #endif -#define __GENMASK(h, l) \ - (((~UL(0)) - (UL(1) << (l)) + 1) & \ -(~UL(0) >> (BITS_PER_LONG - 1 - (h -#define GENMASK(h, l) \ - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) +/* + * Generate a mask for a specific type. @literal is the suffix to be used for + * an integer constant of that type and @width is the bits-per-type. Additional + * checks are made to guarantee the value returned fits in that type, relying + * on shift-count-overflow compiler check to detect incompatible arguments. + * For example, all these create build errors or warnings: + * + * - GENMASK(15, 20): wrong argument order + * - GENMASK(72, 15): doesn't fit unsigned long + * - GENMASK_U32(33, 15): doesn't fit in a u32 + */ +#define __GENMASK(literal, w, h, l) \ + (GENMASK_INPUT_CHECK(h, l) + \ +((~literal(0) - (literal(1) << (l)) + 1) & \ +(~literal(0) >> ((w) - 1 - (h) -#define __GENMASK_ULL(h, l) \ - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h -#define GENMASK_ULL(h, l) \ - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) +#define GENMASK(h, l) __GENMASK(UL, BITS_PER_LONG, h, l) +#define GENMASK_ULL(h, l) __GENMASK(ULL, BITS_PER_LONG_LONG, h, l) +#define GENMASK_U8(h, l) __CAST_T(u8, __GENMASK(UL, 8, h, l)) +#define GENMASK_U16(h, l) __CAST_T(u16, __GENMASK(UL, 16, h, l)) +#define GENMASK_U32(h, l) __CAST_T(u32, __GENMASK(UL, 32, h, l)) +#define GENMASK_U64(h, l) __CAST_T(u64, __GENMASK(ULL, 64, h, l)) #endif /* __LINUX_BITS_H */ diff --git a/lib/test_bits.c b/lib/test_bits.c index c9368a2314e7c..e2fc1a1d38702 100644 --- a/lib/test_bits.c +++ b/lib/test_bits.c @@ -5,7 +5,16 @@ #include #include +#include +#define assert_type(t, x) _Generic(x, t: x,
[PATCH 2/2] nouveau/umem: rename nvkm client lock to umem_lock
From: Dave Airlie
This lock is just protecting the umem list so name it as such.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/nouveau/include/nvkm/core/client.h | 2 +-
drivers/gpu/drm/nouveau/nvkm/core/client.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/mmu/umem.c | 12 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/client.h
b/drivers/gpu/drm/nouveau/include/nvkm/core/client.h
index 932c9fd0b2d8..6ca50e864e86 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/core/client.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/core/client.h
@@ -17,7 +17,7 @@ struct nvkm_client {
int (*event)(u64 token, void *argv, u32 argc);
struct list_head umem;
- spinlock_t lock;
+ spinlock_t umem_lock;
};
int nvkm_client_new(const char *name, u64 device, const char *cfg, const char
*dbg,
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/client.c
b/drivers/gpu/drm/nouveau/nvkm/core/client.c
index c55662937ab2..2885795f71d4 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/client.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/client.c
@@ -183,6 +183,6 @@ nvkm_client_new(const char *name, u64 device, const char
*cfg, const char *dbg,
spin_lock_init(>obj_lock);
client->event = event;
INIT_LIST_HEAD(>umem);
- spin_lock_init(>lock);
+ spin_lock_init(>umem_lock);
return 0;
}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/umem.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/umem.c
index e530bb8b3b17..a16cc20b835f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/umem.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/umem.c
@@ -42,14 +42,14 @@ nvkm_umem_search(struct nvkm_client *client, u64 handle)
object = nvkm_object_search(client, handle, _umem);
if (IS_ERR(object)) {
if (client != master) {
- spin_lock(>lock);
+ spin_lock(>umem_lock);
list_for_each_entry(umem, >umem, head) {
if (umem->object.object == handle) {
memory = nvkm_memory_ref(umem->memory);
break;
}
}
- spin_unlock(>lock);
+ spin_unlock(>umem_lock);
}
} else {
umem = nvkm_umem(object);
@@ -124,9 +124,9 @@ static void *
nvkm_umem_dtor(struct nvkm_object *object)
{
struct nvkm_umem *umem = nvkm_umem(object);
- spin_lock(>object.client->lock);
+ spin_lock(>object.client->umem_lock);
list_del_init(>head);
- spin_unlock(>object.client->lock);
+ spin_unlock(>object.client->umem_lock);
nvkm_memory_unref(>memory);
return umem;
}
@@ -179,9 +179,9 @@ nvkm_umem_new(const struct nvkm_oclass *oclass, void *argv,
u32 argc,
if (ret)
return ret;
- spin_lock(>object.client->lock);
+ spin_lock(>object.client->umem_lock);
list_add(>head, >object.client->umem);
- spin_unlock(>object.client->lock);
+ spin_unlock(>object.client->umem_lock);
args->v0.page = nvkm_memory_page(umem->memory);
args->v0.addr = nvkm_memory_addr(umem->memory);
--
2.43.2
[PATCH 1/2] nouveau: lock the client object tree.
From: Dave Airlie It appears the client object tree has no locking unless I've missed something else. Fix races around adding/removing client objects, mostly vram bar mappings. 4562.099306] general protection fault, probably for non-canonical address 0x6677ed422bceb80c: [#1] PREEMPT SMP PTI [ 4562.099314] CPU: 2 PID: 23171 Comm: deqp-vk Not tainted 6.8.0-rc6+ #27 [ 4562.099324] Hardware name: Gigabyte Technology Co., Ltd. Z390 I AORUS PRO WIFI/Z390 I AORUS PRO WIFI-CF, BIOS F8 11/05/2021 [ 4562.099330] RIP: 0010:nvkm_object_search+0x1d/0x70 [nouveau] [ 4562.099503] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 48 89 f8 48 85 f6 74 39 48 8b 87 a0 00 00 00 48 85 c0 74 12 <48> 8b 48 f8 48 39 ce 73 15 48 8b 40 10 48 85 c0 75 ee 48 c7 c0 fe [ 4562.099506] RSP: :a94cc420bbf8 EFLAGS: 00010206 [ 4562.099512] RAX: 6677ed422bceb814 RBX: 98108791f400 RCX: 9810f26b8f58 [ 4562.099517] RDX: RSI: 9810f26b9158 RDI: 98108791f400 [ 4562.099519] RBP: 9810f26b9158 R08: R09: [ 4562.099521] R10: a94cc420bc48 R11: 0001 R12: 9810f02a7cc0 [ 4562.099526] R13: R14: 00ff R15: 0007 [ 4562.099528] FS: 7f629c5017c0() GS:98142c70() knlGS: [ 4562.099534] CS: 0010 DS: ES: CR0: 80050033 [ 4562.099536] CR2: 7f629a882000 CR3: 00017019e004 CR4: 003706f0 [ 4562.099541] DR0: DR1: DR2: [ 4562.099542] DR3: DR6: fffe0ff0 DR7: 0400 [ 4562.099544] Call Trace: [ 4562.099555] [ 4562.099573] ? die_addr+0x36/0x90 [ 4562.099583] ? exc_general_protection+0x246/0x4a0 [ 4562.099593] ? asm_exc_general_protection+0x26/0x30 [ 4562.099600] ? nvkm_object_search+0x1d/0x70 [nouveau] [ 4562.099730] nvkm_ioctl+0xa1/0x250 [nouveau] [ 4562.099861] nvif_object_map_handle+0xc8/0x180 [nouveau] [ 4562.099986] nouveau_ttm_io_mem_reserve+0x122/0x270 [nouveau] [ 4562.100156] ? dma_resv_test_signaled+0x26/0xb0 [ 4562.100163] ttm_bo_vm_fault_reserved+0x97/0x3c0 [ttm] [ 4562.100182] ? __mutex_unlock_slowpath+0x2a/0x270 [ 4562.100189] nouveau_ttm_fault+0x69/0xb0 [nouveau] [ 4562.100356] __do_fault+0x32/0x150 [ 4562.100362] do_fault+0x7c/0x560 [ 4562.100369] __handle_mm_fault+0x800/0xc10 [ 4562.100382] handle_mm_fault+0x17c/0x3e0 [ 4562.100388] do_user_addr_fault+0x208/0x860 [ 4562.100395] exc_page_fault+0x7f/0x200 [ 4562.100402] asm_exc_page_fault+0x26/0x30 [ 4562.100412] RIP: 0033:0x9b9870 [ 4562.100419] Code: 85 a8 f7 ff ff 8b 8d 80 f7 ff ff 89 08 e9 18 f2 ff ff 0f 1f 84 00 00 00 00 00 44 89 32 e9 90 fa ff ff 0f 1f 84 00 00 00 00 00 <44> 89 32 e9 f8 f1 ff ff 0f 1f 84 00 00 00 00 00 66 44 89 32 e9 e7 [ 4562.100422] RSP: 002b:7fff9ba2dc70 EFLAGS: 00010246 [ 4562.100426] RAX: 0004 RBX: 0dd65e10 RCX: 00fff000 [ 4562.100428] RDX: 7f629a882000 RSI: 7f629a882000 RDI: 0066 [ 4562.100432] RBP: 7fff9ba2e570 R08: R09: 000123ddf000 [ 4562.100434] R10: 0001 R11: 0246 R12: 7fff [ 4562.100436] R13: R14: R15: [ 4562.100446] [ 4562.100448] Modules linked in: nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables libcrc32c nfnetlink cmac bnep sunrpc iwlmvm intel_rapl_msr intel_rapl_common snd_sof_pci_intel_cnl x86_pkg_temp_thermal intel_powerclamp snd_sof_intel_hda_common mac80211 coretemp snd_soc_acpi_intel_match kvm_intel snd_soc_acpi snd_soc_hdac_hda snd_sof_pci snd_sof_xtensa_dsp snd_sof_intel_hda_mlink snd_sof_intel_hda snd_sof kvm snd_sof_utils snd_soc_core snd_hda_codec_realtek libarc4 snd_hda_codec_generic snd_compress snd_hda_ext_core vfat fat snd_hda_intel snd_intel_dspcfg irqbypass iwlwifi snd_hda_codec snd_hwdep snd_hda_core btusb btrtl mei_hdcp iTCO_wdt rapl mei_pxp btintel snd_seq iTCO_vendor_support btbcm snd_seq_device intel_cstate bluetooth snd_pcm cfg80211 intel_wmi_thunderbolt wmi_bmof intel_uncore snd_timer mei_me snd ecdh_generic i2c_i801 [ 4562.100541] ecc mei i2c_smbus soundcore rfkill intel_pch_thermal acpi_pad zram nouveau drm_ttm_helper ttm gpu_sched i2c_algo_bit drm_gpuvm drm_exec mxm_wmi drm_display_helper drm_kms_helper drm crct10dif_pclmul crc32_pclmul nvme e1000e crc32c_intel nvme_core ghash_clmulni_intel video wmi pinctrl_cannonlake ip6_tables ip_tables fuse [ 4562.100616] ---[ end trace ]--- Signed-off-by: Dave Airlie Cc: sta...@vger.kernel.org --- .../drm/nouveau/include/nvkm/core/client.h| 1 + drivers/gpu/drm/nouveau/nvkm/core/client.c| 1 + drivers/gpu/drm/nouveau/nvkm/core/object.c| 21 +++
Re: [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
Hi Jocelyn, kernel test robot noticed the following build warnings: [auto build test WARNING on bfa4437fd3938ae2e186e7664b2db65bb8775670] url: https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20240227-185901 base: bfa4437fd3938ae2e186e7664b2db65bb8775670 patch link: https://lore.kernel.org/r/20240227100459.194478-5-jfalempe%40redhat.com patch subject: [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240301/202403010934.yop7hcsq-...@intel.com/config) compiler: aarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240301/202403010934.yop7hcsq-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202403010934.yop7hcsq-...@intel.com/ All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/pl111/pl111_display.c:18: >> include/drm/drm_fb_dma_helper.h:23:46: warning: 'struct drm_plane' declared >> inside parameter list will not be visible outside of this definition or >> declaration 23 | void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane, | ^ vim +23 include/drm/drm_fb_dma_helper.h 11 12 struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, 13 unsigned int plane); 14 15 dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb, 16 struct drm_plane_state *state, 17 unsigned int plane); 18 19 void drm_fb_dma_sync_non_coherent(struct drm_device *drm, 20struct drm_plane_state *old_state, 21struct drm_plane_state *state); 22 > 23 void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane, 24 struct drm_framebuffer *fb); 25 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[PATCH v4 2/2] drm/bridge: cdns-dsi: Add support for StarFive JH7110 SoC
From: Keith Zhao
Add display bridge support for dsi on StarFive JH7110 SoC.
The mainly modification is followed:
1.Add extra clock and reset operation for JH7110.
2.Add callback for JH7110.
Signed-off-by: Keith Zhao
Signed-off-by: Shengyang Chen
---
drivers/gpu/drm/bridge/cadence/Kconfig| 7 +
drivers/gpu/drm/bridge/cadence/Makefile | 1 +
.../gpu/drm/bridge/cadence/cdns-dsi-core.c| 29 +++-
.../gpu/drm/bridge/cadence/cdns-dsi-core.h| 19 +++
.../gpu/drm/bridge/cadence/cdns-dsi-jh7110.c | 138 ++
.../gpu/drm/bridge/cadence/cdns-dsi-jh7110.h | 16 ++
6 files changed, 209 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c
create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h
diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig
b/drivers/gpu/drm/bridge/cadence/Kconfig
index cced81633ddc..ad9f572f4720 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -19,6 +19,13 @@ config DRM_CDNS_DSI_J721E
help
Support J721E Cadence DSI wrapper. The wrapper manages
the routing of the DSS DPI signal to the Cadence DSI.
+
+config DRM_CDNS_DSI_JH7110
+ bool "JH7110 SOC Cadence DSI support"
+ default n
+ help
+ Cadence DPI to DSI bridge which is embedded in the
+ StarFive JH7110 SoC.
endif
config DRM_CDNS_MHDP8546
diff --git a/drivers/gpu/drm/bridge/cadence/Makefile
b/drivers/gpu/drm/bridge/cadence/Makefile
index c95fd5b81d13..87f603a9f4ad 100644
--- a/drivers/gpu/drm/bridge/cadence/Makefile
+++ b/drivers/gpu/drm/bridge/cadence/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
cdns-dsi-y := cdns-dsi-core.o
cdns-dsi-$(CONFIG_DRM_CDNS_DSI_J721E) += cdns-dsi-j721e.o
+cdns-dsi-$(CONFIG_DRM_CDNS_DSI_JH7110) += cdns-dsi-jh7110.o
obj-$(CONFIG_DRM_CDNS_MHDP8546) += cdns-mhdp8546.o
cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 7457d38622b0..c0c81745e765 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -27,6 +27,10 @@
#include "cdns-dsi-j721e.h"
#endif
+#ifdef CONFIG_DRM_CDNS_DSI_JH7110
+#include "cdns-dsi-jh7110.h"
+#endif
+
#define IP_CONF0x0
#define SP_HS_FIFO_DEPTH(x)(((x) & GENMASK(30, 26)) >> 26)
#define SP_LP_FIFO_DEPTH(x)(((x) & GENMASK(25, 21)) >> 21)
@@ -552,6 +556,10 @@ static int cdns_dsi_adjust_phy_config(struct cdns_dsi *dsi,
/* data rate was in bytes/sec, convert to bits/sec. */
phy_cfg->hs_clk_rate = dlane_bps * 8;
+ if (dsi->platform_ops && dsi->platform_ops->mode_fixup)
+ adj_dsi_htotal = dsi->platform_ops->mode_fixup(dsi, dsi_cfg,
phy_cfg,
+ dpi_hz, dpi_htotal,
dsi_htotal);
+
dsi_hfp_ext = adj_dsi_htotal - dsi_htotal;
dsi_cfg->hfp += dsi_hfp_ext;
dsi_cfg->htotal = dsi_htotal + dsi_hfp_ext;
@@ -683,7 +691,7 @@ static void cdns_dsi_bridge_post_disable(struct drm_bridge
*bridge)
pm_runtime_put(dsi->base.dev);
}
-static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
+void cdns_dsi_hs_init(struct cdns_dsi *dsi)
{
struct cdns_dsi_output *output = >output;
u32 status;
@@ -1026,6 +1034,14 @@ static ssize_t cdns_dsi_transfer(struct mipi_dsi_host
*host,
cdns_dsi_init_link(dsi);
+ /*
+* on JH7110 SoC , when transfer dsi command ,
+* cdns_dsi_hs_init is needed.
+* or the final ret will be error value.
+*/
+ if (dsi->platform_ops && dsi->platform_ops->transfer)
+ dsi->platform_ops->transfer(dsi);
+
ret = mipi_dsi_create_packet(, msg);
if (ret)
goto out;
@@ -1142,6 +1158,9 @@ static int __maybe_unused cdns_dsi_resume(struct device
*dev)
clk_prepare_enable(dsi->dsi_p_clk);
clk_prepare_enable(dsi->dsi_sys_clk);
+ if (dsi->platform_ops && dsi->platform_ops->resume)
+ dsi->platform_ops->resume(dsi);
+
return 0;
}
@@ -1152,6 +1171,10 @@ static int __maybe_unused cdns_dsi_suspend(struct device
*dev)
clk_disable_unprepare(dsi->dsi_sys_clk);
clk_disable_unprepare(dsi->dsi_p_clk);
reset_control_assert(dsi->dsi_p_rst);
+
+ if (dsi->platform_ops && dsi->platform_ops->suspend)
+ dsi->platform_ops->suspend(dsi);
+
dsi->link_initialized = false;
return 0;
}
@@ -1294,6 +1317,10 @@ static const struct of_device_id cdns_dsi_of_match[] = {
#ifdef CONFIG_DRM_CDNS_DSI_J721E
{ .compatible = "ti,j721e-dsi", .data = _ti_j721e_ops, },
#endif
+#ifdef CONFIG_DRM_CDNS_DSI_JH7110
+ { .compatible =
[PATCH v4 0/2] Add StarFive JH7110 SoC DSI support
This series is the series that attempts to support the CDNS DSI driver used to converts DPI to DSI. CDNS DSI is embedded in StarFive JH7110 SoC. The series has been tested on the VisionFive 2 board. change since v3: - Rebased on tag v6.8-rc6. patch 2: - Replace clk API with clk_bulk_ API. - Replace rst API with reset_control_bulk API. - Use roundup() in cdns_dsi_jh7110_mode_fixup(). - Add assert for txbytehs reset operation by reset_control_bulk API in suspend function. - Add clk_bulk_disable_unprepare() in cdns_dsi_jh7110_resume() for deassert failure situation. v3: https://patchwork.kernel.org/project/dri-devel/cover/20240206065709.108684-1-shengyang.c...@starfivetech.com/ change since v2: - Rebased on tag v6.8-rc3. patch 1: - Modify commit message and patch subject - Change 'starfve,jh7110-dsi' to 'starfive,jh7110-dsi' - Add constraints for reset-names and clock names - Add resets, reset-names attribute - Correct reset and clock names patch 2: - Modify commit message and patch subject - Drop useless MAINTAINERS modification - Change callback name from 'update' to 'mode_fixup' - Optimize the mode_fixup function. - Change devm_reset_control_get() to devm_reset_control_get_exclusive() - Correct reset and clock names v2: https://patchwork.kernel.org/project/dri-devel/cover/20240109072516.24328-1-shengyang.c...@starfivetech.com/ changes since v1: - Rebased on tag v6.7. patch 1: - Changed the 'starfive,cdns-dsi' to 'starfve,jh7110-dsi'. - Changed the compatible enum alphabetical order. - Restrict other variants. - Drop 'dsi_' prefix. patch 2: - Optimize the calculation process. - Drop useless definition. v1: https://patchwork.kernel.org/project/dri-devel/cover/20231127113436.57361-1-shengyang.c...@starfivetech.com/ Keith Zhao (2): dt-bindings: display: bridge: cdns: Add display bridge support for dsi on StarFive JH7110 SoC drm/bridge: cdns-dsi: Add support for StarFive JH7110 SoC .../bindings/display/bridge/cdns,dsi.yaml | 56 ++- drivers/gpu/drm/bridge/cadence/Kconfig| 7 + drivers/gpu/drm/bridge/cadence/Makefile | 1 + .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 29 +++- .../gpu/drm/bridge/cadence/cdns-dsi-core.h| 19 +++ .../gpu/drm/bridge/cadence/cdns-dsi-jh7110.c | 138 ++ .../gpu/drm/bridge/cadence/cdns-dsi-jh7110.h | 16 ++ 7 files changed, 263 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h -- 2.17.1
[PATCH v4 1/2] dt-bindings: display: bridge: cdns: Add display bridge support for dsi on StarFive JH7110 SoC
From: Keith Zhao Add compatible to support dsi bridge on StarFive JH7110 SoC Signed-off-by: Keith Zhao Signed-off-by: Shengyang Chen Reviewed-by: Conor Dooley --- .../bindings/display/bridge/cdns,dsi.yaml | 56 ++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml index 23060324d16e..4ad0ecaacaae 100644 --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml @@ -16,6 +16,7 @@ properties: compatible: enum: - cdns,dsi + - starfive,jh7110-dsi - ti,j721e-dsi reg: @@ -27,14 +28,20 @@ properties: Register block for wrapper settings registers in case of TI J7 SoCs. clocks: +minItems: 2 items: - description: PSM clock, used by the IP - description: sys clock, used by the IP + - description: dpi clock, used by the IP + - description: txesc clock, used by the IP clock-names: +minItems: 2 items: - const: dsi_p_clk - const: dsi_sys_clk + - const: dpi + - const: txesc phys: maxItems: 1 @@ -46,10 +53,22 @@ properties: maxItems: 1 resets: -maxItems: 1 +minItems: 1 +items: + - description: apb reset, associated to dsi_p_clk + - description: sys reset, associated to sys clock + - description: dpi reset, associated to dpi clock + - description: txesc reset, associated to txesc clock + - description: txbytehs reset, associated to txbytehs clock reset-names: -const: dsi_p_rst +minItems: 1 +items: + - const: dsi_p_rst + - const: sys + - const: dpi + - const: txesc + - const: txbytehs ports: $ref: /schemas/graph.yaml#/properties/ports @@ -90,6 +109,39 @@ allOf: reg: maxItems: 1 + - if: + properties: +compatible: + contains: +const: starfive,jh7110-dsi +then: + properties: +clocks: + minItems: 4 + maxItems: 4 +clock-names: + minItems: 4 + maxItems: 4 +resets: + minItems: 5 + maxItems: 5 +reset-names: + minItems: 5 + maxItems: 5 + required: +- resets +- reset-names +else: + properties: +clocks: + maxItems: 2 +clock-names: + maxItems: 2 +resets: + maxItems: 1 +reset-names: + maxItems: 1 + required: - compatible - reg -- 2.17.1
Re: [PATCH v2 9/9] drm/panel: truly-nt35597: Don't log an error when DSI host can't be found
On 2/29/2024 4:12 PM, Nícolas F. R. A. Prado wrote: Given that failing to find a DSI host causes the driver to defer probe, make use of dev_err_probe() to log the reason. This makes the defer probe reason available and avoids alerting userspace about something that is not necessarily an error. Suggested-by: AngeloGioacchino Del Regno Signed-off-by: Nícolas F. R. A. Prado --- drivers/gpu/drm/panel/panel-truly-nt35597.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Abhinav Kumar
[PATCH v2 9/9] drm/panel: truly-nt35597: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/panel/panel-truly-nt35597.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c
b/drivers/gpu/drm/panel/panel-truly-nt35597.c
index b73448cf349d..d447db912a61 100644
--- a/drivers/gpu/drm/panel/panel-truly-nt35597.c
+++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c
@@ -550,10 +550,8 @@ static int truly_nt35597_probe(struct mipi_dsi_device *dsi)
dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
of_node_put(dsi1);
- if (!dsi1_host) {
- dev_err(dev, "failed to find dsi host\n");
- return -EPROBE_DEFER;
- }
+ if (!dsi1_host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
host\n");
/* register the second DSI device */
dsi1_device = mipi_dsi_device_register_full(dsi1_host, );
--
2.44.0
[PATCH v2 8/9] drm/panel: novatek-nt35950: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC
panels")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/panel/panel-novatek-nt35950.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
index 648ce9201426..028fdac293f7 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
@@ -556,10 +556,8 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
}
dsi_r_host = of_find_mipi_dsi_host_by_node(dsi_r);
of_node_put(dsi_r);
- if (!dsi_r_host) {
- dev_err(dev, "Cannot get secondary DSI host\n");
- return -EPROBE_DEFER;
- }
+ if (!dsi_r_host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "Cannot get
secondary DSI host\n");
nt->dsi[1] = mipi_dsi_device_register_full(dsi_r_host, info);
if (!nt->dsi[1]) {
--
2.44.0
[PATCH v2 7/9] drm/bridge: dpc3433: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Also move the "failed to attach" error message so that it's only printed
when the devm_mipi_dsi_attach() call fails.
Fixes: 6352cd451ddb ("drm: bridge: Add TI DLPC3433 DSI to DMD bridge")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/ti-dlpc3433.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-dlpc3433.c
b/drivers/gpu/drm/bridge/ti-dlpc3433.c
index ca3348109bcd..6b559e071301 100644
--- a/drivers/gpu/drm/bridge/ti-dlpc3433.c
+++ b/drivers/gpu/drm/bridge/ti-dlpc3433.c
@@ -319,12 +319,11 @@ static int dlpc_host_attach(struct dlpc *dlpc)
.channel = 0,
.node = NULL,
};
+ int ret;
host = of_find_mipi_dsi_host_by_node(dlpc->host_node);
- if (!host) {
- DRM_DEV_ERROR(dev, "failed to find dsi host\n");
- return -EPROBE_DEFER;
- }
+ if (!host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
host\n");
dlpc->dsi = mipi_dsi_device_register_full(host, );
if (IS_ERR(dlpc->dsi)) {
@@ -336,7 +335,11 @@ static int dlpc_host_attach(struct dlpc *dlpc)
dlpc->dsi->format = MIPI_DSI_FMT_RGB565;
dlpc->dsi->lanes = dlpc->dsi_lanes;
- return devm_mipi_dsi_attach(dev, dlpc->dsi);
+ ret = devm_mipi_dsi_attach(dev, dlpc->dsi);
+ if (ret)
+ DRM_DEV_ERROR(dev, "failed to attach dsi host\n");
+
+ return ret;
}
static int dlpc3433_probe(struct i2c_client *client)
@@ -367,10 +370,8 @@ static int dlpc3433_probe(struct i2c_client *client)
drm_bridge_add(>bridge);
ret = dlpc_host_attach(dlpc);
- if (ret) {
- DRM_DEV_ERROR(dev, "failed to attach dsi host\n");
+ if (ret)
goto err_remove_bridge;
- }
return 0;
--
2.44.0
[PATCH v2 6/9] drm/bridge: tc358775: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/tc358775.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 90a89d70d832..fea4f00a20f8 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -610,10 +610,8 @@ static int tc_attach_host(struct tc_data *tc)
};
host = of_find_mipi_dsi_host_by_node(tc->host_node);
- if (!host) {
- dev_err(dev, "failed to find dsi host\n");
- return -EPROBE_DEFER;
- }
+ if (!host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
host\n");
dsi = devm_mipi_dsi_device_register_full(dev, host, );
if (IS_ERR(dsi)) {
--
2.44.0
[PATCH v2 5/9] drm/bridge: lt9611uxc: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index e971b75e90ad..b803899126d5 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -265,10 +265,8 @@ static struct mipi_dsi_device *lt9611uxc_attach_dsi(struct
lt9611uxc *lt9611uxc,
int ret;
host = of_find_mipi_dsi_host_by_node(dsi_node);
- if (!host) {
- dev_err(dev, "failed to find dsi host\n");
- return ERR_PTR(-EPROBE_DEFER);
- }
+ if (!host)
+ return ERR_PTR(dev_err_probe(dev, -EPROBE_DEFER, "failed to
find dsi host\n"));
dsi = devm_mipi_dsi_device_register_full(dev, host, );
if (IS_ERR(dsi)) {
--
2.44.0
[PATCH v2 4/9] drm/bridge: lt9611: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: 23278bf54afe ("drm/bridge: Introduce LT9611 DSI to HDMI bridge")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/lontium-lt9611.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c
b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 9663601ce098..89bdd938757e 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -760,10 +760,8 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct
lt9611 *lt9611,
int ret;
host = of_find_mipi_dsi_host_by_node(dsi_node);
- if (!host) {
- dev_err(lt9611->dev, "failed to find dsi host\n");
- return ERR_PTR(-EPROBE_DEFER);
- }
+ if (!host)
+ return ERR_PTR(dev_err_probe(lt9611->dev, -EPROBE_DEFER,
"failed to find dsi host\n"));
dsi = devm_mipi_dsi_device_register_full(dev, host, );
if (IS_ERR(dsi)) {
--
2.44.0
[PATCH v2 3/9] drm/bridge: lt8912b: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: 30e2ae943c26 ("drm/bridge: Introduce LT8912B DSI to HDMI bridge")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/lontium-lt8912b.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c
b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 273157428c82..15aa890c3e6d 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -496,10 +496,8 @@ static int lt8912_attach_dsi(struct lt8912 *lt)
};
host = of_find_mipi_dsi_host_by_node(lt->host_node);
- if (!host) {
- dev_err(dev, "failed to find dsi host\n");
- return -EPROBE_DEFER;
- }
+ if (!host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
host\n");
dsi = devm_mipi_dsi_device_register_full(dev, host, );
if (IS_ERR(dsi)) {
--
2.44.0
[PATCH v2 2/9] drm/bridge: icn6211: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: 8dde6f7452a1 ("drm: bridge: icn6211: Add I2C configuration support")
Suggested-by: AngeloGioacchino Del Regno
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/chipone-icn6211.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c
b/drivers/gpu/drm/bridge/chipone-icn6211.c
index 82d23e4df09e..ff3284b6b1a3 100644
--- a/drivers/gpu/drm/bridge/chipone-icn6211.c
+++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
@@ -563,10 +563,8 @@ static int chipone_dsi_host_attach(struct chipone *icn)
host = of_find_mipi_dsi_host_by_node(host_node);
of_node_put(host_node);
- if (!host) {
- dev_err(dev, "failed to find dsi host\n");
- return -EPROBE_DEFER;
- }
+ if (!host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
host\n");
dsi = mipi_dsi_device_register_full(host, );
if (IS_ERR(dsi)) {
--
2.44.0
[PATCH v2 1/9] drm/bridge: anx7625: Don't log an error when DSI host can't be found
Given that failing to find a DSI host causes the driver to defer probe,
make use of dev_err_probe() to log the reason. This makes the defer
probe reason available and avoids alerting userspace about something
that is not necessarily an error.
Fixes: 269332997a16 ("drm/bridge: anx7625: Return -EPROBE_DEFER if the dsi host
was not found")
Reviewed-by: AngeloGioacchino Del Regno
Reviewed-by: Neil Armstrong
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 29d91493b101..4ee5614a2623 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2076,10 +2076,8 @@ static int anx7625_setup_dsi_device(struct anx7625_data
*ctx)
};
host = of_find_mipi_dsi_host_by_node(ctx->pdata.mipi_host_node);
- if (!host) {
- DRM_DEV_ERROR(dev, "fail to find dsi host.\n");
- return -EPROBE_DEFER;
- }
+ if (!host)
+ return dev_err_probe(dev, -EPROBE_DEFER, "fail to find dsi
host.\n");
dsi = devm_mipi_dsi_device_register_full(dev, host, );
if (IS_ERR(dsi)) {
--
2.44.0
[PATCH v2 0/9] drm: Switch from dev_err to dev_err_probe for missing DSI host error path
This series changes every occurence of the following pattern:
dsi_host = of_find_mipi_dsi_host_by_node(dsi);
if (!dsi_host) {
dev_err(dev, "failed to find dsi host\n");
return -EPROBE_DEFER;
}
into
dsi_host = of_find_mipi_dsi_host_by_node(dsi);
if (!dsi_host)
return dev_err_probe(dev, -EPROBE_DEFER, "failed to find dsi
host\n");
This registers the defer probe reason (so it can later be printed by the
driver core or checked on demand through the devices_deferred file in
debugfs) and prevents errors to be spammed in the kernel log every time
the driver retries to probe, unnecessarily alerting userspace about
something that is a normal part of the boot process.
I have omitted a Fixes: tag in the last patch, for the truly-nt35597
panel, because it predates the dev_err_probe() helper.
Changes in v2:
- Added patches 2 onwards to fix all occurences of this pattern instead
of just for the anx7625 driver
- Link to v1:
https://lore.kernel.org/r/20240226-anx7625-defer-log-no-dsi-host-v1-1-242b1af31...@collabora.com
---
Nícolas F. R. A. Prado (9):
drm/bridge: anx7625: Don't log an error when DSI host can't be found
drm/bridge: icn6211: Don't log an error when DSI host can't be found
drm/bridge: lt8912b: Don't log an error when DSI host can't be found
drm/bridge: lt9611: Don't log an error when DSI host can't be found
drm/bridge: lt9611uxc: Don't log an error when DSI host can't be found
drm/bridge: tc358775: Don't log an error when DSI host can't be found
drm/bridge: dpc3433: Don't log an error when DSI host can't be found
drm/panel: novatek-nt35950: Don't log an error when DSI host can't be
found
drm/panel: truly-nt35597: Don't log an error when DSI host can't be found
drivers/gpu/drm/bridge/analogix/anx7625.c | 6 ++
drivers/gpu/drm/bridge/chipone-icn6211.c | 6 ++
drivers/gpu/drm/bridge/lontium-lt8912b.c | 6 ++
drivers/gpu/drm/bridge/lontium-lt9611.c | 6 ++
drivers/gpu/drm/bridge/lontium-lt9611uxc.c| 6 ++
drivers/gpu/drm/bridge/tc358775.c | 6 ++
drivers/gpu/drm/bridge/ti-dlpc3433.c | 17 +
drivers/gpu/drm/panel/panel-novatek-nt35950.c | 6 ++
drivers/gpu/drm/panel/panel-truly-nt35597.c | 6 ++
9 files changed, 25 insertions(+), 40 deletions(-)
---
base-commit: 2ae0a045e6814c8c1d676d6153c605a65746aa29
change-id: 20240226-anx7625-defer-log-no-dsi-host-c3f9ccbcb287
Best regards,
--
Nícolas F. R. A. Prado
Re: [PATCH] drm/i915/selftest_hangcheck: Check sanity with more patience
Hi Janusz, On Wed, Feb 28, 2024 at 04:24:41PM +0100, Janusz Krzysztofik wrote: > While trying to reproduce some other issues reported by CI for i915 > hangcheck live selftest, I found them hidden behind timeout failures > reported by igt_hang_sanitycheck -- the very first hangcheck test case > executed. > > Feb 22 19:49:06 DUT1394ACMR kernel: calling mei_gsc_driver_init+0x0/0xff0 > [mei_gsc] @ 121074 > Feb 22 19:49:06 DUT1394ACMR kernel: i915 :03:00.0: [drm] DRM_I915_DEBUG > enabled > Feb 22 19:49:06 DUT1394ACMR kernel: i915 :03:00.0: [drm] Cannot find any > crtc or sizes > Feb 22 19:49:06 DUT1394ACMR kernel: probe of i915.mei-gsc.768 returned 0 > after 1475 usecs > Feb 22 19:49:06 DUT1394ACMR kernel: probe of i915.mei-gscfi.768 returned 0 > after 1441 usecs > Feb 22 19:49:06 DUT1394ACMR kernel: initcall mei_gsc_driver_init+0x0/0xff0 > [mei_gsc] returned 0 after 3010 usecs > Feb 22 19:49:06 DUT1394ACMR kernel: i915 :03:00.0: [drm] > DRM_I915_DEBUG_GEM enabled > Feb 22 19:49:06 DUT1394ACMR kernel: i915 :03:00.0: [drm] > DRM_I915_DEBUG_RUNTIME_PM enabled > Feb 22 19:49:06 DUT1394ACMR kernel: i915: Performing live selftests with > st_random_seed=0x4c26c048 st_timeout=500 > Feb 22 19:49:07 DUT1394ACMR kernel: i915: Running hangcheck > Feb 22 19:49:07 DUT1394ACMR kernel: calling mei_hdcp_driver_init+0x0/0xff0 > [mei_hdcp] @ 121074 > Feb 22 19:49:07 DUT1394ACMR kernel: i915: Running > intel_hangcheck_live_selftests/igt_hang_sanitycheck > Feb 22 19:49:07 DUT1394ACMR kernel: probe of > :00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04 returned 0 after 1398 usecs > Feb 22 19:49:07 DUT1394ACMR kernel: probe of > i915.mei-gsc.768-b638ab7e-94e2-4ea2-a552-d1c54b627f04 returned 0 after 97 > usecs > Feb 22 19:49:07 DUT1394ACMR kernel: initcall mei_hdcp_driver_init+0x0/0xff0 > [mei_hdcp] returned 0 after 101960 usecs > Feb 22 19:49:07 DUT1394ACMR kernel: calling mei_pxp_driver_init+0x0/0xff0 > [mei_pxp] @ 121094 > Feb 22 19:49:07 DUT1394ACMR kernel: probe of > :00:16.0-fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1 returned 0 after 435 usecs > Feb 22 19:49:07 DUT1394ACMR kernel: mei_pxp > i915.mei-gsc.768-fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1: bound :03:00.0 > (ops i915_pxp_tee_component_ops [i915]) > Feb 22 19:49:07 DUT1394ACMR kernel: 100ms wait for request failed on rcs0, > err=-62 > Feb 22 19:49:07 DUT1394ACMR kernel: probe of > i915.mei-gsc.768-fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1 returned 0 after 158425 > usecs > Feb 22 19:49:07 DUT1394ACMR kernel: initcall mei_pxp_driver_init+0x0/0xff0 > [mei_pxp] returned 0 after 224159 usecs > Feb 22 19:49:07 DUT1394ACMR kernel: i915/intel_hangcheck_live_selftests: > igt_hang_sanitycheck failed with error -5 > Feb 22 19:49:07 DUT1394ACMR kernel: i915: probe of :03:00.0 failed with > error -5 > > Those request waits, once timed out after 100ms, have never been > confirmed to still persist over another 100ms, always being able to > complete within the originally requested wait time doubled. > > Taking into account potentially significant additional concurrent workload > generated by new auxiliary drivers that didn't exist before and now are > loaded in parallel with the i915 module also when loaded in selftest mode, > relax our expectations on time consumed by the sanity check request before > it completes. > > Signed-off-by: Janusz Krzysztofik I'm OK with it... Reviewed-by: Andi Shyti Thanks, Andi
Re: [PATCH] drm/i915/selftests: Fix dependency of some timeouts on HZ
Hi Janusz,
On Thu, Feb 22, 2024 at 12:32:40PM +0100, Janusz Krzysztofik wrote:
> Third argument of i915_request_wait() accepts a timeout value in jiffies.
> Most users pass either a simple HZ based expression, or a result of
> msecs_to_jiffies(), or MAX_SCHEDULE_TIMEOUT, or a very small number not
> exceeding 4 if applicable as that value. However, there is one user --
> intel_selftest_wait_for_rq() -- that passes a WAIT_FOR_RESET_TIME symbol,
> defined as a large constant value that most probably represents a desired
> timeout in ms. While that usage results in the intended value of timeout
> on usual x86_64 kernel configurations, it is not portable across different
> architectures and custom kernel configs.
>
> Rename the symbol to clearly indicate intended units and convert it to
> jiffies before use.
>
> Fixes: 3a4bfa091c46 ("drm/i915/selftest: Fix workarounds selftest for GuC
> submission")
> Signed-off-by: Janusz Krzysztofik
> Cc: Rahul Kumar Singh
> Cc: John Harrison
> Cc: Matthew Brost
Reviewed-by: Andi Shyti
Thanks,
Andi
[PATCH v3 4/4] drm/i915/gt: Enable only one CCS for compute workload
Enable only one CCS engine by default with all the compute sices
allocated to it.
While generating the list of UABI engines to be exposed to the
user, exclude any additional CCS engines beyond the first
instance.
This change can be tested with igt i915_query.
Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
Requires: 4e4f77d74878 ("drm/i915/gt: Refactor uabi engine class/instance list
creation")
Signed-off-by: Andi Shyti
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Matt Roper
Cc: # v6.2+
---
drivers/gpu/drm/i915/gt/intel_engine_user.c | 11 +++
drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
3 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c
b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index ec5bcd1c1ec4..6d6ef11f55e5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -208,6 +208,7 @@ void intel_engines_driver_register(struct drm_i915_private
*i915)
struct list_head *it, *next;
struct rb_node **p, *prev;
LIST_HEAD(engines);
+ u16 uabi_ccs = 0;
sort_engines(i915, );
@@ -256,6 +257,16 @@ void intel_engines_driver_register(struct drm_i915_private
*i915)
BIT(_CCS(engine->uabi_instance
continue;
+ /*
+* The load is balanced among all the available compute
+* slices. Expose only the first instance of the compute
+* engine.
+*/
+ if (IS_DG2(i915) &&
+ uabi_class == I915_ENGINE_CLASS_COMPUTE &&
+ uabi_ccs++)
+ continue;
+
GEM_BUG_ON(uabi_class >=
ARRAY_SIZE(i915->engine_uabi_class_count));
i915->engine_uabi_class_count[uabi_class]++;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index a425db5ed3a2..e19df4ef47f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
}
}
+static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
+{
+ if (!IS_DG2(gt->i915))
+ return;
+
+ intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
+}
+
int intel_gt_init_hw(struct intel_gt *gt)
{
struct drm_i915_private *i915 = gt->i915;
@@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
intel_gt_init_swizzling(gt);
+ /* Configure CCS mode */
+ intel_gt_apply_ccs_mode(gt);
+
/*
* At least 830 can leave some of the unused rings
* "active" (ie. head != tail) after resume which
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index cf709f6c05ae..c148113770ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1605,6 +1605,8 @@
#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
#define GEN12_CAGF_MASK REG_GENMASK(19, 11)
+#define XEHP_CCS_MODE _MMIO(0x14804)
+
#define GEN11_GT_INTR_DW(x)_MMIO(0x190018 + ((x) * 4))
#define GEN11_CSME (31)
#define GEN12_HECI_2 (30)
--
2.43.0
[PATCH v3 3/4] drm/i915/gt: Disable HW load balancing for CCS
The hardware should not dynamically balance the load between CCS
engines. Wa_14019159160 recommends disabling it across all
platforms.
Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
Signed-off-by: Andi Shyti
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Matt Roper
Cc: # v6.2+
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 50962cfd1353..cf709f6c05ae 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1478,6 +1478,7 @@
#define GEN12_RCU_MODE _MMIO(0x14800)
#define GEN12_RCU_MODE_CCS_ENABLEREG_BIT(0)
+#define XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE REG_BIT(1)
#define CHV_FUSE_GT_MMIO(VLV_GUNIT_BASE + 0x2168)
#define CHV_FGT_DISABLE_SS0 (1 << 10)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d67d44611c28..57c1f3d2589e 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2945,6 +2945,12 @@ general_render_compute_wa_init(struct intel_engine_cs
*engine, struct i915_wa_li
/* Wa_18028616096 */
wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW,
UGM_FRAGMENT_THRESHOLD_TO_3);
+
+ /*
+* Wa_14019159160: disable the CCS load balancing
+* indiscriminately for all the platforms
+*/
+ wa_masked_en(wal, GEN12_RCU_MODE,
XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE);
}
if (IS_DG2_G11(i915)) {
--
2.43.0
[PATCH v3 2/4] drm/i915/gt: Do not exposed fused off engines.
Some of the CCS engines are disabled. They should not be listed
in the uabi_engine list, that is the list of engines that the
user can see.
Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
Requires: 4e4f77d74878 ("drm/i915/gt: Refactor uabi engine class/instance list
creation")
Signed-off-by: Andi Shyti
---
drivers/gpu/drm/i915/gt/intel_engine_user.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c
b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index cf8f24ad88f6..ec5bcd1c1ec4 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -244,6 +244,18 @@ void intel_engines_driver_register(struct drm_i915_private
*i915)
if (uabi_class > I915_LAST_UABI_ENGINE_CLASS)
continue;
+ /*
+* If the CCS engine is fused off, the corresponding bit
+* in the engine mask is disabled. Do not expose it
+* to the user.
+*
+* By default at least one engine is enabled (check
+* the engine_mask_apply_compute_fuses() function.
+*/
+ if (!(engine->gt->info.engine_mask &
+ BIT(_CCS(engine->uabi_instance
+ continue;
+
GEM_BUG_ON(uabi_class >=
ARRAY_SIZE(i915->engine_uabi_class_count));
i915->engine_uabi_class_count[uabi_class]++;
--
2.43.0
[PATCH v3 1/4] drm/i915/gt: Refactor uabi engine class/instance list creation
For the upcoming changes we need a cleaner way to build the list
of uabi engines.
Suggested-by: Tvrtko Ursulin
Signed-off-by: Andi Shyti
---
drivers/gpu/drm/i915/gt/intel_engine_user.c | 29 -
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c
b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 833987015b8b..cf8f24ad88f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -203,7 +203,7 @@ static void engine_rename(struct intel_engine_cs *engine,
const char *name, u16
void intel_engines_driver_register(struct drm_i915_private *i915)
{
- u16 name_instance, other_instance = 0;
+ u16 class_instance[I915_LAST_UABI_ENGINE_CLASS + 1] = { };
struct legacy_ring ring = {};
struct list_head *it, *next;
struct rb_node **p, *prev;
@@ -214,6 +214,8 @@ void intel_engines_driver_register(struct drm_i915_private
*i915)
prev = NULL;
p = >uabi_engines.rb_node;
list_for_each_safe(it, next, ) {
+ u16 uabi_class;
+
struct intel_engine_cs *engine =
container_of(it, typeof(*engine), uabi_list);
@@ -222,15 +224,14 @@ void intel_engines_driver_register(struct
drm_i915_private *i915)
GEM_BUG_ON(engine->class >= ARRAY_SIZE(uabi_classes));
engine->uabi_class = uabi_classes[engine->class];
- if (engine->uabi_class == I915_NO_UABI_CLASS) {
- name_instance = other_instance++;
- } else {
- GEM_BUG_ON(engine->uabi_class >=
- ARRAY_SIZE(i915->engine_uabi_class_count));
- name_instance =
-
i915->engine_uabi_class_count[engine->uabi_class]++;
- }
- engine->uabi_instance = name_instance;
+
+ if (engine->uabi_class == I915_NO_UABI_CLASS)
+ uabi_class = I915_LAST_UABI_ENGINE_CLASS + 1;
+ else
+ uabi_class = engine->uabi_class;
+
+ GEM_BUG_ON(uabi_class >= ARRAY_SIZE(class_instance));
+ engine->uabi_instance = class_instance[uabi_class]++;
/*
* Replace the internal name with the final user and log facing
@@ -238,11 +239,15 @@ void intel_engines_driver_register(struct
drm_i915_private *i915)
*/
engine_rename(engine,
intel_engine_class_repr(engine->class),
- name_instance);
+ engine->uabi_instance);
- if (engine->uabi_class == I915_NO_UABI_CLASS)
+ if (uabi_class > I915_LAST_UABI_ENGINE_CLASS)
continue;
+ GEM_BUG_ON(uabi_class >=
+ ARRAY_SIZE(i915->engine_uabi_class_count));
+ i915->engine_uabi_class_count[uabi_class]++;
+
rb_link_node(>uabi_node, prev, p);
rb_insert_color(>uabi_node, >uabi_engines);
--
2.43.0
[PATCH v3 0/4] Disable automatic load CCS load balancing
Hi, this series does basically two things: 1. Disables automatic load balancing as adviced by the hardware workaround. 2. Assigns all the CCS slices to one single user engine. The user will then be able to query only one CCS engine I'm using here the "Requires: " tag, but I'm not sure the commit id will be valid, on the other hand, I don't know what commit id I should use. Thanks Tvrtko, Matt and John for your reviews! Andi Changelog = v2 -> v3 - Simplified the algorithm for creating the list of the exported uabi engines. (Patch 1) (Thanks, Tvrtko) - Consider the fused engines when creating the uabi engine list (Patch 2) (Thanks, Matt) - Patch 4 now uses a the refactoring from patch 1, in a cleaner outcome. v1 -> v2 - In Patch 1 use the correct workaround number (thanks Matt). - In Patch 2 do not add the extra CCS engines to the exposed UABI engine list and adapt the engine counting accordingly (thanks Tvrtko). - Reword the commit of Patch 2 (thanks John). Andi Shyti (4): drm/i915/gt: Refactor uabi engine class/instance list creation drm/i915/gt: Do not exposed fused off engines. drm/i915/gt: Disable HW load balancing for CCS drm/i915/gt: Enable only one CCS for compute workload drivers/gpu/drm/i915/gt/intel_engine_user.c | 52 - drivers/gpu/drm/i915/gt/intel_gt.c | 11 + drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 ++ drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 +++ 4 files changed, 60 insertions(+), 12 deletions(-) -- 2.43.0
Colorspace "Default" and the CTA-861 spec
Hey all, I was trying to document the Colorspace property variants with what the sink expects, what kind of data the CRTC should be configured to produce, and what drivers must guarantee. I tried to stick to CTA-861-I for this exercise and after more than an hour to get the "Default" variant right, I gave up and now I'm writing this mail. There are actually 4 distinct parameters that have to be known: the colorimetry (primaries and white point), the transfer characteristics, the YCbCr conversion matrix and the quantization level. Section 5.1 has two paragraphs talking about the default color space (which should imply the rest of the parameters). For IT formats, and CE formats in some situations "the RGB color space used should be the RGB color space the Sink declares in the Basic Display Parameters and Features Block of its EDID". For sinks that accept YCbCr data and InfoFrames, CE formats default to other color spaces. They do support setting InfoFrames though and thus can set Y2=Y1=Y0=0 and force the sink into accepting RGB (Table 18) and set C1=C0=0 for Colorimetry RGB. Foote a helpfully notes that this is the colorimetry declared in the EDID. Now, the first obvious issue is that none of that defines the YCbCr matrix to be used. It is entirely undefined. If drivers use an YCbCr output format, user space gets undefined behavior. The second issue is the transfer characteristics. Section 5.3 defines the transfer characteristics when a specific color space is selected. If the colorimetry is RGB then the BT.709 transfer characteristics are used (and not the gamma from the EDID) whereas the default color space as defined in 5.1 seems to point to the EDID even for gamma. That would means depending on if we have an IT mode or a CE mode with a sink that supports YCbCr and InfoFrames, the default transfer characteristics could be different. User space would have to figure all of that out to know which transfer characteristics it should use. Is the spec just wrong here? There also is the "defaultRGB" colorimetry. This one actually clearly defines that the sink shall expect data with the colorimetry and gamma from the EDID. It seems to have been added in CTA-861-I so support for it is basically non-existent. So what should we do? To me it seems almost impossible to define anything useful for the "Default" property variant, but because the "defaultRGB" colorimetry is not supported at all right now, making "Default" undefined means we can't get predictable colors on almost all displays. Cheers, Sebastian
Re: [PATCH v2 1/2] drm: panel: st7701: Add Hardkernel ODROID-GO Ultra panel support
On 2/29/2024 9:23 AM, Adam Green wrote:
On 26/02/2024 21:29, Jessica Zhang wrote:
> Got it. Was the shorter sleep time breaking the display and is it
> required for the new panel to work?
>
> Thanks,
>
> Jessica Zhang
Hi Jessica,
I will be submitting a v3 shortly, the change to the sleep time was not
necessary for the new panel
to work.
Hi Adam,
Got it. If the panel isn't affected by the 20ms sleep time, I'd prefer
to keep it since 100ms is a pretty big increase.
I have been able to re-use the gip sequence from the kd50t048a panel
used in the Hardkernel Odroid
Go Super as I have been led to believe it is the same elida panel,
unfortunately the same modes
used by that device do not work for the Odroid Go Ultra and so its still
necessary to have the
patchset,
Got it. FWIW, I do see the Odroid Go Ultra being described as having the
kd50t048a panel [1] [2]. Looking forward to seeing the v3 changes.
Thanks,
Jessica Zhang
[1] https://gitlab.com/amlogic-foss/mainline-linux-issues-tracker/-/issues/7
[2] 441e129cbf81 ("dt-bindings: display: panel: sitronix,st7701: Add
Elida KD50T048A Panel")
Best regards,
Adam
Re: [PATCH v3 00/10] Make PCI's devres API more consistent
On Thu, Feb 29, 2024 at 09:31:20AM +0100, Philipp Stanner wrote: > @Bjorn: > Hey Bjorn, are we good with this series? Any more wishes or > suggestions? Sorry, haven't had a chance to go through it yet. FWIW, I just tried to apply these on top of pci/devres, but it failed here: Applying: PCI: Add new set of devres functions Applying: PCI: Deprecate iomap-table functions Applying: PCI: Warn users about complicated devres nature Applying: PCI: Make devres region requests consistent Applying: PCI: Move dev-enabled status bit to struct pci_dev error: patch failed: drivers/pci/pci.h:811 error: drivers/pci/pci.h: patch does not apply Patch failed at 0005 PCI: Move dev-enabled status bit to struct pci_dev Haven't investigated, so maybe it's some trivial easily fixed thing. Bjorn
[PATCH AUTOSEL 6.7 24/24] drm/amd/display: fix input states translation error for dcn35 & dcn351
From: Swapnil Patel
[ Upstream commit 27a6c49394b1a203beeb94752c9a1d6318f24ddf ]
[Why]
Currently there is an error while translating input clock sates into
output clock states. The highest fclk setting from output sates is
being dropped because of this error.
[How]
For dcn35 and dcn351, make output_states equal to input states.
Reviewed-by: Charlene Liu
Acked-by: Rodrigo Siqueira
Tested-by: Daniel Wheeler
Signed-off-by: Swapnil Patel
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
.../drm/amd/display/dc/dml2/dml2_translation_helper.c| 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
index 2c379be19aa84..16452dae4acac 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
@@ -398,7 +398,6 @@ void dml2_init_soc_states(struct dml2_context *dml2, const
struct dc *in_dc,
/* Copy clocks tables entries, if available */
if (dml2->config.bbox_overrides.clks_table.num_states) {
p->in_states->num_states =
dml2->config.bbox_overrides.clks_table.num_states;
-
for (i = 0; i <
dml2->config.bbox_overrides.clks_table.num_entries_per_clk.num_dcfclk_levels;
i++) {
p->in_states->state_array[i].dcfclk_mhz =
dml2->config.bbox_overrides.clks_table.clk_entries[i].dcfclk_mhz;
}
@@ -437,6 +436,14 @@ void dml2_init_soc_states(struct dml2_context *dml2, const
struct dc *in_dc,
}
dml2_policy_build_synthetic_soc_states(s, p);
+ if (dml2->v20.dml_core_ctx.project == dml_project_dcn35 ||
+ dml2->v20.dml_core_ctx.project == dml_project_dcn351) {
+ // Override last out_state with data from last in_state
+ // This will ensure that out_state contains max fclk
+ memcpy(>out_states->state_array[p->out_states->num_states -
1],
+
>in_states->state_array[p->in_states->num_states - 1],
+ sizeof(struct soc_state_bounding_box_st));
+ }
}
void dml2_translate_ip_params(const struct dc *in, struct ip_params_st *out)
--
2.43.0
[PATCH] drm: Document requirements for driver-specific KMS props in new drivers
When extending support for a driver-specific KMS property to additional drivers, we should apply all the requirements for new properties and make sure the semantics are the same and documented. Signed-off-by: Sebastian Wick --- Documentation/gpu/drm-kms.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 13d3627d8bc0..afa10a28035f 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -496,6 +496,11 @@ addition to the one mentioned above: * An IGT test must be submitted where reasonable. +For historical reasons, non-standard, driver-specific properties exist. If a KMS +driver wants to add support for one of those properties, the requirements for +new properties apply where possible. Additionally, the documented behavior must +match the de facto semantics of the existing property to ensure compatibility. + Property Types and Blob Property Support -- 2.43.0
RE: [PATCH 2/4] drm: xlnx: zynqmp_dpsub: Anounce supported input formats
Hi Laurent,
Thanks for the review.
> -Original Message-
> From: dri-devel On Behalf Of Laurent
> Pinchart
> Sent: Wednesday, February 28, 2024 7:58 AM
> To: Klymenko, Anatoliy
> Cc: Maarten Lankhorst ; Maxime Ripard
> ; Thomas Zimmermann ; David
> Airlie ; Daniel Vetter ; Simek, Michal
> ; Andrzej Hajda ; Neil
> Armstrong ; Robert Foss ; Jonas
> Karlman ; Jernej Skrabec ; dri-
> de...@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH 2/4] drm: xlnx: zynqmp_dpsub: Anounce supported input
> formats
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Anatoliy,
>
> Thank you for the patch.
>
> On Mon, Feb 26, 2024 at 08:44:43PM -0800, Anatoliy Klymenko wrote:
> > DPSUB in bridge mode supports multiple input media bus formats.
> >
> > Announce the list of supported input media bus formats via
> > drm_bridge.atomic_get_input_bus_fmts callback.
> >
> > Signed-off-by: Anatoliy Klymenko
> > ---
> > drivers/gpu/drm/xlnx/zynqmp_disp.c | 37
> > +
> > drivers/gpu/drm/xlnx/zynqmp_disp.h | 10 ++
> > drivers/gpu/drm/xlnx/zynqmp_dp.c | 1 +
> > 3 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index e6d26ef60e89..ee99aad915ba 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > @@ -18,6 +18,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -77,12 +78,14 @@ enum zynqmp_dpsub_layer_mode {
> > /**
> > * struct zynqmp_disp_format - Display subsystem format information
> > * @drm_fmt: DRM format (4CC)
> > + * @bus_fmt: Media bus format
> > * @buf_fmt: AV buffer format
> > * @swap: Flag to swap R & B for RGB formats, and U & V for YUV formats
> > * @sf: Scaling factors for color components
> > */
> > struct zynqmp_disp_format {
> > u32 drm_fmt;
> > + u32 bus_fmt;
> > u32 buf_fmt;
> > bool swap;
> > const u32 *sf;
> > @@ -364,6 +367,40 @@ static const struct zynqmp_disp_format
> avbuf_gfx_fmts[] = {
> > },
> > };
> >
> > +/* List of live video layer formats */ static const struct
> > +zynqmp_disp_format avbuf_live_fmts[] = {
> > + {
> > + .drm_fmt= DRM_FORMAT_VYUY,
> > + .bus_fmt= MEDIA_BUS_FMT_VYUY8_1X16,
> > + .buf_fmt= ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> > + .sf = scaling_factors_888,
>
> Is there a reason to have a separate array, instead of populating .bus_fmt in
> the
> existing arrays for the formats that can be supported with the live input,
> and only
> reporting those from
> zynqmp_disp_get_input_bus_fmts() ?
>
There are multiple reasons for this: 1) those formats share some similarities,
although they are different in nature, e.g. memory layout formats vs. video
signal formats; 2) corresponding IP registers are different with incompatible
layouts; 3) ZynqMP DPSUB documentation clearly distinguishes 3 sets of formats:
live video, video packer and graphics packer, ref.
https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Formats.
I think, having separate format arrays will help to avoid ambiguity.
> > + },
> > +};
> > +
> > +u32 *zynqmp_disp_get_input_bus_fmts(struct drm_bridge *bridge,
> > + struct drm_bridge_state *bridge_state,
> > + struct drm_crtc_state *crtc_state,
> > + struct drm_connector_state *conn_state,
> > + u32 output_fmt,
> > + unsigned int *num_input_fmts) {
> > + int i;
> > + u32 *input_fmts;
> > +
> > + input_fmts = kcalloc(ARRAY_SIZE(avbuf_live_fmts), sizeof(*input_fmts),
> GFP_KERNEL);
> > + if (!input_fmts) {
> > + *num_input_fmts = 0;
> > + return input_fmts;
> > + }
> > +
> > + for (i = 0; i < ARRAY_SIZE(avbuf_live_fmts); ++i)
> > + input_fmts[i] = avbuf_live_fmts[i].bus_fmt;
>
> Extra space.
>
ACK. Thank you.
> > + *num_input_fmts = ARRAY_SIZE(avbuf_live_fmts);
> > +
> > + return input_fmts;
> > +}
> > +
> > static u32 zynqmp_disp_avbuf_read(struct zynqmp_disp *disp, int reg)
> > {
> > return readl(disp->avbuf.base + reg); diff --git
> > a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > index 9b8b202224d9..c2c8dd4896ba 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > @@ -27,6 +27,10 @@
> > struct device;
> > struct drm_format_info;
> > struct drm_plane_state;
> > +struct drm_bridge;
> > +struct
[PATCH] etnaviv: Restore some id values
From: Christian Gmeiner
The hwdb selection logic as a feature that allows it to mark some fields
as 'don't care'. If we match with such a field we memcpy(..)
the current etnaviv_chip_identity into ident.
This step can overwrite some id values read from the GPU with the
'don't care' value.
Fix this issue by restoring the affected values after the memcpy(..).
As this is crucial for user space to know when this feature works as
expected increment the minor version too.
Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
Cc: sta...@vger.kernel.org
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6228ce603248..9a2965741dab 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
.desc = "etnaviv DRM",
.date = "20151214",
.major = 1,
- .minor = 3,
+ .minor = 4,
};
/*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 67201242438b..1e38d66702f1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
{
struct etnaviv_chip_identity *ident = >identity;
+ const u32 product_id = ident->product_id;
+ const u32 customer_id = ident->customer_id;
+ const u32 eco_id = ident->eco_id;
int i;
for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
@@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu
*gpu)
etnaviv_chip_identities[i].eco_id == ~0U)) {
memcpy(ident, _chip_identities[i],
sizeof(*ident));
+
+ /* Restore some id values if ~0U aka 'don't care' is
used. */
+ if (etnaviv_chip_identities[i].product_id == ~0U)
+ ident->product_id = product_id;
+
+ if (etnaviv_chip_identities[i].customer_id == ~0U)
+ ident->customer_id = customer_id;
+
+ if (etnaviv_chip_identities[i].eco_id == ~0U)
+ ident->eco_id = eco_id;
+
return true;
}
}
--
2.44.0
Re: [PATCH v6 9/9] usb: misc: onboard_dev: add support for XMOS XVF3500
On Thu, Feb 29, 2024 at 09:34:52AM +0100, Javier Carrasco wrote: > The XMOS XVF3500 VocalFusion Voice Processor[1] is a low-latency, 32-bit > multicore controller for voice processing. > > This device requires a specific power sequence, which consists of > enabling the regulators that control the 3V3 and 1V0 device supplies, > and a reset de-assertion after a delay of at least 100ns. > > Once in normal operation, the XVF3500 registers itself as a USB device, > and it does not require any device-specific operations in the driver. > > [1] https://www.xmos.com/xvf3500/ > > Signed-off-by: Javier Carrasco Acked-by: Matthias Kaehlcke
RE: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
> On Thu, 29 Feb 2024, "Golani, Mitulkumar Ajitkumar" > wrote: > > Thought behind this was to use the function where it was defined. But > > no worries, I have split it with new patch series floated. > > Please do not rush to send so many new versions! Let the review come to a > conclusion first. Apologies, Jani, for the rapid succession of patch releases. I missed including the first patch in the initial series float, leading to its inclusion in the subsequent revision. Regards, Mitul
Re: [PATCH v6 7/9] usb: misc: onboard_dev: add support for non-hub devices
On Thu, Feb 29, 2024 at 09:34:50AM +0100, Javier Carrasco wrote:
> Most of the functionality this driver provides can be used by non-hub
> devices as well.
>
> To account for the hub-specific code, add a flag to the device data
> structure and check its value for hub-specific code.
>
> The 'always_powered_in_supend' attribute is only available for hub
> devices, keeping the driver's default behavior for non-hub devices (keep
> on in suspend).
>
> Signed-off-by: Javier Carrasco
> ---
> drivers/usb/misc/onboard_usb_dev.c | 25 -
> drivers/usb/misc/onboard_usb_dev.h | 10 ++
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/onboard_usb_dev.c
> b/drivers/usb/misc/onboard_usb_dev.c
> index 4ae580445408..f1b174503c44 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
> @@ -261,7 +261,27 @@ static struct attribute *onboard_dev_attrs[] = {
> _attr_always_powered_in_suspend.attr,
> NULL,
> };
> -ATTRIBUTE_GROUPS(onboard_dev);
> +
> +static umode_t onboard_dev_attrs_are_visible(struct kobject *kobj,
> + struct attribute *attr,
> + int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct onboard_dev *onboard_dev = dev_get_drvdata(dev);
> +
> + if (attr == _attr_always_powered_in_suspend.attr &&
> + !onboard_dev->pdata->is_hub)
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static const struct attribute_group onboard_dev_group = {
> + .is_visible = onboard_dev_attrs_are_visible,
> + .attrs = onboard_dev_attrs,
> +};
> +__ATTRIBUTE_GROUPS(onboard_dev);
> +
nit: remove one empty line
>
> static void onboard_dev_attach_usb_driver(struct work_struct *work)
> {
> @@ -286,6 +306,9 @@ static int onboard_dev_probe(struct platform_device *pdev)
> if (!onboard_dev->pdata)
> return -EINVAL;
>
> + if (!onboard_dev->pdata->is_hub)
> + onboard_dev->always_powered_in_suspend = true;
> +
> onboard_dev->dev = dev;
>
> err = onboard_dev_get_regulators(onboard_dev);
> diff --git a/drivers/usb/misc/onboard_usb_dev.h
> b/drivers/usb/misc/onboard_usb_dev.h
> index 4da9f3b7f9e9..58cf8c81b2cf 100644
> --- a/drivers/usb/misc/onboard_usb_dev.h
> +++ b/drivers/usb/misc/onboard_usb_dev.h
> @@ -12,60 +12,70 @@ struct onboard_dev_pdata {
> unsigned long reset_us; /* reset pulse width in us */
> unsigned int num_supplies; /* number of supplies */
> const char * const supply_names[MAX_SUPPLIES];
> + bool is_hub;/* true if the device is a HUB */
nit: either drop the comment (the variable name is pretty self explaining),
or s/HUB/hub/ ('hub' isn't an acronym).
Acked-by: Matthias Kaehlcke
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Thu, Feb 22, 2024 at 07:14:07PM +0100, Maxime Ripard wrote:
> The i915 driver has a property to force the RGB range of an HDMI output.
> The vc4 driver then implemented the same property with the same
> semantics. KWin has support for it, and a PR for mutter is also there to
> support it.
>
> Both drivers implementing the same property with the same semantics,
> plus the userspace having support for it, is proof enough that it's
> pretty much a de-facto standard now and we can provide helpers for it.
>
> Let's plumb it into the newly created HDMI connector.
>
> Reviewed-by: Dave Stevenson
> Signed-off-by: Maxime Ripard
> ---
> Documentation/gpu/kms-properties.csv | 1 -
> drivers/gpu/drm/drm_atomic.c | 2 +
> drivers/gpu/drm/drm_atomic_state_helper.c | 4 +-
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 89
> +++
> include/drm/drm_connector.h | 36 +
> 6 files changed, 134 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/kms-properties.csv
> b/Documentation/gpu/kms-properties.csv
> index 0f9590834829..caef14c532d4 100644
> --- a/Documentation/gpu/kms-properties.csv
> +++ b/Documentation/gpu/kms-properties.csv
> @@ -17,7 +17,6 @@ Owner Module/Drivers,Group,Property Name,Type,Property
> Values,Object attached,De
> ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property
> to suggest an X offset for a connector
> ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest
> an Y offset for a connector
> ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9""
> }",Connector,TDB
> -i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited
> 16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is
> set, the hardware will be programmed with the result of the multiplication of
> CTM by the limited range matrix to ensure the pixels normally in the range
> 0..1.0 are remapped to the range 16/255..235/255."
> ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
> ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" }
> etc.",Connector,TBD
> ,,"""left_margin""",RANGE,"Min=0, Max= SDVO dependent",Connector,TBD
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 26f9e525c0a0..3e57d98d8418 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1145,6 +1145,8 @@ static void drm_atomic_connector_print_state(struct
> drm_printer *p,
>
> if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> + drm_printf(p, "\tbroadcast_rgb=%s\n",
> +
> drm_hdmi_connector_get_broadcast_rgb_name(state->hdmi.broadcast_rgb));
> drm_printf(p, "\toutput_bpc=%u\n", state->hdmi.output_bpc);
> drm_printf(p, "\toutput_format=%s\n",
>
> drm_hdmi_connector_get_output_format_name(state->hdmi.output_format));
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 9f517599f117..0e8fb653965a 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -589,6 +589,7 @@ void __drm_atomic_helper_connector_hdmi_reset(struct
> drm_connector *connector,
>
> new_state->max_bpc = max_bpc;
> new_state->max_requested_bpc = max_bpc;
> + new_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_AUTO;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_hdmi_reset);
>
> @@ -913,7 +914,8 @@ int drm_atomic_helper_connector_hdmi_check(struct
> drm_connector *connector,
> if (ret)
> return ret;
>
> - if (old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
> + if (old_state->hdmi.broadcast_rgb != new_state->hdmi.broadcast_rgb ||
> + old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
> old_state->hdmi.output_format != new_state->hdmi.output_format) {
> struct drm_crtc *crtc = new_state->crtc;
> struct drm_crtc_state *crtc_state;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 29d4940188d4..2b415b4ed506 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -776,6 +776,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->max_requested_bpc = val;
> } else if (property == connector->privacy_screen_sw_state_property) {
> state->privacy_screen_sw_state = val;
> + } else if (property == connector->broadcast_rgb_property) {
> + state->hdmi.broadcast_rgb = val;
> } else if (connector->funcs->atomic_set_property) {
> return
Re: [PATCH v6 6/9] ARM: multi_v7_defconfig: update ONBOARD_USB_HUB to ONBOAD_USB_DEV
On Thu, Feb 29, 2024 at 09:34:49AM +0100, Javier Carrasco wrote: > The onboard_usb_hub driver has been updated to support non-hub devices, > which has led to some renaming. Update to the new name accordingly. > > Update to the new name (ONBOARD_USB_DEV) accordingly. > > Signed-off-by: Javier Carrasco Reviewed-by: Matthias Kaehlcke
Re: [PATCH v6 5/9] arm64: defconfig: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
On Thu, Feb 29, 2024 at 09:34:48AM +0100, Javier Carrasco wrote: > The onboard_usb_hub driver has been updated to support non-hub devices, > which has led to some renaming. > > Update to the new name (ONBOARD_USB_DEV) accordingly. > > Signed-off-by: Javier Carrasco Reviewed-by: Matthias Kaehlcke
Re: [PATCH v6 4/9] drm: ci: arm64.config: update ONBOARD_USB_HUB to ONBOARD_USB_DEV
On Thu, Feb 29, 2024 at 09:34:47AM +0100, Javier Carrasco wrote: > The onboard_usb_hub driver has been updated to support non-hub devices, > which has led to some renaming. > > Update to the new name (ONBOARD_USB_DEV) accordingly. > > Acked-by: Helen Koike > Signed-off-by: Javier Carrasco Reviewed-by: Matthias Kaehlcke
Re: [PATCH v6 3/9] usb: misc: onboard_hub: rename to onboard_dev
On Thu, Feb 29, 2024 at 09:34:46AM +0100, Javier Carrasco wrote:
> This patch prepares onboad_hub to support non-hub devices by renaming
> the driver files and their content, the headers and their references.
>
> The comments and descriptions have been slightly modified to keep
> coherence and account for the specific cases that only affect onboard
> hubs (e.g. peer-hub).
>
> The "hub" variables in functions where "dev" (and similar names) variables
> already exist have been renamed to onboard_dev for clarity, which adds a
> few lines in cases where more than 80 characters are used.
>
> No new functionality has been added.
>
> Signed-off-by: Javier Carrasco
Acked-by: Matthias Kaehlcke
This should land together with "usb: misc: onboard_dev: add support for
non-hub devices".
> diff --git a/drivers/usb/misc/onboard_usb_dev.c
> b/drivers/usb/misc/onboard_usb_dev.c
> new file mode 100644
> index ..4ae580445408
> --- /dev/null
> +++ b/drivers/usb/misc/onboard_usb_dev.c
>
> ...
>
> +static const struct usb_device_id onboard_dev_id_table[] = {
> + { USB_DEVICE(VENDOR_ID_CYPRESS, 0x6504) }, /* CYUSB33{0,1,2}x/CYUSB230x
> 3.0 HUB */
> + { USB_DEVICE(VENDOR_ID_CYPRESS, 0x6506) }, /* CYUSB33{0,1,2}x/CYUSB230x
> 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_CYPRESS, 0x6570) }, /* CY7C6563x 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB
> 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G USB
> 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523 USB
> 3.1 HUB */
> + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2412) }, /* USB2412 USB 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2517) }, /* USB2517 USB 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2744) }, /* USB5744 USB 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x5744) }, /* USB5744 USB 3.0 HUB */
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 HUB */
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 HUB */
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 HUB */
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 HUB */
> + { USB_DEVICE(VENDOR_ID_TI, 0x8140) }, /* TI USB8041 3.0 HUB */
> + { USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 HUB */
> + { USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817 3.1 HUB */
> + { USB_DEVICE(VENDOR_ID_VIA, 0x2817) }, /* VIA VL817 2.0 HUB */
> + {}
> +};
nit: 'hub' isn't an acronym, please s/HUB/hub/ in the next revision.
RE: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
On Thu, 29 Feb 2024, "Golani, Mitulkumar Ajitkumar" wrote: > Thought behind this was to use the function where it was defined. But > no worries, I have split it with new patch series floated. Please do not rush to send so many new versions! Let the review come to a conclusion first. BR, Jani. -- Jani Nikula, Intel
Re: [PATCH v3 4/4] dt-bindings: display: simple: allow panel-common properties
On 23/02/2024 13:36, Raphael Gallais-Pou wrote: > This device inherits properties from panel-common. Those should be allowed > to use, instead of specifying properties to true for each specific use. > > Signed-off-by: Raphael Gallais-Pou > --- > Changes in v3: > - Allow every properties instead of adding each properties to true as > Rob suggested > - Rewrite commit log to match changes > > Changes in v2: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] drm/amdgpu: remove misleading amdgpu_pmops_runtime_idle() comment
Applied. Thanks!
On Thu, Feb 29, 2024 at 1:11 PM Bjorn Helgaas wrote:
>
> From: Bjorn Helgaas
>
> After 4020c2280233 ("drm/amdgpu: don't runtime suspend if there are
> displays attached (v3)"), "ret" is unconditionally set later before being
> used, so there's point in initializing it and the associated comment is no
> longer meaningful.
>
> Remove the comment and the unnecessary initialization.
>
> Signed-off-by: Bjorn Helgaas
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index cc69005f5b46..68416e2a9130 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2744,8 +2744,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
> {
> struct drm_device *drm_dev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(drm_dev);
> - /* we don't want the main rpm_idle to call suspend - we want to
> autosuspend */
> - int ret = 1;
> + int ret;
>
> if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE) {
> pm_runtime_forbid(dev);
> --
> 2.34.1
>
Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
Hi Jocelyn, kernel test robot noticed the following build errors: [auto build test ERROR on bfa4437fd3938ae2e186e7664b2db65bb8775670] url: https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20240227-185901 base: bfa4437fd3938ae2e186e7664b2db65bb8775670 patch link: https://lore.kernel.org/r/20240227100459.194478-4-jfalempe%40redhat.com patch subject: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic. config: m68k-randconfig-r052-20240229 (https://download.01.org/0day-ci/archive/20240301/202403010210.gfty1nkq-...@intel.com/config) compiler: m68k-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240301/202403010210.gfty1nkq-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202403010210.gfty1nkq-...@intel.com/ All errors (new ones prefixed by >>): m68k-linux-ld: drivers/gpu/drm/drm_panic.o: in function `debugfs_start': >> drm_panic.c:(.init.text+0x0): multiple definition of `init_module'; >> drivers/gpu/drm/drm_drv.o:drm_drv.c:(.init.text+0x0): first defined here m68k-linux-ld: drivers/gpu/drm/drm_panic.o: in function `debugfs_end': >> drm_panic.c:(.exit.text+0x0): multiple definition of `cleanup_module'; >> drivers/gpu/drm/drm_drv.o:drm_drv.c:(.text+0x714): first defined here -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[pull] drm/msm: drm-msm-next-2024-02-29 for v6.9
Hi Dave, This is the main pull for v6.9, description below. We may have a second smaller pull next week for CDM support, which depends on a patch in drm-misc-next which was just recently tagged. The following changes since commit 41c177cf354126a22443b5c80cec9fdd313e67e1: Merge tag 'drm-misc-next-2024-02-08' into msm-next (2024-02-11 12:34:39 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git tags/drm-msm-next-2024-02-29 for you to fetch changes up to 18397519cb62248865ca33266a483dbcf7d08b5f: drm/msm/adreno: Add A702 support (2024-02-26 07:29:55 -0800) Updates for v6.9: Core: - Correct bindings for MSM8976 and SM8650 platforms - Start migration of MDP5 platforms to DPU driver - X1E80100 MDSS support DPU: - Improve DSC allocation, fixing several important corner cases - Add support for SDM630/SDM660 platforms - Simplify dpu_encoder_phys_ops - Apply fixes targeting DSC support with a single DSC encoder - Apply fixes for HCTL_EN timing configuration - X1E80100 support DP: - Refactor parser and power submodules DSI: - Clean up obsolete set_split_display support - Update DSC documentation MDP5: - Clean up obsolete set_split_display support GPU: - fix sc7180 UBWC config - fix a7xx LLC config - new gpu support: a305B, a750, a702 - machine support: SM7150 (different power levels than other a618) - a7xx devcoredump support Abel Vesa (4): dt-bindings: display/msm: Document the DPU for X1E80100 dt-bindings: display/msm: Document MDSS on X1E80100 drm/msm: mdss: Add X1E80100 support drm/msm/dpu: Add X1E80100 support Abhinav Kumar (1): drm/msm/dpu: fix the programming of INTF_CFG2_DATA_HCTL_EN Adam Skladowski (2): dt-bindings: dsi-controller-main: Document missing msm8976 compatible dt-bindings: msm: qcom, mdss: Include ommited fam-b compatible Colin Ian King (1): drm/msm/dp: Fix spelling mistake "enale" -> "enable" Connor Abbott (4): drm/msm: Import a7xx crashdump register lists from kgsl drm/msm: Fix snapshotting a7xx indexed regs drm/msm: More fully implement devcoredump for a7xx drm/msm: Fix page fault client detection on a660 family and a7xx Danila Tikhonov (1): drm/msm/adreno: Add support for SM7150 SoC machine Dmitry Baryshkov (35): drm/msm/dsi: do not store internal bridge pointer drm/msm/dsi: drop msm_dsi_device_connected() function drm/msm/dsi: stop calling set_split_display drm/msm/dsi: remove msm_dsi::encoder drm/msm/mdp5: drop split display support drm/msm/dp: drop unused parser definitions drm/msm/dp: drop unused fields from dp_power_private drm/msm/dp: parse DT from dp_parser_get drm/msm/dp: inline dp_power_(de)init drm/msm/dp: fold dp_power into dp_ctrl module drm/msm/dp: simplify stream clocks handling drm/msm/dp: stop parsing clock names from DT drm/msm/dp: split dp_ctrl_clk_enable into four functuions drm/msm/dp: move phy_configure_opts to dp_ctrl drm/msm/dp: remove PHY handling from dp_catalog.c drm/msm/dp: handle PHY directly in dp_ctrl drm/msm/dp: move all IO handling to dp_catalog drm/msm/dp: move link property handling to dp_panel drm/msm/dp: move next_bridge handling to dp_display drm/msm/dp: drop dp_parser drm/msm/dpu: split irq_control into irq_enable and _disable drm/msm/dpu: split _dpu_encoder_resource_control_helper() drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set drm/msm/dpu: move writeback's atomic_check to dpu_writeback.c drm/msm/dpu: drop dpu_encoder_phys_ops::atomic_check() drm/msm/dsi: Document DSC related pclk_rate and hdisplay calculations drm/msm/mdss: generate MDSS data for MDP5 platforms drm/msm/dpu: support binding to the mdp5 devices drm/msm: add a kernel param to select between MDP5 and DPU drivers drm/msm/dpu: add support for SDM660 and SDM630 platforms drm/msm/dpu: finalise global state object drm/msm/dpu: drop global_state_lock drm/msm/mdp5: migrate SMP dumping to using atomic_print_state drm/msm/mdp5: drop global_state_lock drm/msm/a6xx: specify UBWC config for sc7180 Konrad Dybcio (1): drm/msm/adreno: Add A702 support Kuogee Hsieh (2): drm/msm/dpu: improve DSC allocation drm/msm/dp: remove mdss_dp_test_bit_depth_to_bpc() Luca Weiss (2): dt-bindings: display/msm: gpu: Allow multiple digits for patchid drm/msm/adreno: Add A305B support Marijn Suijten (2): drm/msm/dsi: Replace dsi_get_bpp() with mipi_dsi header function drm/msm/dpu: Only enable DSC_MODE_MULTIPLEX if dsc_merge is enabled Neil Armstrong (4): dt-bindings: display/msm/gmu: Document Adreno 750 GMU dt-bindings: arm-smmu: fix SM8[45]50 GPU SMMU if condition
[linux-next:master] BUILD REGRESSION f303a3e2bcfba900efb5aee55236d17030e9f882
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: f303a3e2bcfba900efb5aee55236d17030e9f882 Add linux-next specific files for 20240229 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202402292319.ehmsyi8l-...@intel.com https://lore.kernel.org/oe-kbuild-all/202403010220.welhzqii-...@intel.com Error/Warning: (recently discovered and may have been fixed) (.text+0x721c): undefined reference to `__divdi3' arm-linux-gnueabi-ld: file.c:(.text+0x958): undefined reference to `__aeabi_ldivmod' arm-linux-gnueabi-ld: inode.c:(.text+0x48e): undefined reference to `__aeabi_ldivmod' data.o:(.text+0x33e0): undefined reference to `__moddi3' drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:413:34: error: 'struct cpuinfo_loongarch' has no member named 'x86' drivers/leds/leds-gpio-register.c:23:25: warning: attribute declaration must precede definition [-Wignored-attributes] file.c:(.text+0x944): undefined reference to `__aeabi_ldivmod' file.o:(.text+0x820c): undefined reference to `__moddi3' file.o:(.text+0x97d0): undefined reference to `__divdi3' gc.o:(.text+0x4874): undefined reference to `__moddi3' include/linux/dpll.h:179:1: warning: control reaches end of non-void function [-Wreturn-type] inode.c:(.text+0x47a): undefined reference to `__aeabi_ldivmod' inode.o:(.text+0x1328): undefined reference to `__moddi3' microblaze-linux-ld: data.o:(.text+0x33fc): undefined reference to `__moddi3' microblaze-linux-ld: data.o:(.text+0x3418): undefined reference to `__divdi3' microblaze-linux-ld: file.o:(.text+0x8228): undefined reference to `__moddi3' microblaze-linux-ld: file.o:(.text+0x8244): undefined reference to `__divdi3' microblaze-linux-ld: gc.o:(.text+0x4890): undefined reference to `__moddi3' microblaze-linux-ld: gc.o:(.text+0x48ac): undefined reference to `__divdi3' microblaze-linux-ld: inode.o:(.text+0x1344): undefined reference to `__moddi3' microblaze-linux-ld: inode.o:(.text+0x1360): undefined reference to `__divdi3' microblaze-linux-ld: namei.o:(.text+0x1940): undefined reference to `__moddi3' microblaze-linux-ld: namei.o:(.text+0x195c): undefined reference to `__divdi3' microblaze-linux-ld: segment.o:(.text+0xda30): undefined reference to `__moddi3' microblaze-linux-ld: segment.o:(.text+0xda4c): undefined reference to `__divdi3' microblaze-linux-ld: xattr.o:(.text+0x1930): undefined reference to `__moddi3' microblaze-linux-ld: xattr.o:(.text+0x194c): undefined reference to `__divdi3' namei.o:(.text+0x1924): undefined reference to `__moddi3' segment.o:(.text+0xda14): undefined reference to `__moddi3' xattr.o:(.text+0x1914): undefined reference to `__moddi3' Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg | `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead |-- arc-allmodconfig | |-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg | |-- drivers-gpu-drm-xe-xe_ggtt.c:error:implicit-declaration-of-function-writeq | `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead |-- arc-allyesconfig | |-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg | |-- drivers-gpu-drm-xe-xe_ggtt.c:error:implicit-declaration-of-function-writeq | `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead |-- arm-allmodconfig | |-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg | |-- drivers-gpu-drm-xe-xe_ggtt.c:error:implicit-declaration-of-function-writeq | `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead |-- arm-allyesconfig | |-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg | |-- drivers-gpu-drm-xe-xe_ggtt.c:error:implicit-declaration-of-function-writeq | `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead |-- arm-randconfig-001-20240116 | |-- arm-linux-gnueabi-ld:file.c:(.text):undefined-reference-to-__aeabi_ldivmod | |-- arm-linux-gnueabi-ld:inode.c:(.text):undefined-reference-to-__aeabi_ldivmod | |-- file.c:(.text):undefined-reference-to-__aeabi_ldivmod | `-- inode.c:(.text):undefined-reference-to-__aeabi_ldivmod |-- arm-randconfig-r131-20240229 | |-- drivers-leds-flash-leds-ktd2692.c:sparse:sparse:symbol-ktd2692_timing-was-not-declared.-Should-it-be-static
Re: [PATCH v2 0/7] Add YUV formats to VKMS
On 29/02/24 14:52, Sebastian Wick wrote: > On Wed, Feb 28, 2024 at 08:42:41PM -0300, Arthur Grillo wrote: >> >> >> On 15/01/24 12:06, Sebastian Wick wrote: >>> On Wed, Jan 10, 2024 at 02:44:00PM -0300, Arthur Grillo wrote: This patchset aims to add support for additional buffer YUV formats. More specifically, it adds support to: Semi-planar formats: - NV12 - NV16 - NV24 - NV21 - NV61 - NV42 Planar formats: - YUV440 - YUV422 - YUV444 - YVU440 - YVU422 - YVU444 These formats have more than one plane, and most have chroma subsampling. These properties don't have support on VKMS, so I had to work on this before. To ensure that the conversions from YUV to RGB are working, I wrote a KUnit test. As the work from Harry on creating KUnit tests on VKMS[1] is not yet merged, I took the setup part (Kconfig entry and .kunitfile) from it. Furthermore, I couldn't find any sources with the conversion matrices, so I had to work out the values myself based on the ITU papers[2][3][4]. So, I'm not 100% sure if the values are accurate. I'd appreciate some input if anyone has more knowledge in this area. >>> >>> H.273 CICP [1] has concise descriptions of the required values for most >>> widely used formats and the colour python framework [2] also can be used >>> to quickly get to the desired information most of the time. >>> >>> [1]: https://www.itu.int/rec/T-REC-H.273 >>> [2]: https://www.colour-science.org/ >> >> I want to thank you for suggesting the python framework, it helped >> immensely now that I'm changing the precision from 8-bit to 32-bit[1]. >> >> If I'd known about this framework while developing this patch, I >> would've saved myself a few weeks of pain and suffering recreating a >> part of this library XD. > > I'm glad this is useful for you! We also used it for our color-and-hdr > project https://gitlab.freedesktop.org/pq/color-and-hdr/. Cool project! I'll be sure to give look! Best Regards, ~Arthur Grillo > >> [1]: >> https://lore.kernel.org/all/b23da076-0bfb-48b2-9386-383a6dec1...@riseup.net/ >> >> Best Regards, >> ~Arthur Grillo >> >>> Also, I used two IGT tests to check if the formats were having a correct conversion (all with the --extended flag): - kms_plane@pixel_format - kms_plane@pixel_format_source_clamping. The nonsubsampled formats don't have support on IGT, so I sent a patch fixing this[5]. Currently, this patchset does not add those formats to the writeback, as it would require a rewrite of how the conversions are done (similar to what was done on a previous patch[6]). So, I would like to review this patchset before I start the work on this other part. [1]: https://lore.kernel.org/all/20231108163647.106853-5-harry.wentl...@amd.com/ [2]: https://www.itu.int/rec/R-REC-BT.601-7-201103-I/en [3]: https://www.itu.int/rec/R-REC-BT.709-6-201506-I/en [4]: https://www.itu.int/rec/R-REC-BT.2020-2-201510-I/en [5]: https://lists.freedesktop.org/archives/igt-dev/2024-January/066937.html [6]: https://lore.kernel.org/dri-devel/20230414135151.75975-2-mca...@igalia.com/ Signed-off-by: Arthur Grillo --- Changes in v2: - Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test file (Maxime) - Link to v1: https://lore.kernel.org/r/20240105-vkms-yuv-v1-0-34c4cd345...@riseup.net --- Arthur Grillo (7): drm/vkms: Use drm_frame directly drm/vkms: Add support for multy-planar framebuffers drm/vkms: Add range and encoding properties to pixel_read function drm/vkms: Add chroma subsampling drm/vkms: Add YUV support drm/vkms: Drop YUV formats TODO drm/vkms: Create KUnit tests for YUV conversions Documentation/gpu/vkms.rst| 3 +- drivers/gpu/drm/vkms/Kconfig | 15 ++ drivers/gpu/drm/vkms/Makefile | 1 + drivers/gpu/drm/vkms/tests/.kunitconfig | 4 + drivers/gpu/drm/vkms/tests/Makefile | 3 + drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 drivers/gpu/drm/vkms/vkms_drv.h | 6 +- drivers/gpu/drm/vkms/vkms_formats.c | 247 ++ drivers/gpu/drm/vkms/vkms_formats.h | 9 + drivers/gpu/drm/vkms/vkms_plane.c | 26 ++- drivers/gpu/drm/vkms/vkms_writeback.c | 5 - 11 files changed, 426 insertions(+), 49 deletions(-) --- base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f change-id: 20231226-vkms-yuv-6f7859f32df8 Best regards, -- Arthur Grillo >>> >> >
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Thu, Feb 29, 2024 at 12:21:34PM -0600, Lucas De Marchi wrote: > On Thu, Feb 29, 2024 at 12:49:57PM +0200, Andy Shevchenko wrote: > > On Wed, Feb 28, 2024 at 05:39:21PM -0600, Lucas De Marchi wrote: > > > On Thu, Feb 22, 2024 at 06:49:59AM -0800, Yury Norov wrote: ... > > > I build-tested this in x86-64, x86-32 and arm64. I didn't like much the > > > need to fork the __GENMASK() implementation on the 2 sides of the ifdef > > > since I think the GENMASK_INPUT_CHECK() should be the one covering the > > > input checks. However to make it common we'd need to solve 2 problems: > > > the casts and the sizeof. The sizeof can be passed as arg to > > > __GENMASK(), however the casts I think would need a __CAST_U8(x) > > > or the like and sprinkle it everywhere, which would hurt readability. > > > Not pretty. Or go back to the original submission and make it less > > > horrible :-/ > > > > I'm wondering if we can use _Generic() approach here. > > in assembly? Yes. -- With Best Regards, Andy Shevchenko
RE: Making drm_gpuvm work across gpu devices
Hi Christian/Daniel/Dave/Felix/Thomas, and all, We have been refining our design internally in the past month. Below is our plan. Please let us know if you have any concern. 1) Remove pseudo /dev/xe-svm device. All system allocator interfaces will be through /dev/dri/render devices. Not global interface. 2) Unify userptr and system allocator codes. We will treat userptr as a speciality of system allocator without migration capability. We will introduce the hmmptr concept for system allocator. We will extend vm_bind API to map a range A..B of process address space to a range C..D of GPU address space for hmmptr. For hmmptr, if gpu program accesses an address which is not backed by core mm vma, it is a fatal error. 3) Multiple device support. We have identified p2p use-cases where we might want to leave memory on a foreign device or direct migrations to a foreign device and therefore might need a global structure that tracks or caches the migration state per process address space. We didn't completely settle down this design. We will come back when we have more details. 4)We will first work on this code on xekmd then look to move some common codes to drm layer so it can also be used by other vendors. Thomas and me still have open questions to Christian. We will follow up. Thanks all for this discussion. Regards, Oak > -Original Message- > From: Christian König > Sent: Thursday, February 1, 2024 3:52 AM > To: Zeng, Oak ; Daniel Vetter ; David > Airlie > Cc: Thomas Hellström ; Brost, Matthew > ; Felix Kuehling ; Welty, > Brian ; dri-devel@lists.freedesktop.org; Ghimiray, > Himal > Prasad ; Bommu, Krishnaiah > ; Gupta, saurabhg ; > Vishwanathapura, Niranjana ; intel- > x...@lists.freedesktop.org; Danilo Krummrich ; Shah, Ankur N > ; jgli...@redhat.com; rcampb...@nvidia.com; > apop...@nvidia.com > Subject: Re: Making drm_gpuvm work across gpu devices > > Hi Oak, > > Am 31.01.24 um 21:17 schrieb Zeng, Oak: > > Hi Sima, Dave, > > > > I am well aware nouveau driver is not what Nvidia do with their customer. > > The > key argument is, can we move forward with the concept shared virtual address > space b/t CPU and GPU? This is the foundation of HMM. We already have split > address space support with other driver API. SVM, from its name, it means > shared address space. Are we allowed to implement another driver model to > allow SVM work, along with other APIs supporting split address space? Those > two > scheme can co-exist in harmony. We actually have real use cases to use both > models in one application. > > > > Hi Christian, Thomas, > > > > In your scheme, GPU VA can != GPU VA. This does introduce some flexibility. > But this scheme alone doesn't solve the problem of the proxy process/para- > virtualization. You will still need a second mechanism to partition GPU VA > space > b/t guest process1 and guest process2 because proxy process (or the host > hypervisor whatever you call it) use one single gpu page table for all the > guest/client processes. GPU VA for different guest process can't overlap. If > this > second mechanism exist, we of course can use the same mechanism to partition > CPU VA space between guest processes as well, then we can still use shared VA > b/t CPU and GPU inside one process, but process1 and process2's address space > (for both cpu and gpu) doesn't overlap. This second mechanism is the key to > solve the proxy process problem, not the flexibility you introduced. > > That approach was suggested before, but it doesn't work. First of all > you create a massive security hole when you give the GPU full access to > the QEMU CPU process which runs the virtualization. > > So even if you say CPU VA == GPU VA you still need some kind of > flexibility otherwise you can't implement this use case securely. > > Additional to this the CPU VAs are usually controlled by the OS and not > some driver, so to make sure that host and guest VAs don't overlap you > would need to add some kind of sync between the guest and host OS kernels. > > > In practice, your scheme also have a risk of running out of process space > because you have to partition whole address space b/t processes. Apparently > allowing each guest process to own the whole process space and using separate > GPU/CPU page table for different processes is a better solution than using > single > page table and partition process space b/t processes. > > Yeah that you run out of address space is certainly possible. But as I > said CPUs are switching to 5 level of pages tables and if you look at > for example a "cat maps | cut -c-4 | sort -u" of process you will find > that only a handful of 4GiB segments are actually used and thanks to > recoverable page faults you can map those between host and client on > demand. This gives you at least enough address space to handle a couple > of thousand clients. > > > For Intel GPU, para-virtualization (xenGT, see > > https://github.com/intel/XenGT- >
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Thu, Feb 29, 2024 at 12:49:57PM +0200, Andy Shevchenko wrote: On Wed, Feb 28, 2024 at 05:39:21PM -0600, Lucas De Marchi wrote: On Thu, Feb 22, 2024 at 06:49:59AM -0800, Yury Norov wrote: > On Wed, Feb 21, 2024 at 03:59:06PM -0600, Lucas De Marchi wrote: > > On Wed, Feb 21, 2024 at 11:04:22PM +0200, Andy Shevchenko wrote: > > > On Wed, Feb 21, 2024 at 10:30:02PM +0200, Dmitry Baryshkov wrote: > > > > On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: ... I build-tested this in x86-64, x86-32 and arm64. I didn't like much the need to fork the __GENMASK() implementation on the 2 sides of the ifdef since I think the GENMASK_INPUT_CHECK() should be the one covering the input checks. However to make it common we'd need to solve 2 problems: the casts and the sizeof. The sizeof can be passed as arg to __GENMASK(), however the casts I think would need a __CAST_U8(x) or the like and sprinkle it everywhere, which would hurt readability. Not pretty. Or go back to the original submission and make it less horrible :-/ I'm wondering if we can use _Generic() approach here. in assembly? ... > #define GENMASK_INPUT_CHECK(h, l) 0 > +#define __GENMASK(t, h, l) \ > + ((~0 - (1 << (l)) + 1) & (~0 >> (BITS_PER_LONG - 1 - (h humn... this builds, but does it work if GENMASK_ULL() is used in assembly? That BITS_PER_LONG does not match the type width. UL()/ULL() macros are not just for fun. they are not for fun, but they expand to a nop in assembly. And it's up to the instruction used to be the right one. Since this branch is for assembly only, having them wouldn't really change the current behavior. I'm talking about BITS_PER_LONG vs BITS_PER_LONG_LONG. That introduces a bug here. Lucas De Marchi -- With Best Regards, Andy Shevchenko
[PATCH] drm/fsl-dcu: remove unnecessary NULL checks
From: Bjorn Helgaas
The power management callbacks are never called unless .probe() has already
returned success, which means it has set drvdata to a non-NULL pointer, so
"dev" can never be NULL in the other callbacks.
Remove the unnecessary checks.
Signed-off-by: Bjorn Helgaas
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index ab6c0c6cd0e2..ca23a2ca16bb 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -170,9 +170,6 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
int ret;
- if (!fsl_dev)
- return 0;
-
disable_irq(fsl_dev->irq);
ret = drm_mode_config_helper_suspend(fsl_dev->drm);
@@ -191,9 +188,6 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
int ret;
- if (!fsl_dev)
- return 0;
-
ret = clk_prepare_enable(fsl_dev->clk);
if (ret < 0) {
dev_err(dev, "failed to enable dcu clk\n");
--
2.34.1
[PATCH] drm/amdgpu: remove misleading amdgpu_pmops_runtime_idle() comment
From: Bjorn Helgaas
After 4020c2280233 ("drm/amdgpu: don't runtime suspend if there are
displays attached (v3)"), "ret" is unconditionally set later before being
used, so there's point in initializing it and the associated comment is no
longer meaningful.
Remove the comment and the unnecessary initialization.
Signed-off-by: Bjorn Helgaas
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index cc69005f5b46..68416e2a9130 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2744,8 +2744,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(drm_dev);
- /* we don't want the main rpm_idle to call suspend - we want to
autosuspend */
- int ret = 1;
+ int ret;
if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE) {
pm_runtime_forbid(dev);
--
2.34.1
Re: [PATCH v2 6/9] drm/vkms: Add YUV support
On 29/02/24 09:12, Pekka Paalanen wrote:
> On Wed, 28 Feb 2024 22:52:09 -0300
> Arthur Grillo wrote:
>
>> On 27/02/24 17:01, Arthur Grillo wrote:
>>>
>>>
>>> On 27/02/24 12:02, Louis Chauvet wrote:
Hi Pekka,
For all the comment related to the conversion part, maybe Arthur have an
opinion on it, I took his patch as a "black box" (I did not want to
break (and debug) it).
Le 26/02/24 - 14:19, Pekka Paalanen a écrit :
> On Fri, 23 Feb 2024 12:37:26 +0100
> Louis Chauvet wrote:
>
>> From: Arthur Grillo
>>
>> Add support to the YUV formats bellow:
>>
>> - NV12
>> - NV16
>> - NV24
>> - NV21
>> - NV61
>> - NV42
>> - YUV420
>> - YUV422
>> - YUV444
>> - YVU420
>> - YVU422
>> - YVU444
>>
>> The conversion matrices of each encoding and range were obtained by
>> rounding the values of the original conversion matrices multiplied by
>> 2^8. This is done to avoid the use of fixed point operations.
>>
>> Signed-off-by: Arthur Grillo
>> [Louis Chauvet: Adapted Arthur's work and implemented the read_line_t
>> callbacks for yuv formats]
>> Signed-off-by: Louis Chauvet
>> ---
>> drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
>> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
>> drivers/gpu/drm/vkms/vkms_formats.c | 289
>> +--
>> drivers/gpu/drm/vkms/vkms_formats.h | 4 +
>> drivers/gpu/drm/vkms/vkms_plane.c| 14 +-
>> 5 files changed, 295 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
>> b/drivers/gpu/drm/vkms/vkms_composer.c
>> index e555bf9c1aee..54fc5161d565 100644
>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
>> @@ -312,7 +312,7 @@ static void blend(struct vkms_writeback_job *wb,
>> * buffer [1]
>> */
>> current_plane->pixel_read_line(
>> -current_plane->frame_info,
>> +current_plane,
>> x_start,
>> y_start,
>> direction,
>> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
>> b/drivers/gpu/drm/vkms/vkms_drv.h
>> index ccc5be009f15..a4f6456cb971 100644
>> --- a/drivers/gpu/drm/vkms/vkms_drv.h
>> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
>> @@ -75,6 +75,8 @@ enum pixel_read_direction {
>> READ_RIGHT
>> };
>>
>> +struct vkms_plane_state;
>> +
>> /**
>> <<< HEAD
>> * typedef pixel_read_line_t - These functions are used to read a pixel
>> line in the source frame,
>> @@ -87,8 +89,8 @@ enum pixel_read_direction {
>> * @out_pixel: Pointer where to write the pixel value. Pixels will be
>> written between x_start and
>> * x_end.
>> */
>> -typedef void (*pixel_read_line_t)(struct vkms_frame_info *frame_info,
>> int x_start, int y_start, enum
>> -pixel_read_direction direction, int count, struct
>> pixel_argb_u16 out_pixel[]);
>> +typedef void (*pixel_read_line_t)(struct vkms_plane_state *frame_info,
>> int x_start, int y_start,
>> +enum pixel_read_direction direction, int count, struct
>> pixel_argb_u16 out_pixel[]);
>
> This is the second or third time in this one series changing this type.
> Could you not do the change once, in its own patch if possible?
Sorry, this is not a change here, but a wrong formatting (missed when
rebasing).
Do you think that it make sense to re-order my patches and put this
typedef at the end? This way it is never updated.
>
> I'm not sure, I haven't checked how it would change your patches. The
> intermediate changes might get a lot uglier?
>
> Just try to fold changes so that you don't need to change something
> twice over the series unless there is a good reason to. "How hard would
> it be to review this?" is my measure stick.
>
>
>>
>> /**
>> * vkms_plane_state - Driver specific plane state
>> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c
>> b/drivers/gpu/drm/vkms/vkms_formats.c
>> index 46daea6d3ee9..515c80866a58 100644
>> --- a/drivers/gpu/drm/vkms/vkms_formats.c
>> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
>> @@ -33,7 +33,8 @@ static size_t packed_pixels_offset(const struct
>> vkms_frame_info *frame_info, int
>> */
>> return fb->offsets[plane_index] +
>> (y / drm_format_info_block_width(format, plane_index)) *
>> fb->pitches[plane_index] +
>> - (x / drm_format_info_block_height(format, plane_index))
>> * format->char_per_block[plane_index];
>> +
Re: [PATCH 1/3] dt-bindings: Add Crystal Clear Technology vendor prefix
On Thu, Feb 29, 2024 at 09:54:59AM +0100, Neil Armstrong wrote: > Hi Jérémie, > > On 23/02/2024 19:22, Conor Dooley wrote: > > On Fri, Feb 23, 2024 at 02:45:15PM +0100, Jérémie Dautheribes wrote: > > > Update Documentation/devicetree/bindings/vendor-prefixes.yaml to > > > include "cct" as a vendor prefix for "Crystal Clear Technology". CCT is > > > the vendor of the CMT430B19N00 TFT-LCD panel. > > > > > > > Acked-by: Conor Dooley > > > > And add a > > Link: http://www.cct.com.my/ > > as that actually explains why "cct" is the right choice. > > Can you send a v2 with this change ? Does your workflow not allow you to pick up Link: tags while applying patches? signature.asc Description: PGP signature
Re: [PATCH v2 0/7] Add YUV formats to VKMS
On Wed, Feb 28, 2024 at 08:42:41PM -0300, Arthur Grillo wrote: > > > On 15/01/24 12:06, Sebastian Wick wrote: > > On Wed, Jan 10, 2024 at 02:44:00PM -0300, Arthur Grillo wrote: > >> This patchset aims to add support for additional buffer YUV formats. > >> More specifically, it adds support to: > >> > >> Semi-planar formats: > >> > >> - NV12 > >> - NV16 > >> - NV24 > >> - NV21 > >> - NV61 > >> - NV42 > >> > >> Planar formats: > >> > >> - YUV440 > >> - YUV422 > >> - YUV444 > >> - YVU440 > >> - YVU422 > >> - YVU444 > >> > >> These formats have more than one plane, and most have chroma > >> subsampling. These properties don't have support on VKMS, so I had to > >> work on this before. > >> > >> To ensure that the conversions from YUV to RGB are working, I wrote a > >> KUnit test. As the work from Harry on creating KUnit tests on VKMS[1] is > >> not yet merged, I took the setup part (Kconfig entry and .kunitfile) > >> from it. > >> > >> Furthermore, I couldn't find any sources with the conversion matrices, > >> so I had to work out the values myself based on the ITU papers[2][3][4]. > >> So, I'm not 100% sure if the values are accurate. I'd appreciate some > >> input if anyone has more knowledge in this area. > > > > H.273 CICP [1] has concise descriptions of the required values for most > > widely used formats and the colour python framework [2] also can be used > > to quickly get to the desired information most of the time. > > > > [1]: https://www.itu.int/rec/T-REC-H.273 > > [2]: https://www.colour-science.org/ > > I want to thank you for suggesting the python framework, it helped > immensely now that I'm changing the precision from 8-bit to 32-bit[1]. > > If I'd known about this framework while developing this patch, I > would've saved myself a few weeks of pain and suffering recreating a > part of this library XD. I'm glad this is useful for you! We also used it for our color-and-hdr project https://gitlab.freedesktop.org/pq/color-and-hdr/. > [1]: > https://lore.kernel.org/all/b23da076-0bfb-48b2-9386-383a6dec1...@riseup.net/ > > Best Regards, > ~Arthur Grillo > > > > >> Also, I used two IGT tests to check if the formats were having a correct > >> conversion (all with the --extended flag): > >> > >> - kms_plane@pixel_format > >> - kms_plane@pixel_format_source_clamping. > >> > >> The nonsubsampled formats don't have support on IGT, so I sent a patch > >> fixing this[5]. > >> > >> Currently, this patchset does not add those formats to the writeback, as > >> it would require a rewrite of how the conversions are done (similar to > >> what was done on a previous patch[6]). So, I would like to review this > >> patchset before I start the work on this other part. > >> > >> [1]: > >> https://lore.kernel.org/all/20231108163647.106853-5-harry.wentl...@amd.com/ > >> [2]: https://www.itu.int/rec/R-REC-BT.601-7-201103-I/en > >> [3]: https://www.itu.int/rec/R-REC-BT.709-6-201506-I/en > >> [4]: https://www.itu.int/rec/R-REC-BT.2020-2-201510-I/en > >> [5]: > >> https://lists.freedesktop.org/archives/igt-dev/2024-January/066937.html > >> [6]: > >> https://lore.kernel.org/dri-devel/20230414135151.75975-2-mca...@igalia.com/ > >> > >> Signed-off-by: Arthur Grillo > >> --- > >> Changes in v2: > >> - Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test > >> file (Maxime) > >> - Link to v1: > >> https://lore.kernel.org/r/20240105-vkms-yuv-v1-0-34c4cd345...@riseup.net > >> > >> --- > >> Arthur Grillo (7): > >> drm/vkms: Use drm_frame directly > >> drm/vkms: Add support for multy-planar framebuffers > >> drm/vkms: Add range and encoding properties to pixel_read function > >> drm/vkms: Add chroma subsampling > >> drm/vkms: Add YUV support > >> drm/vkms: Drop YUV formats TODO > >> drm/vkms: Create KUnit tests for YUV conversions > >> > >> Documentation/gpu/vkms.rst| 3 +- > >> drivers/gpu/drm/vkms/Kconfig | 15 ++ > >> drivers/gpu/drm/vkms/Makefile | 1 + > >> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 + > >> drivers/gpu/drm/vkms/tests/Makefile | 3 + > >> drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 > >> drivers/gpu/drm/vkms/vkms_drv.h | 6 +- > >> drivers/gpu/drm/vkms/vkms_formats.c | 247 > >> ++ > >> drivers/gpu/drm/vkms/vkms_formats.h | 9 + > >> drivers/gpu/drm/vkms/vkms_plane.c | 26 ++- > >> drivers/gpu/drm/vkms/vkms_writeback.c | 5 - > >> 11 files changed, 426 insertions(+), 49 deletions(-) > >> --- > >> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f > >> change-id: 20231226-vkms-yuv-6f7859f32df8 > >> > >> Best regards, > >> -- > >> Arthur Grillo > >> > > >
Re: (subset) [PATCH 13/18] mfd: mt6397-core: register mt6357 sound codec
On Mon, 26 Feb 2024 15:01:51 +0100, amerg...@baylibre.com wrote: > Add MT6357 codec entry in the MFD driver. > > Applied, thanks! [13/18] mfd: mt6397-core: register mt6357 sound codec commit: 79d98102a31ab777b4a6632d799ab2bc63654cf8 -- Lee Jones [李琼斯]
Re: [PATCH] net: ethernet: ti: am65-cpsw: Add minimal XDP support
On 2/29/24 17:46, Andrew Lunn wrote:
On Thu, Feb 29, 2024 at 05:19:44PM +0100, Julien Panis wrote:
On 2/27/24 00:18, Andrew Lunn wrote:
+static struct sk_buff *am65_cpsw_alloc_skb(struct net_device *ndev, unsigned
int len)
+{
+ struct page *page;
+ struct sk_buff *skb;
+
+ page = dev_alloc_pages(0);
You are likely to get better performance if you use the page_pool.
When FEC added XDP support, the first set of changes was to make use
of page_pool. That improved the drivers performance. Then XDP was
added on top. Maybe you can follow that pattern.
Andrew
Hello Andrew,
Thanks for this suggestion. I've been working on it over the last days.
I encountered issues and I begin to wonder if that's a good option for
this driver. Let me explain...
I'm not a page pool expert, so hopefully those that are will jump in
and help.
This device has 3 ports:
- Port0 is the host port (internal to the subsystem and referred as CPPI
in the driver).
- Port1 and 2 are the external ethernet ports.
Each port's RX FIFO has 1 queue.
As mentioned in page_pool documentation:
https://docs.kernel.org/networking/page_pool.html
"The number of pools created MUST match the number of hardware
queues unless hardware restrictions make that impossible. This would
otherwise beat the purpose of page pool, which is allocate pages fast
from cache without locking."
My guess is, this last bit is the important part. Locking. Do ports 1
and port 2 rx and tx run in parallel on different CPUs? Hence do you
need locking?
No.
So, for this driver I should use 2 page pools (for port1 and 2) if possible.
Maybe, maybe not. It is not really the number of front panel interface
which matters here. It is the number of entities which need buffers.
But, if I I replace any alloc_skb() with page_pool_alloc() in the original
driver, I will have to create only 1 page_pool.
This is visible in am65_cpsw_nuss_common_open(), which starts with:
"if (common->usage_count) return 0;" to ensure that subsequent code
will be executed only once even if the 2 interfaces are ndo_open'd.
IOW, skbs will be allocated for only 1 RX channel. I checked that behavior,
and that's the way it works.
This is because the host port (CPPI) has only 1 RX queue. This single
queue is used to get all the packets, from both Ports and 2 (port ID is
stored in each descriptor).
So you have one entity which needs buffers. CPPI. It seems like Port1
and Port2 do not need buffers? So to me, you need one page pool.
Yes, only one entity (CPPI) needs buffers.
So, because of this constraint, I ended up working on the "single
page pool" option.
Questions:
1) Is the behavior described above usual for ethernet switch devices ?
This might be the first time page pool has been used by a switch? I
would check the melanox and sparx5 driver and see if they use page
pool.
It seems that sparx5 does not use page pools, mellanox does.
2) Given that I can't use a page pool for each HW queue, is it worth
using the page pool memory model ?
3) Currently I use 2 xdp_rxq (one for port1 and another one for port2).
If an XDP program is attached to port1, my page pool dma parameter
will need to be DMA_BIDIRECTIONAL (because of XDP_TX). As a result,
even if there is no XDP program attached to port2, the setting for page
pool will need to be DMA_BIDIRECTIONAL instead of DMA_FROM_DEVICE.
In such situation, isn't it a problem for port2 ?
4) Because of 1) and 2), will page pool performance really be better for
this driver ?
You probably need to explain the TX architecture a bit. How are
packets passed to the hardware? Is it again via a single CPPI entity?
Or are there two entities?
Yes, packets are passed to the hardware via a single CPPI entity.
DMA_BIDIRECTIONAL and DMA_FROM_DEVICE is about cache flushing and
invalidation. Before you pass a buffer to the hardware for it to
receive into, you need to invalidate the cache. That means when the
hardware gives the buffer back with a packet in it, there is a cache
miss and it fetches the new data from memory. If that packet gets
turned into an XDP_TX, you need to flush the cache to force any
changes out of the cache and into memory. The DMA from memory then
gets the up to date packet contents.
My guess would be, always using DMA_BIDIRECTIONAL is fine, so long as
your cache operations are correct. Turn on DMA_API_DEBUG and make sure
it is happy.
Andrew
Thank you for all these explanations.
I'll carry on working on this single page pool option, so.
Julien
Re: (subset) [PATCH RESEND 0/4] Ensure all backlight drivers zero the properties structure
On Tue, 20 Feb 2024 15:35:23 +, Daniel Thompson wrote: > [Sorry for the RESEND so soon... embarrassingly I got Lee's e-mail > address wrong the first time!] > > Luca Weiss recently shared a patch to zero the properties structure for > lm3630a... and shortly afterwards I realized I should probably scan for > a similar class of errors in other drivers. > > [...] Applied, thanks! [1/4] backlight: da9052: Fully initialize backlight_properties during probe commit: fc159b40e7980e7f78dbaa72dcc4e8f523dbfd92 [2/4] backlight: lm3639: Fully initialize backlight_properties during probe commit: aeb7ab878e90041776eae839faa117570dbcce93 [3/4] backlight: lp8788: Fully initialize backlight_properties during probe commit: cd1995b6ac7384149ad755b74e3c3eb25195ab81 -- Lee Jones [李琼斯]
Re: [PATCH 0/4] TTM unlockable restartable LRU list iteration
Hi, Christian. Thanks for having a look. On Thu, 2024-02-29 at 16:08 +0100, Christian König wrote: > Am 16.02.24 um 15:20 schrieb Thomas Hellström: > [SNIP] > > > My approach was basically to not only lock the current BO, but > > > also > > > the > > > next one. Since only a locked BO can move on the LRU we > > > effectively > > > created an anchor. > > > > > > Before I dig into the code a couple of questions: > > These are described in the patches but brief comments inline. > > > > > 1. How do you distinct BOs and iteration anchors on the LRU? > > Using a struct ttm_lru_item, containing a struct list_head and the > > type. List nodes embeds this instead of a struct list_head. This is > > larger than the list head but makes it explicit what we're doing. > > Need to look deeper into the code of this, but it would be nice if we > could abstract that better somehow. Do you have any specific concerns or improvements in mind? I think patch 1 and 2 are pretty straigthforward. Patch 3 is indeed a bit hairy. > > > > 2. How do you detect that a bulk list moved on the LRU? > > An age u64 counter on the bulk move that we're comparing against. > > It's > > updated each time it moves. > > > > > > > 3. How do you remove the iteration anchors from the bulk list? > > A function call at the end of iteration, that the function > > iterating is > > requried to call. > > Thinking quite a bit about that in the last week and I came to the > conclusion that this might be overkill. > > All BOs in a bulk share the same reservation object. So when you > acquired one you can just keep the dma-resv locked even after > evicting > the BO. > > Since moving BOs requires locking the dma-resv object the whole > problem > then just boils down to a list_for_each_element_safe(). > > That's probably a bit simpler than doing the add/remove dance. I think the problem with the "lock the next object" approach is that there are situations that it might not work. First, where not asserting anywhere that all bulk move resource have the same lock, and after individualization they certainly don't. (I think I had a patch somewhere to try to enforce that, but I don't think it ever got reviewed). I tried to sort out the locking rules at one point for resources switching bos to ghost object but I long since forgot those. I guess it all boils down to the list elements being resources, not bos. Also I'm concerned about keeping a resv held for a huge number of evictions will block out a higher priority ticket for a substantial amount of time. I think while the suggested solution here might be a bit of an overkill, it's simple enough to understand, but the locking implications of resources switching resvs arent. But please let me know how we should proceed here. This is a blocker for other pending work we have. /Thomas > > Regards, > Christian. > > > > > > > /Thomas > > > > > Regards, > > > Christian. > > > > > > > The restartable property is used in patch 4 to restart swapout > > > > if > > > > needed, but the main purpose is this paves the way for > > > > shrinker- and exhaustive eviction. > > > > > > > > Cc: Christian König > > > > Cc: > > > > > > > > Thomas Hellström (4): > > > > drm/ttm: Allow TTM LRU list nodes of different types > > > > drm/ttm: Use LRU hitches > > > > drm/ttm: Consider hitch moves within bulk sublist moves > > > > drm/ttm: Allow continued swapout after -ENOSPC falure > > > > > > > > drivers/gpu/drm/ttm/ttm_bo.c | 1 + > > > > drivers/gpu/drm/ttm/ttm_device.c | 33 +++-- > > > > drivers/gpu/drm/ttm/ttm_resource.c | 202 > > > > +++- > > > > - > > > > include/drm/ttm/ttm_resource.h | 91 +++-- > > > > 4 files changed, 267 insertions(+), 60 deletions(-) > > > >
Re: [PATCH v2 2/3] dt-bindings: display: mediatek: gamma: Add support for MT8188
On 29/02/2024 15:48, Jason-JH.Lin wrote: > The gamma LUT setting of MT8188 and MT8195 are the same, so we create > a one of items for MT8188 to reuse the driver data settings of MT8195. > > Signed-off-by: Jason-JH.Lin > Reviewed-by: AngeloGioacchino Del Regno > > --- > .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml | 4 Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 1/2] drm: panel: st7701: Add Hardkernel ODROID-GO Ultra panel support
On 26/02/2024 21:29, Jessica Zhang wrote: > Got it. Was the shorter sleep time breaking the display and is it > required for the new panel to work? > > Thanks, > > Jessica Zhang Hi Jessica, I will be submitting a v3 shortly, the change to the sleep time was not necessary for the new panel to work. I have been able to re-use the gip sequence from the kd50t048a panel used in the Hardkernel Odroid Go Super as I have been led to believe it is the same elida panel, unfortunately the same modes used by that device do not work for the Odroid Go Ultra and so its still necessary to have the patchset, Best regards, Adam
Re: Making drm_gpuvm work across gpu devices
Hi, Christian. On Thu, 2024-02-29 at 10:41 +0100, Christian König wrote: > Am 28.02.24 um 20:51 schrieb Zeng, Oak: > > > > The mail wasn’t indent/preface correctly. Manually format it. > > > > *From:*Christian König > > *Sent:* Tuesday, February 27, 2024 1:54 AM > > *To:* Zeng, Oak ; Danilo Krummrich > > ; Dave Airlie ; Daniel Vetter > > ; Felix Kuehling ; > > jgli...@redhat.com > > *Cc:* Welty, Brian ; > > dri-devel@lists.freedesktop.org; intel...@lists.freedesktop.org; > > Bommu, Krishnaiah ; Ghimiray, Himal > > Prasad > > ; > > thomas.hellst...@linux.intel.com; > > Vishwanathapura, Niranjana ; > > Brost, Matthew ; Gupta, saurabhg > > > > *Subject:* Re: Making drm_gpuvm work across gpu devices > > > > Hi Oak, > > > > Am 23.02.24 um 21:12 schrieb Zeng, Oak: > > > > Hi Christian, > > > > I go back this old email to ask a question. > > > > > > sorry totally missed that one. > > > > Quote from your email: > > > > “Those ranges can then be used to implement the SVM feature > > required for higher level APIs and not something you need at > > the > > UAPI or even inside the low level kernel memory management.” > > > > “SVM is a high level concept of OpenCL, Cuda, ROCm etc.. This > > should not have any influence on the design of the kernel > > UAPI.” > > > > There are two category of SVM: > > > > 1.driver svm allocator: this is implemented in user space, > > i.g., > > cudaMallocManaged (cuda) or zeMemAllocShared (L0) or > > clSVMAlloc(openCL). Intel already have gem_create/vm_bind in > > xekmd > > and our umd implemented clSVMAlloc and zeMemAllocShared on top > > of > > gem_create/vm_bind. Range A..B of the process address space is > > mapped into a range C..D of the GPU address space, exactly as > > you > > said. > > > > 2.system svm allocator: This doesn’t introduce extra driver > > API > > for memory allocation. Any valid CPU virtual address can be > > used > > directly transparently in a GPU program without any extra > > driver > > API call. Quote from kernel Documentation/vm/hmm.hst: “Any > > application memory region (private anonymous, shared memory, or > > regular file backed memory) can be used by a device > > transparently” > > and “to share the address space by duplicating the CPU page > > table > > in the device page table so the same address points to the same > > physical memory for any valid main memory address in the > > process > > address space”. In system svm allocator, we don’t need that > > A..B > > C..D mapping. > > > > It looks like you were talking of 1). Were you? > > > > > > No, even when you fully mirror the whole address space from a > > process > > into the GPU you still need to enable this somehow with an IOCTL. > > > > And while enabling this you absolutely should specify to which part > > of > > the address space this mirroring applies and where it maps to. > > > > */[Zeng, Oak] /* > > > > Lets say we have a hardware platform where both CPU and GPU support > > 57bit(use it for example. The statement apply to any address range) > > virtual address range, how do you decide “which part of the address > > space this mirroring applies”? You have to mirror the whole address > > space [0~2^57-1], do you? As you designed it, the gigantic > > vm_bind/mirroring happens at the process initialization time, and > > at > > that time, you don’t know which part of the address space will be > > used > > for gpu program. Remember for system allocator, *any* valid CPU > > address can be used for GPU program. If you add an offset to > > [0~2^57-1], you get an address out of 57bit address range. Is this > > a > > valid concern? > > > > Well you can perfectly mirror on demand. You just need something > similar > to userfaultfd() for the GPU. This way you don't need to mirror the > full > address space, but can rather work with large chunks created on > demand, > let's say 1GiB or something like that. What we're looking at as the current design is an augmented userptr (A..B -> C..D mapping) which is internally sparsely populated in chunks. KMD manages the population using gpu pagefaults. We acknowledge that some parts of this mirror will not have a valid CPU mapping. That is, no vma so a gpu page-fault that resolves to such a mirror address will cause an error. Would you have any concerns / objections against such an approach? Thanks, Thomas
Re: [PATCH 1/2] drm_edid: Add a function to get EDID base block
Hi, On Thu, Feb 29, 2024 at 8:43 AM Jani Nikula wrote: > > On Wed, 28 Feb 2024, Doug Anderson wrote: > > Hi, > > > > On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang wrote: > >> > >> On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula > >> wrote: > >> > > >> > On Fri, 23 Feb 2024, Hsin-Yi Wang wrote: > >> > > It's found that some panels have variants that they share the same > >> > > panel id > >> > > although their EDID and names are different. Besides panel id, now we > >> > > need > >> > > the hash of entire EDID base block to distinguish these panel variants. > >> > > > >> > > Add drm_edid_get_base_block to returns the EDID base block, so caller > >> > > can > >> > > further use it to get panel id and/or the hash. > >> > > >> > Please reconsider the whole approach here. > >> > > >> > Please let's not add single-use special case functions to read an EDID > >> > base block. > >> > > >> > Please consider reading the whole EDID, using the regular EDID reading > >> > functions, and use that instead. > >> > > >> > Most likely you'll only have 1-2 blocks anyway. And you might consider > >> > caching the EDID in struct panel_edp if reading the entire EDID is too > >> > slow. (And if it is, this is probably sensible even if the EDID only > >> > consists of one block.) > >> > > >> > Anyway, please do *not* merge this as-is. > >> > > >> > >> hi Jani, > >> > >> I sent a v2 here implementing this method: > >> https://lore.kernel.org/lkml/20240228011133.1238439-2-hsi...@chromium.org/ > >> > >> We still have to read edid twice due to: > >> 1. The first caller is in panel probe, at that time, connector is > >> still unknown, so we can't update connector status (eg. update > >> edid_corrupt). > >> 2. It's possible that the connector can have some override > >> (drm_edid_override_get) to EDID, that is still unknown during the > >> first read. > > > > I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the > > fact that we can't cache the EDID (because we don't yet have a > > "drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that > > allows reading just the first block. If we try to boot a device with a > > multi-block EDID we're now wastefully reading all the blocks of the > > EDID twice at bootup which will slow boot time. > > > > If you can see a good solution to avoid reading the EDID twice then > > that would be amazing, but if not it seems like we should go back to > > what's here in v1. What do you think? Anyone else have any opinions? > > I haven't replied so far, because I've been going back and forth with > this. I'm afraid I don't really like either approach now. Handling the > no connector case in v2 is a bit ugly too. :( > > Seems like you only need this to extend the panel ID with a hash. And > panel-edp.c is the only user of drm_edid_get_panel_id(). And EDID quirks > in drm_edid.c could theoretically hit the same problem you're solving. Good point. That would imply that more of the logic could go into "drm_edid.c" in case the EDID quirks code eventually needs it. > So maybe something like: > > u32 drm_edid_get_panel_id(struct i2c_adapter *adapter, u32 *hash); > > or if you want to be fancy add a struct capturing both id and hash: > > bool drm_edid_get_panel_id(struct i2c_adapter *adapter, struct > drm_edid_panel_id *id); > > And put the hash (or whatever mechanism you have) computation in > drm_edid.c. Just hide it all in drm_edid.c, and keep the EDID interfaces > neat. > > How would that work for you? The problem is that Dmitry didn't like the idea of using a hash and in v2 Hsin-Yi has moved to using the name of the display. ...except of course that eDP panels don't always properly specify "EDID_DETAIL_MONITOR_NAME". See the discussion [1]. If you want to see some of the EDIDs involved, you can see Hsin-Yi's post [2]. The panels included stuff like this: Alphanumeric Data String: 'AUO' Alphanumeric Data String: 'B116XAN04.0 ' The fact that there is more than one string in there makes it hard to just "return" the display name in a generic way. The way Hsin-Yi's code was doing it was that it would consider it a match if the panel name was in any of the strings... How about this as a solution: we change drm_edid_get_panel_id() to return an opaque type (struct drm_edid_panel_id_blob) that's really just the first block of the EDID but we can all pretend that it isn't. Then we can add a function in drm_edid.c that takes this opaque blob, a 32-bit integer (as per drm_edid_encode_panel_id()), and a string name and it can tell us if the blob matches? [1] https://lore.kernel.org/r/CAD=FV=vmvr+ej7eyulga671fmgh6zx9zpokbkzyj0h79mz2...@mail.gmail.com [2] https://lore.kernel.org/r/cajmqk-gfkbdphyjecj5ux0dnrx3y-emlstiv9nj+u3rmej3...@mail.gmail.com
[PATCH v14 6/9] drm/i915/display: Compute AS SDP parameters
Add necessary function definitions to compute AS SDP data.
The new intel_dp_compute_as_sdp function computes AS SDP
values based on the display configuration, ensuring proper
handling of Variable Refresh Rate (VRR).
--v2:
- Added DP_SDP_ADAPTIVE_SYNC to infoframe_type_to_idx(). [Ankit]
- Separated patch for intel_read/write_dp_sdp. [Ankit]
- _HSW_VIDEO_DIP_ASYNC_DATA_A should be from ADL onward. [Ankit]
- Fixed indentation issues. [Ankit]
--v3:
- Added VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes.
--v4:
- Added HAS_VRR check before writing AS SDP.
--v5:
Added missed HAS_VRR check before reading AS SDP.
--v6:
- Used Adaptive Sync sink status as a check for read/write SDP. (Ankit)
--v7:
- Remove as_sdp_enable from crtc_state.
- Add a comment mentioning current support of
DP_AS_SDP_AVT_FIXED_VTOTAL.
- Add state checker for AS_SDP infoframe enable.
--v8:
- Drop conn_state from intel_dp_compute_as_sdp, as not used.
- Remove fullstop in subject line.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_dp.c | 26 +
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index b26efce4a041..86de854516ef 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2626,6 +2626,31 @@ static void intel_dp_compute_vsc_colorimetry(const
struct intel_crtc_state *crtc
vsc->content_type = DP_CONTENT_TYPE_NOT_DEFINED;
}
+static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_dp_as_sdp *as_sdp = _state->infoframes.as_sdp;
+ struct intel_connector *connector = intel_dp->attached_connector;
+ const struct drm_display_mode *adjusted_mode =
+ _state->hw.adjusted_mode;
+ int vrefresh = drm_mode_vrefresh(adjusted_mode);
+
+ if (!intel_vrr_is_in_range(connector, vrefresh) ||
+ !intel_dp_as_sdp_supported(intel_dp))
+ return;
+
+ crtc_state->infoframes.enable |=
intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
+
+ /* Currently only DP_AS_SDP_AVT_FIXED_VTOTAL mode supported */
+ as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC;
+ as_sdp->length = 0x9;
+ as_sdp->mode = DP_AS_SDP_AVT_FIXED_VTOTAL;
+ as_sdp->vtotal = adjusted_mode->vtotal;
+ as_sdp->target_rr = 0;
+ as_sdp->duration_incr_ms = 0;
+ as_sdp->duration_incr_ms = 0;
+}
+
static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state,
const struct drm_connector_state
*conn_state)
@@ -2951,6 +2976,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
g4x_dp_set_clock(encoder, pipe_config);
intel_vrr_compute_config(pipe_config, conn_state);
+ intel_dp_compute_as_sdp(intel_dp, pipe_config);
intel_psr_compute_config(intel_dp, pipe_config, conn_state);
intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16);
intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
--
2.25.1
[PATCH v14 8/9] drm/i915/display: Compute vrr_vsync params
Compute vrr_vsync_start/end, which sets the position
for hardware to send the Vsync at a fixed position
relative to the end of the Vblank.
--v2:
- Updated VSYNC_START/END macros to VRR_VSYNC_START/END. (Ankit)
- Updated bit fields of VRR_VSYNC_START/END. (Ankit)
--v3:
- Add PIPE_CONF_CHECK_I(vrr.vsync_start/end).
- Read/write vrr_vsync params only when we intend to send
adaptive_sync sdp.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_display.c | 2 ++
.../drm/i915/display/intel_display_types.h| 1 +
drivers/gpu/drm/i915/display/intel_vrr.c | 29 +--
drivers/gpu/drm/i915/i915_reg.h | 7 +
4 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index be0a5fae4e58..c523eec4d626 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5367,6 +5367,8 @@ intel_pipe_config_compare(const struct intel_crtc_state
*current_config,
PIPE_CONF_CHECK_I(vrr.flipline);
PIPE_CONF_CHECK_I(vrr.pipeline_full);
PIPE_CONF_CHECK_I(vrr.guardband);
+ PIPE_CONF_CHECK_I(vrr.vsync_start);
+ PIPE_CONF_CHECK_I(vrr.vsync_end);
}
#undef PIPE_CONF_CHECK_X
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1256730ea276..45b30d3ceb06 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1417,6 +1417,7 @@ struct intel_crtc_state {
bool enable, in_range;
u8 pipeline_full;
u16 flipline, vmin, vmax, guardband;
+ u32 vsync_end, vsync_start;
} vrr;
/* Stream Splitter for eDP MSO */
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 5d905f932cb4..d24a42902e69 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -9,6 +9,7 @@
#include "intel_de.h"
#include "intel_display_types.h"
#include "intel_vrr.h"
+#include "intel_dp.h"
bool intel_vrr_is_capable(struct intel_connector *connector)
{
@@ -113,6 +114,7 @@ intel_vrr_compute_config(struct intel_crtc_state
*crtc_state,
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
struct intel_connector *connector =
to_intel_connector(conn_state->connector);
+ struct intel_dp *intel_dp = intel_attached_dp(connector);
struct drm_display_mode *adjusted_mode = _state->hw.adjusted_mode;
const struct drm_display_info *info = >base.display_info;
int vmin, vmax;
@@ -165,6 +167,15 @@ intel_vrr_compute_config(struct intel_crtc_state
*crtc_state,
if (crtc_state->uapi.vrr_enabled) {
crtc_state->vrr.enable = true;
crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
+
+ if (intel_dp_as_sdp_supported(intel_dp)) {
+ crtc_state->vrr.vsync_start =
+ (crtc_state->hw.adjusted_mode.crtc_vtotal -
+
VRR_VSYNC_START(crtc_state->hw.adjusted_mode.vsync_start));
+ crtc_state->vrr.vsync_end =
+ (crtc_state->hw.adjusted_mode.crtc_vtotal -
+
(VRR_VSYNC_END(crtc_state->hw.adjusted_mode.vsync_end) >> 16));
+ }
}
}
@@ -203,6 +214,10 @@ void intel_vrr_set_transcoder_timings(const struct
intel_crtc_state *crtc_state)
intel_de_write(dev_priv, TRANS_VRR_VMAX(cpu_transcoder),
crtc_state->vrr.vmax - 1);
intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
trans_vrr_ctl(crtc_state));
intel_de_write(dev_priv, TRANS_VRR_FLIPLINE(cpu_transcoder),
crtc_state->vrr.flipline - 1);
+
+ if (crtc_state->vrr.vsync_end && crtc_state->vrr.vsync_start)
+ intel_de_write(dev_priv, TRANS_VRR_VSYNC(cpu_transcoder),
+ crtc_state->vrr.vsync_end << 16 |
crtc_state->vrr.vsync_start);
}
void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
@@ -263,7 +278,7 @@ void intel_vrr_get_config(struct intel_crtc_state
*crtc_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- u32 trans_vrr_ctl;
+ u32 trans_vrr_ctl, trans_vrr_vsync;
trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
@@ -283,6 +298,16 @@ void intel_vrr_get_config(struct intel_crtc_state
*crtc_state)
crtc_state->vrr.vmin = intel_de_read(dev_priv,
TRANS_VRR_VMIN(cpu_transcoder)) + 1;
}
- if (crtc_state->vrr.enable)
+ if (crtc_state->vrr.enable) {
crtc_state->mode_flags |=
[PATCH v14 9/9] drm/i915/display: Read/Write AS sdp only when sink/source has enabled
Write/Read Adaptive sync SDP only when Sink and Source is enabled for the same. Also along with write TRANS_VRR_VSYNC values. Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_ddi.c | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index bea441590204..6b8088321582 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3971,6 +3971,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder, intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA); intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); + intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC); intel_audio_codec_get_config(encoder, pipe_config); } diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 86de854516ef..9309abeda241 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4294,6 +4294,7 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, return; intel_write_dp_sdp(encoder, crtc_state, DP_SDP_VSC); + intel_write_dp_sdp(encoder, crtc_state, DP_SDP_ADAPTIVE_SYNC); intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } -- 2.25.1
[PATCH v14 4/9] drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
Add the necessary structures and functions to handle reading and
unpacking Adaptive Sync Secondary Data Packets. Also add support
to write and pack AS SDP.
--v2:
- Correct use of REG_BIT and REG_GENMASK. [Jani]
- Use as_sdp instead of async. [Jani]
- Remove unrelated comments and changes. [Jani]
- Correct code indent. [Jani]
--v3:
- Update definition names for AS SDP which are starting from
HSW, as these defines are applicable for ADLP+.(Ankit)
--v4:
- Remove as_sdp_mode from crtc_state.
- Drop metadata keyword.
- For consistency, update ADL_ prefix or post fix as required.
--v5:
- Check if AS_SDP bit is set in crtc_state->infoframes.enable. If not
return.
- Check for HAS_AS_SDP() before setting VIDEO_DIP_ENABLE_AS_ADL mask.
Signed-off-by: Mitul Golani
---
.../drm/i915/display/intel_display_device.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 91 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 12 ++-
drivers/gpu/drm/i915/i915_reg.h | 8 ++
include/drm/display/drm_dp_helper.h | 2 +-
5 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
b/drivers/gpu/drm/i915/display/intel_display_device.h
index fe4268813786..6399fbc6c738 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -68,6 +68,7 @@ struct drm_printer;
#define HAS_TRANSCODER(i915, trans)
((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
BIT(trans)) != 0)
#define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
+#define HAS_AS_SDP(i915) (DISPLAY_VER(i915) >= 13)
#define INTEL_NUM_PIPES(i915)
(hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
#define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)->has_hotplug)
#define OVERLAY_NEEDS_PHYSICAL(i915)
(DISPLAY_INFO(i915)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index e13121dc3a03..7cf849015797 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4089,6 +4089,32 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state
*crtc_state,
return false;
}
+static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *as_sdp,
+ struct dp_sdp *sdp, size_t size)
+{
+ size_t length = sizeof(struct dp_sdp);
+
+ if (size < length)
+ return -ENOSPC;
+
+ memset(sdp, 0, size);
+
+ /* Prepare AS (Adaptive Sync) SDP Header */
+ sdp->sdp_header.HB0 = 0;
+ sdp->sdp_header.HB1 = as_sdp->sdp_type;
+ sdp->sdp_header.HB2 = 0x02;
+ sdp->sdp_header.HB3 = as_sdp->length;
+
+ /* Fill AS (Adaptive Sync) SDP Payload */
+ sdp->db[0] = as_sdp->mode;
+ sdp->db[1] = as_sdp->vtotal & 0xFF;
+ sdp->db[2] = (as_sdp->vtotal >> 8) & 0xFF;
+ sdp->db[3] = as_sdp->target_rr;
+ sdp->db[4] = (as_sdp->target_rr >> 8) & 0x3;
+
+ return length;
+}
+
static ssize_t
intel_dp_hdr_metadata_infoframe_sdp_pack(struct drm_i915_private *i915,
const struct hdmi_drm_infoframe
*drm_infoframe,
@@ -4188,6 +4214,10 @@ static void intel_write_dp_sdp(struct intel_encoder
*encoder,
_state->infoframes.drm.drm,
,
sizeof(sdp));
break;
+ case DP_SDP_ADAPTIVE_SYNC:
+ len = intel_dp_as_sdp_pack(_state->infoframes.as_sdp, ,
+ sizeof(sdp));
+ break;
default:
MISSING_CASE(type);
return;
@@ -4209,6 +4239,10 @@ void intel_dp_set_infoframes(struct intel_encoder
*encoder,
u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW |
VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK;
+
+ if (HAS_AS_SDP(dev_priv))
+ dip_enable |= VIDEO_DIP_ENABLE_AS_ADL;
+
u32 val = intel_de_read(dev_priv, reg) & ~dip_enable;
/* TODO: Sanitize DSC enabling wrt. intel_dsc_dp_pps_write(). */
@@ -4230,6 +4264,36 @@ void intel_dp_set_infoframes(struct intel_encoder
*encoder,
intel_write_dp_sdp(encoder, crtc_state,
HDMI_PACKET_TYPE_GAMUT_METADATA);
}
+static
+int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *as_sdp,
+ const void *buffer, size_t size)
+{
+ const struct dp_sdp *sdp = buffer;
+
+ if (size < sizeof(struct dp_sdp))
+ return -EINVAL;
+
+ memset(as_sdp, 0, sizeof(*as_sdp));
+
+ if (sdp->sdp_header.HB0 != 0)
+ return -EINVAL;
+
+ if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC)
+
[PATCH v14 7/9] drm/i915/display: Add state checker for Adaptive Sync SDP
Enable infoframe and add state checker for Adaptive Sync
SDP enablement.
--v1:
- crtc_state->infoframes.enable, to add on correct place holder.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_display.c | 46
1 file changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 00ac65a14029..be0a5fae4e58 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4781,6 +4781,17 @@ intel_compare_dp_vsc_sdp(const struct drm_dp_vsc_sdp *a,
a->content_type == b->content_type;
}
+static bool
+intel_compare_dp_as_sdp(const struct drm_dp_as_sdp *a,
+ const struct drm_dp_as_sdp *b)
+{
+ return a->vtotal == b->vtotal &&
+ a->target_rr == b->target_rr &&
+ a->duration_incr_ms == b->duration_incr_ms &&
+ a->duration_decr_ms == b->duration_decr_ms &&
+ a->mode == b->mode;
+}
+
static bool
intel_compare_buffer(const u8 *a, const u8 *b, size_t len)
{
@@ -4836,6 +4847,30 @@ pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private
*i915,
drm_dp_vsc_sdp_log(, b);
}
+static void
+pipe_config_dp_as_sdp_mismatch(struct drm_i915_private *i915,
+ bool fastset, const char *name,
+ const struct drm_dp_as_sdp *a,
+ const struct drm_dp_as_sdp *b)
+{
+ struct drm_printer p;
+
+ if (fastset) {
+ p = drm_dbg_printer(>drm, DRM_UT_KMS, NULL);
+
+ drm_printf(, "fastset requirement not met in %s dp sdp\n",
name);
+ } else {
+ p = drm_err_printer(>drm, NULL);
+
+ drm_printf(, "mismatch in %s dp sdp\n", name);
+ }
+
+ drm_printf(, "expected:\n");
+ drm_dp_as_sdp_log(, a);
+ drm_printf(, "found:\n");
+ drm_dp_as_sdp_log(, b);
+}
+
/* Returns the length up to and including the last differing byte */
static size_t
memcmp_diff_len(const u8 *a, const u8 *b, size_t len)
@@ -5089,6 +5124,16 @@ intel_pipe_config_compare(const struct intel_crtc_state
*current_config,
} \
} while (0)
+#define PIPE_CONF_CHECK_DP_AS_SDP(name) do { \
+ if (!intel_compare_dp_as_sdp(_config->infoframes.name, \
+ _config->infoframes.name)) { \
+ pipe_config_dp_as_sdp_mismatch(dev_priv, fastset,
__stringify(name), \
+
_config->infoframes.name, \
+ _config->infoframes.name);
\
+ ret = false; \
+ } \
+} while (0)
+
#define PIPE_CONF_CHECK_BUFFER(name, len) do { \
BUILD_BUG_ON(sizeof(current_config->name) != (len)); \
BUILD_BUG_ON(sizeof(pipe_config->name) != (len)); \
@@ -5270,6 +5315,7 @@ intel_pipe_config_compare(const struct intel_crtc_state
*current_config,
PIPE_CONF_CHECK_INFOFRAME(hdmi);
PIPE_CONF_CHECK_INFOFRAME(drm);
PIPE_CONF_CHECK_DP_VSC_SDP(vsc);
+ PIPE_CONF_CHECK_DP_AS_SDP(as_sdp);
PIPE_CONF_CHECK_X(sync_mode_slaves_mask);
PIPE_CONF_CHECK_I(master_transcoder);
--
2.25.1
[PATCH v14 5/9] drm/i915/dp: Add wrapper function to check AS SDP
Add a wrapper function to check if both the source and
sink support Adaptive Sync SDP.
--v1:
Just use drm/i915/dp in subject line.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/i915/display/intel_dp.c | 8
drivers/gpu/drm/i915/display/intel_dp.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 7cf849015797..b26efce4a041 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -120,6 +120,14 @@ bool intel_dp_is_edp(struct intel_dp *intel_dp)
return dig_port->base.type == INTEL_OUTPUT_EDP;
}
+bool intel_dp_as_sdp_supported(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+ return HAS_AS_SDP(i915) &&
+ drm_dp_as_sdp_supported(_dp->aux, intel_dp->dpcd);
+}
+
static void intel_dp_unset_edid(struct intel_dp *intel_dp);
/* Is link rate UHBR and thus 128b/132b? */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 530cc97bc42f..cc5e069091ff 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -80,6 +80,7 @@ void intel_dp_audio_compute_config(struct intel_encoder
*encoder,
struct drm_connector_state *conn_state);
bool intel_dp_has_hdmi_sink(struct intel_dp *intel_dp);
bool intel_dp_is_edp(struct intel_dp *intel_dp);
+bool intel_dp_as_sdp_supported(struct intel_dp *intel_dp);
bool intel_dp_is_uhbr(const struct intel_crtc_state *crtc_state);
int intel_dp_link_symbol_size(int rate);
int intel_dp_link_symbol_clock(int rate);
--
2.25.1
[PATCH v14 3/9] drm: Add crtc state dump for Adaptive Sync SDP
Add crtc state dump for Adaptive Sync SDP to know which
crtc specifically caused the failure.
Signed-off-by: Mitul Golani
---
.../gpu/drm/i915/display/intel_crtc_state_dump.c| 13 +
drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
2 files changed, 14 insertions(+)
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 4bcf446c75f4..1e4618271156 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -51,6 +51,15 @@ intel_dump_infoframe(struct drm_i915_private *i915,
hdmi_infoframe_log(KERN_DEBUG, i915->drm.dev, frame);
}
+static void
+intel_dump_dp_as_sdp(struct drm_i915_private *i915,
+const struct drm_dp_as_sdp *as_sdp)
+{
+ struct drm_printer p = drm_dbg_printer(>drm, DRM_UT_KMS,
"AS_SDP");
+
+ drm_dp_as_sdp_log(, as_sdp);
+}
+
static void
intel_dump_dp_vsc_sdp(struct drm_i915_private *i915,
const struct drm_dp_vsc_sdp *vsc)
@@ -302,6 +311,10 @@ void intel_crtc_state_dump(const struct intel_crtc_state
*pipe_config,
if (pipe_config->infoframes.enable &
intel_hdmi_infoframe_enable(DP_SDP_VSC))
intel_dump_dp_vsc_sdp(i915, _config->infoframes.vsc);
+ if (pipe_config->infoframes.enable &
+ intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC))
+ intel_dump_dp_as_sdp(i915, _config->infoframes.as_sdp);
+
if (pipe_config->has_audio)
intel_dump_buffer(i915, "ELD: ", pipe_config->eld,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8ce986fadd9a..1256730ea276 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1335,6 +1335,7 @@ struct intel_crtc_state {
union hdmi_infoframe hdmi;
union hdmi_infoframe drm;
struct drm_dp_vsc_sdp vsc;
+ struct drm_dp_as_sdp as_sdp;
} infoframes;
u8 eld[MAX_ELD_BYTES];
--
2.25.1
[PATCH v14 1/9] drm/dp: Add support to indicate if sink supports AS SDP
Add an API that indicates support for Adaptive Sync SDP in
the sink, which can be utilized by the rest of the DP programming.
--v1:
- Format commit message properly.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/display/drm_dp_helper.c | 25 +
include/drm/display/drm_dp_helper.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index 9ac52cf5d4d8..f94c04db7187 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2913,6 +2913,31 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const
struct drm_dp_vsc_sdp *vsc)
}
EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
+/**
+ * drm_dp_as_sdp_supported() - check if adaptive sync sdp is supported
+ * @aux: DisplayPort AUX channel
+ * @dpcd: DisplayPort configuration data
+ *
+ * Returns true if adaptive sync sdp is supported, else returns false
+ */
+bool drm_dp_as_sdp_supported(struct drm_dp_aux *aux, const u8
dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ u8 rx_feature;
+
+ if (dpcd[DP_DPCD_REV] < DP_DPCD_REV_13)
+ return false;
+
+ if (drm_dp_dpcd_readb(aux, DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1,
+ _feature) != 1) {
+ drm_dbg_dp(aux->drm_dev,
+ "Failed to read
DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1\n");
+ return false;
+ }
+
+ return (rx_feature & DP_ADAPTIVE_SYNC_SDP_SUPPORTED);
+}
+EXPORT_SYMBOL(drm_dp_as_sdp_supported);
+
/**
* drm_dp_vsc_sdp_supported() - check if vsc sdp is supported
* @aux: DisplayPort AUX channel
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index 0c1a4021e098..7c1aa3a703c8 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -101,6 +101,7 @@ struct drm_dp_vsc_sdp {
void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp
*vsc);
bool drm_dp_vsc_sdp_supported(struct drm_dp_aux *aux, const u8
dpcd[DP_RECEIVER_CAP_SIZE]);
+bool drm_dp_as_sdp_supported(struct drm_dp_aux *aux, const u8
dpcd[DP_RECEIVER_CAP_SIZE]);
int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
--
2.25.1
[PATCH v14 2/9] drm: Add Adaptive Sync SDP logging
Add structure representing Adaptive Sync Secondary Data Packet (AS SDP).
Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate
debugging.
--v2:
- Update logging. [Jani, Ankit]
- Use 'as_sdp' instead of 'async' [Ankit]
- Correct define placeholders to where they are actually used. [Jani]
- Update members in 'as_sdp' structure to make it uniform. [Jani]
--v3:
- Added changes to dri-devel mailing list. No code changes.
--v4:
- Instead of directly using operation mode, use an enum to accommodate
all operation modes (Ankit).
--v5:
Nit-pick changes to commit message.
--v6:
- Add correct place holder and name change for AS_SDP_OP_MODE.
- Separate i915 changes from drm changes.
- Remove extra lines.
Signed-off-by: Mitul Golani
---
drivers/gpu/drm/display/drm_dp_helper.c | 12 ++
include/drm/display/drm_dp.h| 10 +
include/drm/display/drm_dp_helper.h | 29 +
3 files changed, 51 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index f94c04db7187..b1459ac92aea 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2913,6 +2913,18 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const
struct drm_dp_vsc_sdp *vsc)
}
EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
+void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp
*as_sdp)
+{
+ drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n",
+ as_sdp->revision, as_sdp->length);
+ drm_printf(p, "vtotal: %d\n", as_sdp->vtotal);
+ drm_printf(p, "target_rr: %d\n", as_sdp->target_rr);
+ drm_printf(p, "duration_incr_ms: %d\n", as_sdp->duration_incr_ms);
+ drm_printf(p, "duration_decr_ms: %d\n", as_sdp->duration_decr_ms);
+ drm_printf(p, "operation_mode: %d\n", as_sdp->mode);
+}
+EXPORT_SYMBOL(drm_dp_as_sdp_log);
+
/**
* drm_dp_as_sdp_supported() - check if adaptive sync sdp is supported
* @aux: DisplayPort AUX channel
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 281afff6ee4e..ba388ad48ade 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -1149,6 +1149,7 @@
#define DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1 0x2214 /* 2.0 E11 */
# define DP_ADAPTIVE_SYNC_SDP_SUPPORTED(1 << 0)
+# define DP_ADAPTIVE_SYNC_SDP_OPERATION_MODE GENMASK(1, 0)
# define DP_AS_SDP_FIRST_HALF_LINE_OR_3840_PIXEL_CYCLE_WINDOW_NOT_SUPPORTED (1
<< 1)
# define DP_VSC_EXT_SDP_FRAMEWORK_VERSION_1_SUPPORTED (1 << 4)
@@ -1578,10 +1579,12 @@ enum drm_dp_phy {
#define DP_SDP_AUDIO_COPYMANAGEMENT0x05 /* DP 1.2 */
#define DP_SDP_ISRC0x06 /* DP 1.2 */
#define DP_SDP_VSC 0x07 /* DP 1.2 */
+#define DP_SDP_ADAPTIVE_SYNC 0x22 /* DP 1.4 */
#define DP_SDP_CAMERA_GENERIC(i) (0x08 + (i)) /* 0-7, DP 1.3 */
#define DP_SDP_PPS 0x10 /* DP 1.4 */
#define DP_SDP_VSC_EXT_VESA0x20 /* DP 1.4 */
#define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */
+
/* 0x80+ CEA-861 infoframe types */
#define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b
@@ -1737,4 +1740,11 @@ enum dp_content_type {
DP_CONTENT_TYPE_GAME = 0x04,
};
+enum operation_mode {
+ DP_AS_SDP_AVT_DYNAMIC_VTOTAL = 0x00,
+ DP_AS_SDP_AVT_FIXED_VTOTAL = 0x01,
+ DP_AS_SDP_FAVT_TRR_NOT_REACHED = 0x02,
+ DP_AS_SDP_FAVT_TRR_REACHED = 0x03
+};
+
#endif /* _DRM_DP_H_ */
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index 7c1aa3a703c8..94eb0d92abae 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -98,6 +98,35 @@ struct drm_dp_vsc_sdp {
enum dp_content_type content_type;
};
+/**
+ * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP
+ *
+ * This structure represents a DP AS SDP of drm
+ * It is based on DP 2.1 spec [Table 2-126: Adaptive-Sync SDP Header Bytes]
and
+ * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8]
+ *
+ * @sdp_type: Secondary-data packet type
+ * @revision: Revision Number
+ * @length: Number of valid data bytes
+ * @vtotal: Minimum Vertical Vtotal
+ * @target_rr: Target Refresh
+ * @duration_incr_ms: Successive frame duration increase
+ * @duration_decr_ms: Successive frame duration decrease
+ * @operation_mode: Adaptive Sync Operation Mode
+ */
+struct drm_dp_as_sdp {
+ unsigned char sdp_type;
+ unsigned char revision;
+ unsigned char length;
+ int vtotal;
+ int target_rr;
+ int duration_incr_ms;
+ int duration_decr_ms;
+ enum operation_mode mode;
+};
+
+void drm_dp_as_sdp_log(struct drm_printer *p,
+ const struct drm_dp_as_sdp *as_sdp);
void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp
*vsc);
bool drm_dp_vsc_sdp_supported(struct drm_dp_aux *aux, const u8
[PATCH v14 0/9] Enable Adaptive Sync SDP Support for DP
An Adaptive-Sync-capable DP protocol converter indicates its support by setting the related bit in the DPCD register. Computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR) in the context of Adaptive Sync. --v2: - Update logging to Patch-1 - use as_sdp instead of async - Put definitions to correct placeholders from where it is defined. - Update member types of as_sdp for uniformity. - Correct use of REG_BIT and REG_GENMASK. - Remove unrelated comments and changes. - Correct code indents. - separate out patch changes for intel_read/write_dp_sdp. --v3: - Add VIDEO_DIP_ASYNC_DATA_SIZE definition and comment in as_sdp_pack function to patch 2 as originally used there. [Patch 2]. - Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes [Patch 3]. --v4: - Add check for HAS_VRR before writing AS SDP. [Patch 3]. --v5: - Add missing check for HAS_VRR before reading AS SDP as well [Patch 3]. --v6: - Rebase all patches. - Compute TRANS_VRR_VSYNC. -v7: - Move vrr_vsync_start/end to compute config. - Use correct function for drm_debug_printer. -v8: - Code refactoring. - Update, VSYNC_START/END macros to VRR_VSYNC_START/END.(Ankit) - Update bit fields of VRR_VSYNC_START/END.(Ankit) - Send patches to dri-devel.(Ankit) - Update definition names for AS SDP which are starting from HSW, as these defines are applicable for ADLP+.(Ankit) - Remove unused bitfield define, AS_SDP_ENABLE. - Add support in drm for Adaptive Sync sink status, which can be used later as a check for read/write sdp. (Ankit) -v9: - Add enum to operation mode to represent different AVT and FAVT modes. (Ankit) - Operation_mode, target_rr etc should be filled from as_sdp struct. (Ankit) - Fill as_sdp->*All Params* from compute config, read from the sdp. (Ankit) - Move configs to the appropriate patch where it used first.(Ankit) - There should be a check if as sdp is enable is set or not. (Ankit) - Add variables in crtc state->vrr for ad sdp enable and operation mode. (Ankit) - Use above variables for tracking AS SDP. (Ankit) - Revert unused changes. (Ankit) -v10: - Send Patches to dri-devel (Ankit). -v11: - Remove as_sdp_mode and enable from crtc_state. - For consistency, update ADL_ prefix or post fix as required. - Add a comment mentioning current support of DP_AS_SDP_AVT_FIXED_VTOTAL. - Add state checker for AS_SDP infoframe enable. - Add PIPE_CONF_CHECK_I(vrr.vsync_start/end). - Read/write vrr_vsync params only when we intend to send adaptive_sync sdp. -v12: - Update cover letter -v13: - Add correct place holder and name change for AS_SDP_OP_MODE. - Separate i915 changes from drm changes. - Remove extra lines. - Check if AS_SDP bit is set in crtc_state->infoframes.enable. If not return. - Check for HAS_AS_SDP() before setting VIDEO_DIP_ENABLE_AS_ADL mask. - Just use drm/i915/dp in subject line. - Drop conn_state from intel_dp_compute_as_sdp, as not used. - Remove fullstop in subject line. - crtc_state->infoframes.enable, to add on correct place holder. --v14: Mistakenly dropped first patch, adding back. Signed-off-by: Mitul Golani i Mitul Golani (9): drm/dp: Add support to indicate if sink supports AS SDP drm: Add Adaptive Sync SDP logging drm: Add crtc state dump for Adaptive Sync SDP drm/i915/dp: Add Read/Write support for Adaptive Sync SDP drm/i915/dp: Add wrapper function to check AS SDP drm/i915/display: Compute AS SDP parameters drm/i915/display: Add state checker for Adaptive Sync SDP drm/i915/display: Compute vrr_vsync params drm/i915/display: Read/Write AS sdp only when sink/source has enabled drivers/gpu/drm/display/drm_dp_helper.c | 37 + .../drm/i915/display/intel_crtc_state_dump.c | 13 ++ drivers/gpu/drm/i915/display/intel_ddi.c | 1 + drivers/gpu/drm/i915/display/intel_display.c | 48 +++ .../drm/i915/display/intel_display_device.h | 1 + .../drm/i915/display/intel_display_types.h| 2 + drivers/gpu/drm/i915/display/intel_dp.c | 126 ++ drivers/gpu/drm/i915/display/intel_dp.h | 1 + drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +- drivers/gpu/drm/i915/display/intel_vrr.c | 29 +++- drivers/gpu/drm/i915/i915_reg.h | 15 +++ include/drm/display/drm_dp.h | 10 ++ include/drm/display/drm_dp_helper.h | 30 + 13 files changed, 322 insertions(+), 3 deletions(-) -- 2.25.1
Re: [PATCH] net: ethernet: ti: am65-cpsw: Add minimal XDP support
On Thu, Feb 29, 2024 at 05:19:44PM +0100, Julien Panis wrote:
> On 2/27/24 00:18, Andrew Lunn wrote:
> > > +static struct sk_buff *am65_cpsw_alloc_skb(struct net_device *ndev,
> > > unsigned int len)
> > > +{
> > > + struct page *page;
> > > + struct sk_buff *skb;
> > > +
> > > + page = dev_alloc_pages(0);
> > You are likely to get better performance if you use the page_pool.
> >
> > When FEC added XDP support, the first set of changes was to make use
> > of page_pool. That improved the drivers performance. Then XDP was
> > added on top. Maybe you can follow that pattern.
> >
> >Andrew
>
> Hello Andrew,
>
> Thanks for this suggestion. I've been working on it over the last days.
> I encountered issues and I begin to wonder if that's a good option for
> this driver. Let me explain...
I'm not a page pool expert, so hopefully those that are will jump in
and help.
> This device has 3 ports:
> - Port0 is the host port (internal to the subsystem and referred as CPPI
> in the driver).
> - Port1 and 2 are the external ethernet ports.
> Each port's RX FIFO has 1 queue.
>
> As mentioned in page_pool documentation:
> https://docs.kernel.org/networking/page_pool.html
> "The number of pools created MUST match the number of hardware
> queues unless hardware restrictions make that impossible. This would
> otherwise beat the purpose of page pool, which is allocate pages fast
> from cache without locking."
My guess is, this last bit is the important part. Locking. Do ports 1
and port 2 rx and tx run in parallel on different CPUs? Hence do you
need locking?
> So, for this driver I should use 2 page pools (for port1 and 2) if possible.
Maybe, maybe not. It is not really the number of front panel interface
which matters here. It is the number of entities which need buffers.
> But, if I I replace any alloc_skb() with page_pool_alloc() in the original
> driver, I will have to create only 1 page_pool.
> This is visible in am65_cpsw_nuss_common_open(), which starts with:
> "if (common->usage_count) return 0;" to ensure that subsequent code
> will be executed only once even if the 2 interfaces are ndo_open'd.
> IOW, skbs will be allocated for only 1 RX channel. I checked that behavior,
> and that's the way it works.
> This is because the host port (CPPI) has only 1 RX queue. This single
> queue is used to get all the packets, from both Ports and 2 (port ID is
> stored in each descriptor).
So you have one entity which needs buffers. CPPI. It seems like Port1
and Port2 do not need buffers? So to me, you need one page pool.
> So, because of this constraint, I ended up working on the "single
> page pool" option.
>
> Questions:
> 1) Is the behavior described above usual for ethernet switch devices ?
This might be the first time page pool has been used by a switch? I
would check the melanox and sparx5 driver and see if they use page
pool.
> 2) Given that I can't use a page pool for each HW queue, is it worth
> using the page pool memory model ?
> 3) Currently I use 2 xdp_rxq (one for port1 and another one for port2).
> If an XDP program is attached to port1, my page pool dma parameter
> will need to be DMA_BIDIRECTIONAL (because of XDP_TX). As a result,
> even if there is no XDP program attached to port2, the setting for page
> pool will need to be DMA_BIDIRECTIONAL instead of DMA_FROM_DEVICE.
> In such situation, isn't it a problem for port2 ?
> 4) Because of 1) and 2), will page pool performance really be better for
> this driver ?
You probably need to explain the TX architecture a bit. How are
packets passed to the hardware? Is it again via a single CPPI entity?
Or are there two entities?
DMA_BIDIRECTIONAL and DMA_FROM_DEVICE is about cache flushing and
invalidation. Before you pass a buffer to the hardware for it to
receive into, you need to invalidate the cache. That means when the
hardware gives the buffer back with a packet in it, there is a cache
miss and it fetches the new data from memory. If that packet gets
turned into an XDP_TX, you need to flush the cache to force any
changes out of the cache and into memory. The DMA from memory then
gets the up to date packet contents.
My guess would be, always using DMA_BIDIRECTIONAL is fine, so long as
your cache operations are correct. Turn on DMA_API_DEBUG and make sure
it is happy.
Andrew
Re: [PATCH 1/2] drm_edid: Add a function to get EDID base block
On Wed, 28 Feb 2024, Doug Anderson wrote: > Hi, > > On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang wrote: >> >> On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula >> wrote: >> > >> > On Fri, 23 Feb 2024, Hsin-Yi Wang wrote: >> > > It's found that some panels have variants that they share the same panel >> > > id >> > > although their EDID and names are different. Besides panel id, now we >> > > need >> > > the hash of entire EDID base block to distinguish these panel variants. >> > > >> > > Add drm_edid_get_base_block to returns the EDID base block, so caller can >> > > further use it to get panel id and/or the hash. >> > >> > Please reconsider the whole approach here. >> > >> > Please let's not add single-use special case functions to read an EDID >> > base block. >> > >> > Please consider reading the whole EDID, using the regular EDID reading >> > functions, and use that instead. >> > >> > Most likely you'll only have 1-2 blocks anyway. And you might consider >> > caching the EDID in struct panel_edp if reading the entire EDID is too >> > slow. (And if it is, this is probably sensible even if the EDID only >> > consists of one block.) >> > >> > Anyway, please do *not* merge this as-is. >> > >> >> hi Jani, >> >> I sent a v2 here implementing this method: >> https://lore.kernel.org/lkml/20240228011133.1238439-2-hsi...@chromium.org/ >> >> We still have to read edid twice due to: >> 1. The first caller is in panel probe, at that time, connector is >> still unknown, so we can't update connector status (eg. update >> edid_corrupt). >> 2. It's possible that the connector can have some override >> (drm_edid_override_get) to EDID, that is still unknown during the >> first read. > > I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the > fact that we can't cache the EDID (because we don't yet have a > "drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that > allows reading just the first block. If we try to boot a device with a > multi-block EDID we're now wastefully reading all the blocks of the > EDID twice at bootup which will slow boot time. > > If you can see a good solution to avoid reading the EDID twice then > that would be amazing, but if not it seems like we should go back to > what's here in v1. What do you think? Anyone else have any opinions? I haven't replied so far, because I've been going back and forth with this. I'm afraid I don't really like either approach now. Handling the no connector case in v2 is a bit ugly too. :( Seems like you only need this to extend the panel ID with a hash. And panel-edp.c is the only user of drm_edid_get_panel_id(). And EDID quirks in drm_edid.c could theoretically hit the same problem you're solving. So maybe something like: u32 drm_edid_get_panel_id(struct i2c_adapter *adapter, u32 *hash); or if you want to be fancy add a struct capturing both id and hash: bool drm_edid_get_panel_id(struct i2c_adapter *adapter, struct drm_edid_panel_id *id); And put the hash (or whatever mechanism you have) computation in drm_edid.c. Just hide it all in drm_edid.c, and keep the EDID interfaces neat. How would that work for you? BR, Jani. -- Jani Nikula, Intel
RE: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging
> -Original Message- > From: Nikula, Jani > Sent: Thursday, February 29, 2024 4:08 PM > To: Golani, Mitulkumar Ajitkumar ; > intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Nautiyal, Ankit K > ; Golani, Mitulkumar Ajitkumar > > Subject: Re: [PATCH v12 2/8] drm: Add Adaptive Sync SDP logging > > On Wed, 28 Feb 2024, Mitul Golani > wrote: > > Add structure representing Adaptive Sync Secondary Data Packet (AS SDP). > > Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate > > debugging. > > To be honest, the division of patches is a bit weird. There's no reason to > change > i915 here, is there? > Hi Jani, Thought behind this was to use the function where it was defined. But no worries, I have split it with new patch series floated. Regards, Mitul
