Re: [net-next v1 09/16] page_pool: device memory support
On Sun, Dec 10, 2023 at 8:04 PM Mina Almasry wrote:
>
> On Sun, Dec 10, 2023 at 6:26 PM Mina Almasry wrote:
> >
> > On Sun, Dec 10, 2023 at 6:04 PM Yunsheng Lin wrote:
> > >
> > > On 2023/12/9 0:05, Mina Almasry wrote:
> > > > On Fri, Dec 8, 2023 at 1:30 AM Yunsheng Lin
> > > > wrote:
> > > >>
> > > >>
> > > >> As mentioned before, it seems we need to have the above checking every
> > > >> time we need to do some per-page handling in page_pool core, is there
> > > >> a plan in your mind how to remove those kind of checking in the future?
> > > >>
> > > >
> > > > I see 2 ways to remove the checking, both infeasible:
> > > >
> > > > 1. Allocate a wrapper struct that pulls out all the fields the page
> > > > pool needs:
> > > >
> > > > struct netmem {
> > > > /* common fields */
> > > > refcount_t refcount;
> > > > bool is_pfmemalloc;
> > > > int nid;
> > > > ...
> > > > union {
> > > > struct dmabuf_genpool_chunk_owner *owner;
> > > > struct page * page;
> > > > };
> > > > };
> > > >
> > > > The page pool can then not care if the underlying memory is iov or
> > > > page. However this introduces significant memory bloat as this struct
> > > > needs to be allocated for each page or ppiov, which I imagine is not
> > > > acceptable for the upside of removing a few static_branch'd if
> > > > statements with no performance cost.
> > > >
> > > > 2. Create a unified struct for page and dmabuf memory, which the mm
> > > > folks have repeatedly nacked, and I imagine will repeatedly nack in
> > > > the future.
> > > >
> > > > So I imagine the special handling of ppiov in some form is critical
> > > > and the checking may not be removable.
> > >
> > > If the above is true, perhaps devmem is not really supposed to be
> > > intergated
> > > into page_pool.
> > >
> > > Adding a checking for every per-page handling in page_pool core is just
> > > too
> > > hacky to be really considerred a longterm solution.
> > >
> >
> > The only other option is to implement another page_pool for ppiov and
> > have the driver create page_pool or ppiov_pool depending on the state
> > of the netdev_rx_queue (or some helper in the net stack to do that for
> > the driver). This introduces some code duplication. The ppiov_pool &
> > page_pool would look similar in implementation.
> >
> > But this was all discussed in detail in RFC v2 and the last response I
> > heard from Jesper was in favor if this approach, if I understand
> > correctly:
> >
> > https://lore.kernel.org/netdev/7aedc5d5-0daf-63be-21bc-3b724cc1c...@redhat.com/
> >
> > Would love to have the maintainer weigh in here.
> >
>
> I should note we may be able to remove some of the checking, but maybe not
> all.
>
> - Checks that disable page fragging for ppiov can be removed once
> ppiov has frag support (in this series or follow up).
>
> - If we use page->pp_frag_count (or page->pp_ref_count) for
> refcounting ppiov, we can remove the if checking in the refcounting.
>
> - We may be able to store the dma_addr of the ppiov in page->dma_addr,
> but I'm unsure if that actually works, because the dma_buf dmaddr is
> dma_addr_t (u32 or u64), but page->dma_addr is unsigned long (4 bytes
> I think). But if it works for pages I may be able to make it work for
> ppiov as well.
>
> - Checks that obtain the page->pp can work with ppiov if we align the
> offset of page->pp and ppiov->pp.
>
> - Checks around page->pp_magic can be removed if we also have offset
> aligned ppiov->pp_magic.
>
> Sadly I don't see us removing the checking for these other cases:
>
> - page_is_pfmemalloc(): I'm not allowed to pass a non-struct page into
> that helper.
>
> - page_to_nid(): I'm not allowed to pass a non-struct page into that helper.
>
> - page_pool_free_va(): ppiov have no va.
>
> - page_pool_sync_for_dev/page_pool_dma_map: ppiov backed by dma-buf
> fundamentally can't get mapped again.
>
> Are the removal (or future removal) of these checks enough to resolve this?
>
I took a deeper look here, and with some effort I'm able to remove
almost all the custom checks for ppiov. The only remaining checks for
devmem are the checks around these mm calls:
page_is_pfmemalloc()
page_to_nid()
page_ref_count()
compound_head()
page_is_pfmemalloc() checks can be removed by using a bit
page->pp_magic potentially to indicate pfmemalloc().
The other 3, I'm not sure I can remove. They rely on the page flags or
other fields not specific to page_pool pages. The next version should
come with the most minimal amount of devmem checks for the page_pool.
> > > It is somewhat ironical that devmem is using static_branch to alliviate
> > > the
> > > performance impact for normal memory at the possible cost of performance
> > > degradation for devmem, does it not defeat some purpose of intergating
> > > devmem
> > > to page_pool?
> > >
> >
> > I don't see the issue. The static branch sets the non-ppiov path as
> > default if no memory
Re: [PATCH 0/5] clk: Make clk_rate_exclusive_get() return void
Hello Maxime, On Wed, Dec 13, 2023 at 08:16:04AM +0100, Maxime Ripard wrote: > On Tue, Dec 12, 2023 at 06:26:37PM +0100, Uwe Kleine-König wrote: > > clk_rate_exclusive_get() returns zero unconditionally. Most users "know" > > that and don't check the return value. This series fixes the four users > > that do error checking on the returned value and then makes function > > return void. > > > > Given that the changes to the drivers are simple and so merge conflicts > > (if any) should be easy to handle, I suggest to merge this complete > > series via the clk tree. > > I don't think it's the right way to go about it. > > clk_rate_exclusive_get() should be expected to fail. For example if > there's another user getting an exclusive rate on the same clock. > > If we're not checking for it right now, then it should probably be > fixed, but the callers checking for the error are right to do so if they > rely on an exclusive rate. It's the ones that don't that should be > modified. If some other consumer has already "locked" a clock that I call clk_rate_exclusive_get() for, this isn't an error. In my bubble I call this function because I don't want the rate to change e.g. because I setup some registers in the consuming device to provide a fixed UART baud rate or i2c bus frequency (and that works as expected). In this case I won't be able to change the rate of the clock, but that is signalled by clk_set_rate() failing (iff and when I need awother rate) which also seems the right place to fail to me. It's like that since clk_rate_exclusive_get() was introduced in 2017 (commit 55e9b8b7b806ec3f9a8817e13596682a5981c19c). BTW, I just noticed that my assertion "Most users \"know\" that [clk_rate_exclusive_get() returns zero unconditionally]" is wrong. As of next-20231213 there are 3 callers ignoring the return value of clk_rate_exclusive_get() and 4 that handle (imaginary) returned errors. I expected this function to be used more extensively. (In fact I think it should be used more as several drivers rely on the clk rate not changing.) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH 0/5] clk: Make clk_rate_exclusive_get() return void
Hi, On Tue, Dec 12, 2023 at 06:26:37PM +0100, Uwe Kleine-König wrote: > Hello, > > clk_rate_exclusive_get() returns zero unconditionally. Most users "know" > that and don't check the return value. This series fixes the four users > that do error checking on the returned value and then makes function > return void. > > Given that the changes to the drivers are simple and so merge conflicts > (if any) should be easy to handle, I suggest to merge this complete > series via the clk tree. I don't think it's the right way to go about it. clk_rate_exclusive_get() should be expected to fail. For example if there's another user getting an exclusive rate on the same clock. If we're not checking for it right now, then it should probably be fixed, but the callers checking for the error are right to do so if they rely on an exclusive rate. It's the ones that don't that should be modified. Maxime signature.asc Description: PGP signature
Re: [PATCH v2] dt-bindings: panel-simple-dsi: move LG 5" HD TFT LCD panel into DSI yaml
On 12/12/2023 21:09, David Heidelberg wrote:
> Originally was in the panel-simple, but belongs to panel-simple-dsi.
>
> See arch/arm/boot/dts/nvidia/tegra114-roth.dts for more details.
>
> Resolves the following warning:
> ```
> arch/arm/boot/dts/tegra114-roth.dt.yaml: panel@0: 'reg' does not match any of
> the regexes: 'pinctrl-[0-9]+'
> From schema:
> Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> ```
>
> Fixes: 310abcea76e9 ("dt-bindings: display: convert simple lg panels to DT
> Schema")
> Signed-off-by: David Heidelberg
> ---
> v2: added Fixes tag (thx to Jessica)
>
> .../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
> .../devicetree/bindings/display/panel/panel-simple.yaml | 2 --
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
[PATCH] drm/uapi: drm_mode.h: fix spellos and grammar
Correct spellos reported by codespell.
Fix some grammar (as 's' to a few words).
Signed-off-by: Randy Dunlap
Cc: David Airlie
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
---
include/uapi/drm/drm_mode.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff -- a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -36,10 +36,10 @@ extern "C" {
/**
* DOC: overview
*
- * DRM exposes many UAPI and structure definition to have a consistent
- * and standardized interface with user.
+ * DRM exposes many UAPI and structure definitions to have a consistent
+ * and standardized interface with users.
* Userspace can refer to these structure definitions and UAPI formats
- * to communicate to driver
+ * to communicate to drivers.
*/
#define DRM_CONNECTOR_NAME_LEN 32
@@ -540,7 +540,7 @@ struct drm_mode_get_connector {
/* the PROP_ATOMIC flag is used to hide properties from userspace that
* is not aware of atomic properties. This is mostly to work around
* older userspace (DDX drivers) that read/write each prop they find,
- * witout being aware that this could be triggering a lengthy modeset.
+ * without being aware that this could be triggering a lengthy modeset.
*/
#define DRM_MODE_PROP_ATOMIC0x8000
@@ -664,7 +664,7 @@ struct drm_mode_fb_cmd {
};
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */
-#define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */
+#define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifier[] */
/**
* struct drm_mode_fb_cmd2 - Frame-buffer metadata.
@@ -881,8 +881,8 @@ struct hdr_metadata_infoframe {
* These are coded as unsigned 16-bit values in units of
* 0.2, where 0x represents zero and 0xC350
* represents 1..
-* @display_primaries.x: X cordinate of color primary.
-* @display_primaries.y: Y cordinate of color primary.
+* @display_primaries.x: X coordinate of color primary.
+* @display_primaries.y: Y coordinate of color primary.
*/
struct {
__u16 x, y;
@@ -892,8 +892,8 @@ struct hdr_metadata_infoframe {
* These are coded as unsigned 16-bit values in units of
* 0.2, where 0x represents zero and 0xC350
* represents 1..
-* @white_point.x: X cordinate of whitepoint of color primary.
-* @white_point.y: Y cordinate of whitepoint of color primary.
+* @white_point.x: X coordinate of whitepoint of color primary.
+* @white_point.y: Y coordinate of whitepoint of color primary.
*/
struct {
__u16 x, y;
[PATCH] drm/i915/uapi: fix typos/spellos and punctuation
Use "its" for possessive form instead of "it's".
Hyphenate multi-word adjectives.
Correct some spelling.
End one line of code with ';' instead of ','. The before and after
object files are identical.
Signed-off-by: Randy Dunlap
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-...@lists.freedesktop.org
---
include/uapi/drm/i915_drm.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff -- a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -693,7 +693,7 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_HAS_EXEC_FENCE 44
/* Query whether DRM_I915_GEM_EXECBUFFER2 supports the ability to capture
- * user specified bufffers for post-mortem debugging of GPU hangs. See
+ * user-specified buffers for post-mortem debugging of GPU hangs. See
* EXEC_OBJECT_CAPTURE.
*/
#define I915_PARAM_HAS_EXEC_CAPTURE 45
@@ -1606,7 +1606,7 @@ struct drm_i915_gem_busy {
* is accurate.
*
* The returned dword is split into two fields to indicate both
-* the engine classess on which the object is being read, and the
+* the engine classes on which the object is being read, and the
* engine class on which it is currently being written (if any).
*
* The low word (bits 0:15) indicate if the object is being written
@@ -1815,7 +1815,7 @@ struct drm_i915_gem_madvise {
__u32 handle;
/* Advice: either the buffer will be needed again in the near future,
-* or wont be and could be discarded under memory pressure.
+* or won't be and could be discarded under memory pressure.
*/
__u32 madv;
@@ -3246,7 +3246,7 @@ struct drm_i915_query_topology_info {
* // enough to hold our array of engines. The kernel will fill out the
* // item.length for us, which is the number of bytes we need.
* //
- * // Alternatively a large buffer can be allocated straight away enabling
+ * // Alternatively a large buffer can be allocated straightaway enabling
* // querying in one pass, in which case item.length should contain the
* // length of the provided buffer.
* err = ioctl(fd, DRM_IOCTL_I915_QUERY, );
@@ -3256,7 +3256,7 @@ struct drm_i915_query_topology_info {
* // Now that we allocated the required number of bytes, we call the ioctl
* // again, this time with the data_ptr pointing to our newly allocated
* // blob, which the kernel can then populate with info on all engines.
- * item.data_ptr = (uintptr_t),
+ * item.data_ptr = (uintptr_t)
*
* err = ioctl(fd, DRM_IOCTL_I915_QUERY, );
* if (err) ...
@@ -3286,7 +3286,7 @@ struct drm_i915_query_topology_info {
/**
* struct drm_i915_engine_info
*
- * Describes one engine and it's capabilities as known to the driver.
+ * Describes one engine and its capabilities as known to the driver.
*/
struct drm_i915_engine_info {
/** @engine: Engine class and instance. */
[PATCH] drm/fourcc: fix spelling/typos
Correct spelling mistakes that were identified by codespell.
Signed-off-by: Randy Dunlap
Cc: David Airlie
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
---
include/uapi/drm/drm_fourcc.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff -- a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -54,7 +54,7 @@ extern "C" {
* Format modifiers may change any property of the buffer, including the number
* of planes and/or the required allocation size. Format modifiers are
* vendor-namespaced, and as such the relationship between a fourcc code and a
- * modifier is specific to the modifer being used. For example, some modifiers
+ * modifier is specific to the modifier being used. For example, some modifiers
* may preserve meaning - such as number of planes - from the fourcc code,
* whereas others may not.
*
@@ -79,7 +79,7 @@ extern "C" {
* format.
* - Higher-level programs interfacing with KMS/GBM/EGL/Vulkan/etc: these users
* see modifiers as opaque tokens they can check for equality and intersect.
- * These users musn't need to know to reason about the modifier value
+ * These users mustn't need to know to reason about the modifier value
* (i.e. they are not expected to extract information out of the modifier).
*
* Vendors should document their modifier usage in as much detail as
@@ -540,7 +540,7 @@ extern "C" {
* This is a tiled layout using 4Kb tiles in row-major layout.
* Within the tile pixels are laid out in 16 256 byte units / sub-tiles which
* are arranged in four groups (two wide, two high) with column-major layout.
- * Each group therefore consits out of four 256 byte units, which are also laid
+ * Each group therefore consists out of four 256 byte units, which are also
laid
* out as 2x2 column-major.
* 256 byte units are made out of four 64 byte blocks of pixels, producing
* either a square block or a 2:1 unit.
@@ -1103,7 +1103,7 @@ drm_fourcc_canonicalize_nvidia_format_mo
*/
/*
- * The top 4 bits (out of the 56 bits alloted for specifying vendor specific
+ * The top 4 bits (out of the 56 bits allotted for specifying vendor specific
* modifiers) denote the category for modifiers. Currently we have three
* categories of modifiers ie AFBC, MISC and AFRC. We can have a maximum of
* sixteen different categories.
@@ -1419,7 +1419,7 @@ drm_fourcc_canonicalize_nvidia_format_mo
* Amlogic FBC Memory Saving mode
*
* Indicates the storage is packed when pixel size is multiple of word
- * boudaries, i.e. 8bit should be stored in this mode to save allocation
+ * boundaries, i.e. 8bit should be stored in this mode to save allocation
* memory.
*
* This mode reduces body layout to 3072 bytes per 64x32 superblock with
[PATCH] iosys-map: fix typos
Correct spellos/typos in comments. Signed-off-by: Randy Dunlap Cc: Thomas Zimmermann Cc: dri-devel@lists.freedesktop.org --- include/linux/iosys-map.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff -- a/include/linux/iosys-map.h b/include/linux/iosys-map.h --- a/include/linux/iosys-map.h +++ b/include/linux/iosys-map.h @@ -34,7 +34,7 @@ * the same driver for allocation, read and write operations. * * Open-coding access to :c:type:`struct iosys_map ` is considered - * bad style. Rather then accessing its fields directly, use one of the provided + * bad style. Rather than accessing its fields directly, use one of the provided * helper functions, or implement your own. For example, instances of * :c:type:`struct iosys_map ` can be initialized statically with * IOSYS_MAP_INIT_VADDR(), or at runtime with iosys_map_set_vaddr(). These @@ -85,7 +85,7 @@ * if (iosys_map_is_equal(_map, _map)) * // always false * - * A set up instance of struct iosys_map can be used to access or manipulate the + * A setup instance of struct iosys_map can be used to access or manipulate the * buffer memory. Depending on the location of the memory, the provided helpers * will pick the correct operations. Data can be copied into the memory with * iosys_map_memcpy_to(). The address can be manipulated with iosys_map_incr().
[PATCH] drm/drm_modeset_helper_vtables.h: fix typos/spellos
Fix spelling problems as identified by codespell.
Signed-off-by: Randy Dunlap
Cc: David Airlie
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
---
include/drm/drm_modeset_helper_vtables.h |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff -- a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -134,7 +134,7 @@ struct drm_crtc_helper_funcs {
* Since this function is both called from the check phase of an atomic
* commit, and the mode validation in the probe paths it is not allowed
* to look at anything else but the passed-in mode, and validate it
-* against configuration-invariant hardward constraints. Any further
+* against configuration-invariant hardware constraints. Any further
* limits which depend upon the configuration can only be checked in
* @mode_fixup or @atomic_check.
*
@@ -550,7 +550,7 @@ struct drm_encoder_helper_funcs {
* Since this function is both called from the check phase of an atomic
* commit, and the mode validation in the probe paths it is not allowed
* to look at anything else but the passed-in mode, and validate it
-* against configuration-invariant hardward constraints. Any further
+* against configuration-invariant hardware constraints. Any further
* limits which depend upon the configuration can only be checked in
* @mode_fixup or @atomic_check.
*
@@ -1474,7 +1474,7 @@ struct drm_mode_config_helper_funcs {
* swapped into the various state pointers. The passed in state
* therefore contains copies of the old/previous state. This hook should
* commit the new state into hardware. Note that the helpers have
-* already waited for preceeding atomic commits and fences, but drivers
+* already waited for preceding atomic commits and fences, but drivers
* can add more waiting calls at the start of their implementation, e.g.
* to wait for driver-internal request for implicit syncing, before
* starting to commit the update to the hardware.
Re: [Freedreno] [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix
On 11/30/2023 11:39 PM, Dmitry Baryshkov wrote: On Fri, 1 Dec 2023 at 03:31, Jessica Zhang wrote: This series drops the frame_count and enable parameters (as they're always set to the same value). It also sets input_sel=0x1 for INTF. Signed-off-by: Jessica Zhang --- Jessica Zhang (2): drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() drm/msm/dpu: Set input_sel bit for INTF Please change the order of the commits: fix (input_sel) comes first, then comes the improvement. Otherwise if one needs to backport this fix, they either need to pick up the irrelevant patch, or they have to rework the fix. Acked. Thanks, Jessica Zhang drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 18 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 11 +-- 8 files changed, 26 insertions(+), 30 deletions(-) --- base-commit: 4047f50eb64d980fcd581a19bbe6164dab25ebc7 change-id: 20231122-encoder-fixup-61c190b16085 Best regards, -- Jessica Zhang -- With best wishes Dmitry
Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider
On 12/12/23 6:09 PM, Mina Almasry wrote:
> OK, I imagine this is not that hard to implement - it's really whether
> the change is acceptable to reviewers.
>
> I figure I can start by implementing a no-op abstraction to page*:
>
> typedef struct page netmem_t
>
> and replace the page* in the following places with netmem_t*:
>
> 1. page_pool API (not internals)
> 2. drivers using the page_pool.
> 3. skb_frag_t.
>
accessors to skb_frag_t field are now consolidated to
include/linux/skbuff.h (the one IB driver was fixed in Sept by
4ececeb83986), so changing skb_frag_t from bio_vec to something like:
typedef struct skb_frag {
void *addr;
unsigned int length;
unsigned int offset;
};
is trivial. From there, addr can default to `struct page *`. If LSB is
set, strip it and return `struct page_pool_iov *` or `struct buffer_pool *`
Re: [v3 5/6] drm/vs: Add hdmi driver
On 2023/12/11 20:13, Andy Yan wrote:
> Hi Keith：
>
> 在 2023-12-11 18:24:35，"Keith Zhao" 写道：
>>hi Maxime:
>>hi Andy:
>>
>>On 2023/12/8 17:14, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Fri, Dec 08, 2023 at 11:23:37AM +0800, Andy Yan wrote:
在 2023-12-08 11:00:31，"Keith Zhao" 写道：
>
>
>On 2023/12/8 8:37, Andy Yan wrote:
>> Hi Keth：
>>
>>
>>
>>
>>
>>
>> 在 2023-12-07 18:48:13，"Keith Zhao" 写道：
>>>
>>>
>>>On 2023/12/7 17:02, Andy Yan wrote:
Hi Keith：
At 2023-12-06 22:11:33, "Keith Zhao"
wrote:
>
>
>On 2023/12/6 20:56, Maxime Ripard wrote:
>> On Wed, Dec 06, 2023 at 08:02:55PM +0800, Keith Zhao wrote:
>>> >> +static const struct of_device_id starfive_hdmi_dt_ids[] = {
>>> >> + { .compatible = "starfive,jh7110-inno-hdmi",},
>>> >
>>> > So it's inno hdmi, just like Rockchip then?
>>> >
>>> > This should be a common driver.
>>>
>>> Rockchip has a inno hdmi IP. and Starfive has a inno hdmi IP.
>>> but the harewawre difference of them is big , it is not easy to
>>> use the common driver
>>> maybe i need the inno hdmi version here to make a distinction
>>
>> I just had a look at the rockchip header file: all the registers
>> but the
>> STARFIVE_* ones are identical.
>>
>> There's no need to have two identical drivers then, please use the
>> rockchip driver instead.
>>
>> Maxime
>
>ok, have a simple test , edid can get . i will continue
Maybe you can take drivers/gpu/drm/bridge/synopsys/dw-hdmi as a
reference， this
is also a hdmi ip used by rockchip/meson/sunxi/jz/imx。
We finally make it share one driver。
>
>>>hi Andy:
>>>
>>>dw_hdmi seems a good choice , it can handle inno hdmi hardware by
>>>define its dw_hdmi_plat_data.
>>>does it means i can write own driver files such as(dw_hdmi-starfive.c)
>>>based on dw_hdmi instead of add plat_data in inno_hdmi.c
>>>
>>
>> I think the process maybe like this：
>>
>> 1. split the inno_hdmi.c under rockchip to inno_hdmi.c(the common
>> part), inno_hdmi-rockchip.c(the soc specific part)
>> 2. move the common part inno_hdmi.c to
>> drivers/gpu/drm/bridge/innosilicon/
>> 3. add startfive specific part, inno_hdmi-startfive.c
>>
>> bellow git log from kernel three show how we convert dw_hdmi to a
>> common driver:
>>
>>
>>
>> 12b9f204e804 drm: bridge/dw_hdmi: add rockchip rk3288 support
>> 74af9e4d03b8 dt-bindings: Add documentation for rockchip dw hdmi
>> d346c14eeea9 drm: bridge/dw_hdmi: add function dw_hdmi_phy_enable_spare
>> a4d3b8b050d5 drm: bridge/dw_hdmi: clear i2cmphy_stat0 reg in
>> hdmi_phy_wait_i2c_done
>> 632d035bace2 drm: bridge/dw_hdmi: add mode_valid support
>> 0cd9d1428322 drm: bridge/dw_hdmi: add support for multi-byte register
>> width access
>> cd152393967e dt-bindings: add document for dw_hdmi
>> b21f4b658df8 drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
>> aaa757a092c2 drm: imx: imx-hdmi: split phy configuration to platform
>> driver
>> 3d1b35a3d9f3 drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
>> c2c3848851a7 drm: imx: imx-hdmi: return defer if can't get ddc i2c
>> adapter
>> b587833933de drm: imx: imx-hdmi: make checkpatch happy
>>
>hi Andy:
>I got you means,
>as I don't have a rockchip board on hand , to split the inno_hdmi.c can
>not be tested.
>
>how adout this idea:
>1、split the starfive_hdmi.c under verisilicion to inno_hdmi.c(the common
>part), inno_hdmi-starfive.c(the soc specific part)
>2. move the common part inno_hdmi.c to drivers/gpu/drm/bridge/innosilicon/
>3. In the future, inno hdmi.c under rockchip will reuse the public driver.
I am not sure if drm maintainers are happy with this。
>>>
>>> Not really, no.
>>>
>>> Because we would still have two drivers for the same controller, and a
>>> common one that haven't really been tested on anything but a single
>>> platform. So arguably a worse situation than what you were suggesting in
>>> the first place.
>>>
>>> The best solution would be to find someone with a Rockchip board to test
>>> your changes, or to get one if it's doable so you can test yourself.
>>
>>ok I will also try to buy a Rockchip 3036 board for self-test.
>>According to the commit log idea provided by Andy before, make the inno_hdmi
>>driver common module.
>
> I
Re: [PATCH v2] dt-bindings: panel-simple-dsi: move LG 5" HD TFT LCD panel into DSI yaml
On 12/12/2023 12:09 PM, David Heidelberg wrote:
Originally was in the panel-simple, but belongs to panel-simple-dsi.
See arch/arm/boot/dts/nvidia/tegra114-roth.dts for more details.
Resolves the following warning:
```
arch/arm/boot/dts/tegra114-roth.dt.yaml: panel@0: 'reg' does not match any of
the regexes: 'pinctrl-[0-9]+'
From schema:
Documentation/devicetree/bindings/display/panel/panel-simple.yaml
```
Fixes: 310abcea76e9 ("dt-bindings: display: convert simple lg panels to DT
Schema")
Signed-off-by: David Heidelberg
Acked-by: Jessica Zhang
---
v2: added Fixes tag (thx to Jessica)
.../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
.../devicetree/bindings/display/panel/panel-simple.yaml | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 73674baea75d..f9160d7bac3c 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -42,6 +42,8 @@ properties:
- lg,acx467akm-7
# LG Corporation 7" WXGA TFT LCD panel
- lg,ld070wx3-sl01
+# LG Corporation 5" HD TFT LCD panel
+ - lg,lh500wx1-sd03
# One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
- osddisplays,osd101t2587-53ts
# Panasonic 10" WUXGA TFT LCD panel
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 2021aa82871a..634a10c6f2dd 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -212,8 +212,6 @@ properties:
- lemaker,bl035-rgb-002
# LG 7" (800x480 pixels) TFT LCD panel
- lg,lb070wv8
-# LG Corporation 5" HD TFT LCD panel
- - lg,lh500wx1-sd03
# LG LP079QX1-SP0V 7.9" (1536x2048 pixels) TFT LCD panel
- lg,lp079qx1-sp0v
# LG 9.7" (2048x1536 pixels) TFT LCD panel
--
2.43.0
Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider
On Tue, Dec 12, 2023 at 7:08 AM Jason Gunthorpe wrote: > > On Tue, Dec 12, 2023 at 06:58:17AM -0800, Mina Almasry wrote: > > > Jason, we set the LSB on page_pool_iov pointers before casting it to > > struct page pointers. The resulting pointers are not useable as page > > pointers at all. > > I understand that, the second ask is about maintainability of the mm > by using correct types. > > > > Perhaps you can simply avoid this by arranging for this driver to also > > > exclusively use some special type to indicate the dual nature of the > > > pointer and leave the other drivers as using the struct page version. > > > > This is certainly possible, but it requires us to rename all the page > > pointers in the page_pool to the new type, and requires the driver > > adding devmem TCP support to rename all the page* pointer instances to > > the new type. It's possible but it introduces lots of code churn. Is > > the LSB + cast not a reasonable compromise here? I feel like the trick > > of setting the least significant bit on a pointer to indicate it's > > something else has a fair amount of precedent in the kernel. > > Linus himself has complained about exactly this before, and written a cleanup: > > https://lore.kernel.org/linux-mm/20221108194139.57604-1-torva...@linux-foundation.org/ > > If you mangle a pointer *so it is no longer a pointer* then give it a > proper opaque type so the compiler can check everything statically and > require that the necessary converters are called in all cases. > > You call it churn, I call it future maintainability. :( > > No objection to using the LSB, just properly type a LSB mangled > pointer so everyone knows what is going on and don't call it MM's > struct page *. > > I would say this is important here because it is a large driver facing > API surface. > OK, I imagine this is not that hard to implement - it's really whether the change is acceptable to reviewers. I figure I can start by implementing a no-op abstraction to page*: typedef struct page netmem_t and replace the page* in the following places with netmem_t*: 1. page_pool API (not internals) 2. drivers using the page_pool. 3. skb_frag_t. I think that change needs to be a separate series by itself. Then the devmem patches would on top of that change netmem_t such that it can be a union between struct page and page_pool_iov and add the special handling of page_pool_iov. Does this sound reasonable? -- Thanks, Mina
Re: [PATCH] drm/msm/dpu: move CSC tables to dpu_hw_util.c
On Wed, 13 Dec 2023 at 02:59, Abhinav Kumar wrote:
>
>
>
> On 12/12/2023 4:57 PM, Dmitry Baryshkov wrote:
> > Move CSC tables out of the header file to fix following kind of warnings:
> >
> > In file included from drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h:8,
> > from drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:5:
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:54:33: warning:
> > ‘dpu_csc10_rgb2yuv_601l’ defined but not used [-Wunused-const-variable=]
> > 54 | static const struct dpu_csc_cfg dpu_csc10_rgb2yuv_601l = {
> >| ^~
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:39:33: warning:
> > ‘dpu_csc10_YUV2RGB_601L’ defined but not used [-Wunused-const-variable=]
> > 39 | static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
> >| ^~
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:24:33: warning:
> > ‘dpu_csc_YUV2RGB_601L’ defined but not used [-Wunused-const-variable=]
> > 24 | static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
> >| ^~~~
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 44 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 44 ++---
> > 2 files changed, 47 insertions(+), 41 deletions(-)
> >
>
> Strange, I didnt hit this but change LGTM,
It shows with W=1, maybe that's why you didn't observe them.
>
> Reviewed-by: Abhinav Kumar
--
With best wishes
Dmitry
Re: [PATCH] drm/msm/dpu: move CSC tables to dpu_hw_util.c
On 12/12/2023 4:57 PM, Dmitry Baryshkov wrote:
Move CSC tables out of the header file to fix following kind of warnings:
In file included from drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h:8,
from drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:5:
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:54:33: warning:
‘dpu_csc10_rgb2yuv_601l’ defined but not used [-Wunused-const-variable=]
54 | static const struct dpu_csc_cfg dpu_csc10_rgb2yuv_601l = {
| ^~
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:39:33: warning:
‘dpu_csc10_YUV2RGB_601L’ defined but not used [-Wunused-const-variable=]
39 | static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
| ^~
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:24:33: warning:
‘dpu_csc_YUV2RGB_601L’ defined but not used [-Wunused-const-variable=]
24 | static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
| ^~~~
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 44 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 44 ++---
2 files changed, 47 insertions(+), 41 deletions(-)
Strange, I didnt hit this but change LGTM,
Reviewed-by: Abhinav Kumar
linux-next: manual merge of the fbdev tree with the drm tree
Hi all,
Today's linux-next merge of the fbdev tree got a conflict in:
drivers/video/fbdev/vermilion/vermilion.c
between commit:
76f92201b821 ("fbdev: Push pgprot_decrypted() into mmap implementations")
from the drm tree and commit:
d8a47ee16884 ("fbdev: Remove support for Carillo Ranch driver")
from the fbdev tree.
I fixed it up (I just removed the file) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgpYclufR6yWf.pgp
Description: OpenPGP digital signature
[PATCH] drm/msm/dpu: move CSC tables to dpu_hw_util.c
Move CSC tables out of the header file to fix following kind of warnings:
In file included from drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h:8,
from drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:5:
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:54:33: warning:
‘dpu_csc10_rgb2yuv_601l’ defined but not used [-Wunused-const-variable=]
54 | static const struct dpu_csc_cfg dpu_csc10_rgb2yuv_601l = {
| ^~
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:39:33: warning:
‘dpu_csc10_YUV2RGB_601L’ defined but not used [-Wunused-const-variable=]
39 | static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
| ^~
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h:24:33: warning:
‘dpu_csc_YUV2RGB_601L’ defined but not used [-Wunused-const-variable=]
24 | static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
| ^~~~
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 44 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 44 ++---
2 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
index 0b05061e3e62..395fdcea28b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
@@ -561,3 +561,47 @@ bool dpu_hw_clk_force_ctrl(struct dpu_hw_blk_reg_map *c,
return clk_forced_on;
}
+
+#define TO_S15D16(_x_)((_x_) << 7)
+
+const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
+ {
+ /* S15.16 format */
+ 0x00012A00, 0x, 0x00019880,
+ 0x00012A00, 0x9B80, 0x3000,
+ 0x00012A00, 0x00020480, 0x,
+ },
+ /* signed bias */
+ { 0xfff0, 0xff80, 0xff80,},
+ { 0x0, 0x0, 0x0,},
+ /* unsigned clamp */
+ { 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,},
+ { 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,},
+};
+
+const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
+ {
+ /* S15.16 format */
+ 0x00012A00, 0x, 0x00019880,
+ 0x00012A00, 0x9B80, 0x3000,
+ 0x00012A00, 0x00020480, 0x,
+ },
+ /* signed bias */
+ { 0xffc0, 0xfe00, 0xfe00,},
+ { 0x0, 0x0, 0x0,},
+ /* unsigned clamp */
+ { 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,},
+ { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
+};
+
+const struct dpu_csc_cfg dpu_csc10_rgb2yuv_601l = {
+ {
+ TO_S15D16(0x0083), TO_S15D16(0x0102), TO_S15D16(0x0032),
+ TO_S15D16(0x1fb5), TO_S15D16(0x1f6c), TO_S15D16(0x00e1),
+ TO_S15D16(0x00e1), TO_S15D16(0x1f45), TO_S15D16(0x1fdc)
+ },
+ { 0x00, 0x00, 0x00 },
+ { 0x0040, 0x0200, 0x0200 },
+ { 0x000, 0x3ff, 0x000, 0x3ff, 0x000, 0x3ff },
+ { 0x040, 0x3ac, 0x040, 0x3c0, 0x040, 0x3c0 },
+};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
index c0aaad2023da..25cc13b57756 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -21,47 +21,9 @@
#define TO_S15D16(_x_)((_x_) << 7)
-static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
- {
- /* S15.16 format */
- 0x00012A00, 0x, 0x00019880,
- 0x00012A00, 0x9B80, 0x3000,
- 0x00012A00, 0x00020480, 0x,
- },
- /* signed bias */
- { 0xfff0, 0xff80, 0xff80,},
- { 0x0, 0x0, 0x0,},
- /* unsigned clamp */
- { 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,},
- { 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,},
-};
-
-static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
- {
- /* S15.16 format */
- 0x00012A00, 0x, 0x00019880,
- 0x00012A00, 0x9B80, 0x3000,
- 0x00012A00, 0x00020480, 0x,
- },
- /* signed bias */
- { 0xffc0, 0xfe00, 0xfe00,},
- { 0x0, 0x0, 0x0,},
- /* unsigned clamp */
- { 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,},
- { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
-};
-
-static const struct dpu_csc_cfg dpu_csc10_rgb2yuv_601l = {
- {
- TO_S15D16(0x0083), TO_S15D16(0x0102), TO_S15D16(0x0032),
- TO_S15D16(0x1fb5), TO_S15D16(0x1f6c), TO_S15D16(0x00e1),
- TO_S15D16(0x00e1), TO_S15D16(0x1f45), TO_S15D16(0x1fdc)
- },
- { 0x00, 0x00, 0x00 },
- { 0x0040, 0x0200, 0x0200 },
- { 0x000, 0x3ff, 0x000, 0x3ff, 0x000, 0x3ff },
- { 0x040, 0x3ac, 0x040, 0x3c0, 0x040, 0x3c0 },
-};
+extern const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L;
+extern const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L;
+extern const struct dpu_csc_cfg
linux-next: manual merge of the fbdev tree with the drm tree
Hi all,
Today's linux-next merge of the fbdev tree got a conflict in:
drivers/video/fbdev/amba-clcd.c
between commit:
76f92201b821 ("fbdev: Push pgprot_decrypted() into mmap implementations")
from the drm tree and commit:
13366c25125e ("fbdev: amba-clcd: Delete the old CLCD driver")
from the fbdev tree.
I fixed it up (I just deleted the file) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgpitTSzDuKSR.pgp
Description: OpenPGP digital signature
Re: [PATCH v4 0/1] Stabilize use of vblank_refcount
On Tue, 12 Dec 2023 15:10:57 -0800, Paloma Arellano wrote: > There is currently a race condition occuring when accessing > vblank_refcount. Therefore, vblank irq timeouts may occur. > > Avoid any vblank irq timeouts by stablizing the use of vblank_refcount. > > Changes from prior versions: >v4: - Removed vblank_ctl_lock from dpu_encoder_virt, so it is only a > parameter of dpu_encoder_phys. >- Switch from atomic refcnt to a simple int counter as mutex has > now been added >v3: - Mistakenly did not change wording of patch #2 in last version. > It is done now. >v2: - Slightly changed wording of patch #2 commit message > > [...] Applied, thanks! [1/1] drm/msm/dpu: Add mutex lock in control vblank irq https://gitlab.freedesktop.org/lumag/msm/-/commit/b9dc09b4e534 Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 0/3] drm/msm/dpu: enable writeback on several platforms
On Sun, 03 Dec 2023 03:27:40 +0300, Dmitry Baryshkov wrote: > This enables writeback on several platforms where I could actually test > it. > > Changes since v1: > - Fixed the CLK_CTRL for SDM845 platform. > > Dmitry Baryshkov (3): > drm/msm/dpu: enable writeback on SDM845 > drm/msm/dpu: enable writeback on SM8350 > drm/msm/dpu: enable writeback on SM8450 > > [...] Applied, thanks! [1/3] drm/msm/dpu: enable writeback on SDM845 https://gitlab.freedesktop.org/lumag/msm/-/commit/d907efe518e4 [2/3] drm/msm/dpu: enable writeback on SM8350 https://gitlab.freedesktop.org/lumag/msm/-/commit/1a8dfd51d6e4 [3/3] drm/msm/dpu: enable writeback on SM8450 https://gitlab.freedesktop.org/lumag/msm/-/commit/e512b4a8ec37 Best regards, -- Dmitry Baryshkov
Re: [PATCH v4 00/15] Add CDM support for MSM writeback
On Tue, 12 Dec 2023 12:52:38 -0800, Abhinav Kumar wrote: > Chroma Down Sampling (CDM) block is a hardware block in the DPU pipeline > which among other things has a CSC block that can convert RGB input > from the DPU to YUV data. > > This block can be used with either HDMI, DP or writeback interface. > > In this series, lets first add the support for CDM block to be used > with writeback and then follow-up with support for other interfaces such > as DP. > > [...] Applied, thanks! [01/15] drm/msm/dpu: add formats check for writeback encoder https://gitlab.freedesktop.org/lumag/msm/-/commit/100a72bdb03f [02/15] drm/msm/dpu: rename dpu_encoder_phys_wb_setup_cdp to match its functionality https://gitlab.freedesktop.org/lumag/msm/-/commit/e1e59787d7c8 [03/15] drm/msm/dpu: fix writeback programming for YUV cases https://gitlab.freedesktop.org/lumag/msm/-/commit/545979247e5d [04/15] drm/msm/dpu: move csc matrices to dpu_hw_util https://gitlab.freedesktop.org/lumag/msm/-/commit/636d1d4dfb24 [05/15] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/f8bc2bc3a673 [06/15] drm/msm/dpu: add cdm blocks to sm8250 dpu_hw_catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/9b61ea803a9a [07/15] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block https://gitlab.freedesktop.org/lumag/msm/-/commit/faa729ee2e48 [08/15] drm/msm/dpu: add cdm blocks to RM https://gitlab.freedesktop.org/lumag/msm/-/commit/059bffe7a88d [09/15] drm/msm/dpu: add support to allocate CDM from RM https://gitlab.freedesktop.org/lumag/msm/-/commit/a324c046bb7d [10/15] drm/msm/dpu: add CDM related logic to dpu_hw_ctl layer https://gitlab.freedesktop.org/lumag/msm/-/commit/d7a329684b77 [11/15] drm/msm/dpu: add an API to setup the CDM block for writeback https://gitlab.freedesktop.org/lumag/msm/-/commit/055376f936e8 [12/15] drm/msm/dpu: plug-in the cdm related bits to writeback setup https://gitlab.freedesktop.org/lumag/msm/-/commit/1364a52a9a77 [13/15] drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output https://gitlab.freedesktop.org/lumag/msm/-/commit/8142ee18e902 [14/15] drm/msm/dpu: introduce separate wb2_format arrays for rgb and yuv https://gitlab.freedesktop.org/lumag/msm/-/commit/a074ca909811 [15/15] drm/msm/dpu: add cdm blocks to dpu snapshot https://gitlab.freedesktop.org/lumag/msm/-/commit/c86495429c4a Best regards, -- Dmitry Baryshkov
Re: [PATCH RESEND v2 0/3] drm: introduce per-encoder debugfs directory
On Sun, 03 Dec 2023 14:53:12 +0300, Dmitry Baryshkov wrote: > Resending, patch 1 needs review from DRM core maintainers, but it got no > attention since October. > > Each of connectors and CRTCs used by the DRM device provides debugfs > directory, which is used by several standard debugfs files and can > further be extended by the driver. Add such generic debugfs directories > for encoder. As a showcase for this dir, migrate `bridge_chains' debugfs > file (which contains per-encoder data) and MSM custom encoder status to > this new debugfs directory. > > [...] Applied, thanks! [3/3] drm/msm/dpu: move encoder status to standard encoder debugfs dir https://gitlab.freedesktop.org/lumag/msm/-/commit/62d35629da80 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
On Mon, 11 Dec 2023 10:19:55 -0800, Rob Clark wrote: > When we start getting these, we get a *lot*. So ratelimit it to not > flood dmesg. > > Applied, thanks! [1/1] drm/msm/dpu: Ratelimit framedone timeout msgs https://gitlab.freedesktop.org/lumag/msm/-/commit/e37cb117b819 Best regards, -- Dmitry Baryshkov
Re: [PATCH 0/9] arm64: dts: qcom: sm8150-hdk: enable display output
On Sun, 10 Dec 2023 02:21:23 +0300, Dmitry Baryshkov wrote: > Enable display output on the SM8150 HDK device. This includes HDMI > output through the onboard DSI-HDMI bridge and DP output on the USB-C > port. > > Dmitry Baryshkov (9): > dt-bindings: display: msm: dp: declare compatible string for sm8150 > arm64: dts: qcom: sm8150: use SoC-specific compat for RPMh stats > arm64: dts: qcom: sm8150: make dispcc cast minimal vote on MMCX > arm64: dts: qcom: sm8150-hdk: enable HDMI output > arm64: dts: qcom: sm8150-hdk: fix SS USB regulators > arm64: dts: qcom: sm8150: add DisplayPort controller > arm64: dts: qcom: sm8150: add USB-C ports to the USB+DP QMP PHY > arm64: dts: qcom: sm8150: add USB-C ports to the OTG USB host > arm64: dts: qcom: sm8150-hdk: enable DisplayPort and USB-C altmode > > [...] Applied, thanks! [1/9] dt-bindings: display: msm: dp: declare compatible string for sm8150 https://gitlab.freedesktop.org/lumag/msm/-/commit/0d196633771c Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/dpu: remove extra drm_encoder_cleanup from the error path
On Mon, 11 Dec 2023 17:54:40 +0300, Dmitry Baryshkov wrote: > The drmm handler will perform drm_encoder_cleanup() for us. Moreover if > we call drm_encoder_cleanup() manually, the drmm_encoder_alloc_release() > will spawn warnings at drivers/gpu/drm/drm_encoder.c:214. Drop these > extra drm_encoder_cleanup() calls. > > Applied, thanks! [1/1] drm/msm/dpu: remove extra drm_encoder_cleanup from the error path https://gitlab.freedesktop.org/lumag/msm/-/commit/014a07f72a33 Best regards, -- Dmitry Baryshkov
Re: [PATCH v4 1/1] drm/msm/dpu: Add mutex lock in control vblank irq
On Wed, 13 Dec 2023 at 01:11, Paloma Arellano wrote: > > Add a mutex lock to control vblank irq to synchronize vblank > enable/disable operations happening from different threads to prevent > race conditions while registering/unregistering the vblank irq callback. > > v4: -Removed vblank_ctl_lock from dpu_encoder_virt, so it is only a > parameter of dpu_encoder_phys. > -Switch from atomic refcnt to a simple int counter as mutex has > now been added > v3: Mistakenly did not change wording in last version. It is done now. > v2: Slightly changed wording of commit message > > Signed-off-by: Paloma Arellano > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 - > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++- > .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 -- > .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 33 --- > 4 files changed, 47 insertions(+), 23 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v4 1/1] drm/msm/dpu: Add mutex lock in control vblank irq
Add a mutex lock to control vblank irq to synchronize vblank
enable/disable operations happening from different threads to prevent
race conditions while registering/unregistering the vblank irq callback.
v4: -Removed vblank_ctl_lock from dpu_encoder_virt, so it is only a
parameter of dpu_encoder_phys.
-Switch from atomic refcnt to a simple int counter as mutex has
now been added
v3: Mistakenly did not change wording in last version. It is done now.
v2: Slightly changed wording of commit message
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 -
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++-
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 --
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 33 ---
4 files changed, 47 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1cf7ff6caff4e..73bd4c086a46f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2497,7 +2497,6 @@ void dpu_encoder_phys_init(struct dpu_encoder_phys
*phys_enc,
phys_enc->enc_spinlock = p->enc_spinlock;
phys_enc->enable_state = DPU_ENC_DISABLED;
- atomic_set(_enc->vblank_refcount, 0);
atomic_set(_enc->pending_kickoff_cnt, 0);
atomic_set(_enc->pending_ctlstart_cnt, 0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 6f04c3d56e77c..96bda57b69596 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -155,6 +155,7 @@ enum dpu_intr_idx {
* @hw_wb: Hardware interface to the wb registers
* @dpu_kms: Pointer to the dpu_kms top level
* @cached_mode: DRM mode cached at mode_set time, acted on in enable
+ * @vblank_ctl_lock: Vblank ctl mutex lock to protect vblank_refcount
* @enabled: Whether the encoder has enabled and running a mode
* @split_role:Role to play in a split-panel configuration
* @intf_mode: Interface mode
@@ -183,11 +184,12 @@ struct dpu_encoder_phys {
struct dpu_hw_wb *hw_wb;
struct dpu_kms *dpu_kms;
struct drm_display_mode cached_mode;
+ struct mutex vblank_ctl_lock;
enum dpu_enc_split_role split_role;
enum dpu_intf_mode intf_mode;
spinlock_t *enc_spinlock;
enum dpu_enc_enable_state enable_state;
- atomic_t vblank_refcount;
+ int vblank_refcount;
atomic_t vsync_cnt;
atomic_t underrun_cnt;
atomic_t pending_ctlstart_cnt;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index be185fe69793b..2d788c5e26a83 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -244,7 +244,8 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(
return -EINVAL;
}
- refcount = atomic_read(_enc->vblank_refcount);
+ mutex_lock(_enc->vblank_ctl_lock);
+ refcount = phys_enc->vblank_refcount;
/* Slave encoders don't report vblank */
if (!dpu_encoder_phys_cmd_is_master(phys_enc))
@@ -260,16 +261,24 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(
phys_enc->hw_pp->idx - PINGPONG_0,
enable ? "true" : "false", refcount);
- if (enable && atomic_inc_return(_enc->vblank_refcount) == 1)
- ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_RDPTR],
- dpu_encoder_phys_cmd_te_rd_ptr_irq,
- phys_enc);
- else if (!enable && atomic_dec_return(_enc->vblank_refcount) == 0)
- ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_RDPTR]);
+ if (enable) {
+ if (phys_enc->vblank_refcount == 0)
+ ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_RDPTR],
+ dpu_encoder_phys_cmd_te_rd_ptr_irq,
+ phys_enc);
+ if (!ret)
+ phys_enc->vblank_refcount++;
+ } else if (!enable) {
+ if (phys_enc->vblank_refcount == 1)
+ ret =
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_RDPTR]);
+ if (!ret)
+ phys_enc->vblank_refcount--;
+ }
end:
+ mutex_unlock(_enc->vblank_ctl_lock);
if (ret) {
DRM_ERROR("vblank irq err id:%u pp:%d ret:%d, enable
[PATCH v4 0/1] Stabilize use of vblank_refcount
There is currently a race condition occuring when accessing vblank_refcount. Therefore, vblank irq timeouts may occur. Avoid any vblank irq timeouts by stablizing the use of vblank_refcount. Changes from prior versions: v4: - Removed vblank_ctl_lock from dpu_encoder_virt, so it is only a parameter of dpu_encoder_phys. - Switch from atomic refcnt to a simple int counter as mutex has now been added v3: - Mistakenly did not change wording of patch #2 in last version. It is done now. v2: - Slightly changed wording of patch #2 commit message Paloma Arellano (1): drm/msm/dpu: Add mutex lock in control vblank irq drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 - .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 -- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 33 --- 4 files changed, 47 insertions(+), 23 deletions(-) -- 2.39.2
Re: nouveau 0000:01:00.0: drm_WARN_ON(!found_head)
On Sat, Nov 11, 2023 at 01:03:23PM +0100, Borislav Petkov wrote: > Hi, > > this is ontop of Linus' tree from the 4th (lemme know if I should try > the latest) on one of my test boxes: > > nouveau :01:00.0: vgaarb: deactivate vga console > Console: switching to colour dummy device 80x25 > nouveau :01:00.0: NVIDIA GT218 (0a8280b1) > CE: hpet increased min_delta_ns to 20115 nsec > nouveau :01:00.0: bios: version 70.18.49.00.00 > nouveau :01:00.0: fb: 1024 MiB DDR3 > nouveau :01:00.0: DRM: VRAM: 1024 MiB > nouveau :01:00.0: DRM: GART: 1048576 MiB > nouveau :01:00.0: DRM: TMDS table version 2.0 > nouveau :01:00.0: DRM: MM: using COPY for buffer copies > [ cut here ] > nouveau :01:00.0: drm_WARN_ON(!found_head) > WARNING: CPU: 4 PID: 786 at drivers/gpu/drm/nouveau/dispnv50/disp.c:2731 > nv50_display_init+0x28c/0x4f0 [nouveau] > Modules linked in: nouveau(+) drm_ttm_helper ttm video drm_exec drm_gpuvm > gpu_sched drm_display_helper wmi > CPU: 4 PID: 786 Comm: systemd-udevd Not tainted 6.6.0+ #1 This still fires on -rc5: [4.577348] nouveau :01:00.0: vgaarb: deactivate vga console [4.584482] Console: switching to colour dummy device 80x25 [4.590120] nouveau :01:00.0: NVIDIA GT218 (0a8280b1) [4.718171] nouveau :01:00.0: bios: version 70.18.49.00.00 [4.724788] nouveau :01:00.0: fb: 1024 MiB DDR3 [6.047984] nouveau :01:00.0: DRM: VRAM: 1024 MiB [6.053031] nouveau :01:00.0: DRM: GART: 1048576 MiB [6.058340] nouveau :01:00.0: DRM: TMDS table version 2.0 [6.065892] nouveau :01:00.0: DRM: MM: using COPY for buffer copies [6.078375] [ cut here ] [6.082994] nouveau :01:00.0: drm_WARN_ON(!found_head) [6.083023] WARNING: CPU: 3 PID: 779 at drivers/gpu/drm/nouveau/dispnv50/disp.c:2731 nv50_display_init+0x28c/0x4f0 [nouve au] [6.099800] Modules linked in: nouveau(+) drm_ttm_helper ttm video drm_exec drm_gpuvm gpu_sched drm_display_helper wmi [6.110490] CPU: 3 PID: 779 Comm: systemd-udevd Not tainted 6.7.0-rc5+ #2 [6.117272] Hardware name: MICRO-STAR INTERNATIONAL CO.,LTD MS-7599/870-C45 (MS-7599), BIOS V1.15 03/04/2011 [6.127087] RIP: 0010:nv50_display_init+0x28c/0x4f0 [nouveau] [6.132915] Code: 4c 8b 6f 50 4d 85 ed 75 03 4c 8b 2f e8 cd 16 37 e1 48 c7 c1 4c 55 2d a0 48 89 c6 4c 89 ea 48 c7 c7 42 5 5 2d a0 e8 44 5a e8 e0 <0f> 0b 48 8b 43 08 49 39 c6 48 8d 58 f8 0f 85 41 ff ff ff 48 8d 7c [6.151660] RSP: 0018:c936ba98 EFLAGS: 00010286 [6.156885] RAX: 002e RBX: 8881009fbc00 RCX: [6.164013] RDX: 0002 RSI: c936b9b0 RDI: 0001 [6.171141] RBP: 888103fc8ad0 R08: 888136ffdfe8 R09: 0058 [6.178263] R10: 027a R11: 888136401b70 R12: 888103fc8800 [6.185393] R13: 888100abddf0 R14: 888103fc8ab0 R15: [6.192521] FS: 7fdc144858c0() GS:88812f4c() knlGS: [6.200601] CS: 0010 DS: ES: CR0: 80050033 [6.206339] CR2: 55676cc01000 CR3: 000103f6c000 CR4: 06f0 [6.213466] Call Trace: [6.215921] [6.218015] ? __warn+0x96/0x160 [6.221240] ? nv50_display_init+0x28c/0x4f0 [nouveau] [6.226461] ? report_bug+0x1ec/0x200 [6.230119] ? handle_bug+0x3c/0x70 [6.233611] ? exc_invalid_op+0x1f/0x90 [6.237442] ? asm_exc_invalid_op+0x16/0x20 [6.241622] ? nv50_display_init+0x28c/0x4f0 [nouveau] [6.246840] ? nv50_display_init+0x28c/0x4f0 [nouveau] [6.252058] ? sched_set_fifo+0x46/0x60 [6.255897] nouveau_display_init+0xa0/0xd0 [nouveau] [6.261031] nouveau_drm_device_init+0x42a/0x990 [nouveau] [6.266604] nouveau_drm_probe+0x105/0x240 [nouveau] [6.271651] ? __pm_runtime_resume+0x68/0xa0 [6.275920] pci_device_probe+0xaa/0x140 [6.279840] really_probe+0xc2/0x2d0 [6.283411] __driver_probe_device+0x73/0x120 [6.287761] driver_probe_device+0x2c/0xb0 [6.291851] __driver_attach+0xa0/0x150 [6.295683] ? __device_attach_driver+0xc0/0xc0 [6.300205] bus_for_each_dev+0x67/0xa0 [6.304044] bus_add_driver+0x10e/0x210 [6.307874] driver_register+0x5c/0x120 [6.311706] ? 0xa0336000 [6.315017] do_one_initcall+0x44/0x200 [6.318851] ? kmalloc_trace+0x37/0xc0 [6.322595] do_init_module+0x64/0x230 [6.326344] init_module_from_file+0x8d/0xd0 [6.330609] idempotent_init_module+0x15a/0x210 [6.335136] __x64_sys_finit_module+0x67/0xb0 [6.339490] do_syscall_64+0x41/0xf0 [6.343066] entry_SYSCALL_64_after_hwframe+0x4b/0x53 [6.348118] RIP: 0033:0x7fdc14947ee9 [6.351691] Code: 08 44 89 e0 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d f7 ee 0e 00 f7 d8 64 89 01 48 [6.370433] RSP:
Re: [PATCH v4 01/15] drm/msm/dpu: add formats check for writeback encoder
On 12/12/2023 2:03 PM, Dmitry Baryshkov wrote:
On Tue, 12 Dec 2023 at 23:30, Dmitry Baryshkov
wrote:
On Tue, 12 Dec 2023 at 22:53, Abhinav Kumar wrote:
In preparation for adding more formats to dpu writeback add
format validation to it to fail any unsupported formats.
changes in v4:
- change the failure message of the API
drm_atomic_helper_check_wb_connector_state() to a generic
one in case it checks more errors later and moreoever it
already has debug message to indicate its failure
- change the corresponding DPU_ERROR to DPU_DEBUG in-line with
other atomic_check failure messages
changes in v3:
- rebase on top of msm-next
- replace drm_atomic_helper_check_wb_encoder_state() with
drm_atomic_helper_check_wb_connector_state() due to the
rebase
changes in v2:
- correct some grammar in the commit text
Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
writeback")
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index bb94909caa25..4953d87affa1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check(
{
struct drm_framebuffer *fb;
const struct drm_display_mode *mode = _state->mode;
+ int ret;
DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
phys_enc->hw_wb->idx, mode->name, mode->hdisplay,
mode->vdisplay);
@@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check(
return -EINVAL;
}
+ ret = drm_atomic_helper_check_wb_connector_state(conn_state->connector,
conn_state->state);
+ if (ret < 0) {
+ DPU_DEBUG("wb check connector state failed ret = %d\n", ret);
We already have a debug message in
drm_atomic_helper_check_wb_connector_state(). Can we please drop this
one?
If you don't mind, I can just drop it while applying
Ack. Thank you.
+ return ret;
+ }
+
return 0;
}
--
2.40.1
--
With best wishes
Dmitry
Re: [PATCH v4 01/15] drm/msm/dpu: add formats check for writeback encoder
On Tue, 12 Dec 2023 at 23:30, Dmitry Baryshkov
wrote:
>
> On Tue, 12 Dec 2023 at 22:53, Abhinav Kumar wrote:
> >
> > In preparation for adding more formats to dpu writeback add
> > format validation to it to fail any unsupported formats.
> >
> > changes in v4:
> > - change the failure message of the API
> > drm_atomic_helper_check_wb_connector_state() to a generic
> > one in case it checks more errors later and moreoever it
> > already has debug message to indicate its failure
> > - change the corresponding DPU_ERROR to DPU_DEBUG in-line with
> > other atomic_check failure messages
> >
> > changes in v3:
> > - rebase on top of msm-next
> > - replace drm_atomic_helper_check_wb_encoder_state() with
> > drm_atomic_helper_check_wb_connector_state() due to the
> > rebase
> >
> > changes in v2:
> > - correct some grammar in the commit text
> >
> > Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
> > writeback")
> > Signed-off-by: Abhinav Kumar
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> > index bb94909caa25..4953d87affa1 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> > @@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check(
> > {
> > struct drm_framebuffer *fb;
> > const struct drm_display_mode *mode = _state->mode;
> > + int ret;
> >
> > DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
> > phys_enc->hw_wb->idx, mode->name, mode->hdisplay,
> > mode->vdisplay);
> > @@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check(
> > return -EINVAL;
> > }
> >
> > + ret =
> > drm_atomic_helper_check_wb_connector_state(conn_state->connector,
> > conn_state->state);
> > + if (ret < 0) {
> > + DPU_DEBUG("wb check connector state failed ret = %d\n",
> > ret);
>
> We already have a debug message in
> drm_atomic_helper_check_wb_connector_state(). Can we please drop this
> one?
If you don't mind, I can just drop it while applying
>
> > + return ret;
> > + }
> > +
> > return 0;
> > }
> >
> > --
> > 2.40.1
> >
>
>
> --
> With best wishes
> Dmitry
--
With best wishes
Dmitry
Re: [PATCH v4] drm/msm/dpu: improve DSC allocation
On Tue, 12 Dec 2023 at 23:37, Kuogee Hsieh wrote:
>
> At DSC V1.1 DCE (Display Compression Engine) contains a DSC encoder.
> However, at DSC V1.2 DCE consists of two DSC encoders, one has an odd
> index and another one has an even index. Each encoder can work
> independently. But only two DSC encoders from same DCE can be paired
> to work together to support DSC merge mode at DSC V1.2. For DSC V1.1
> two consecutive DSC encoders (start with even index) have to be paired
> to support DSC merge mode. In addition, the DSC with even index have
> to be mapped to even PINGPONG index and DSC with odd index have to be
> mapped to odd PINGPONG index at its data path in regardless of DSC
> V1.1 or V1.2. This patch improves DSC allocation mechanism with
> consideration of those factors.
>
> Changes in V4:
> -- rework commit message
> -- use reserved_by_other()
> -- add _dpu_rm_pingpong_next_index()
> -- revise _dpu_rm_pingpong_dsc_check()
>
> Changes in V3:
> -- add dpu_rm_pingpong_dsc_check()
> -- for pair allocation use i += 2 at for loop
>
> Changes in V2:
> -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and
>_dpu_rm_reserve_dsc_pair()
>
> Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM")
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 178
> ++---
> 1 file changed, 163 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f9215643..15317b6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -461,29 +461,177 @@ static int _dpu_rm_reserve_ctls(
> return 0;
> }
>
> -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> +static int _dpu_rm_pingpong_next_index(int start,
> +uint32_t enc_id,
> +uint32_t *pp_to_enc_id,
> +int pp_max)
Wrong indentation.
> +{
> + int pp_ndx;
> +
> + for (pp_ndx = start; pp_ndx < pp_max; pp_ndx++) {
git grep _ndx returns nearly nothing for the whole kernel. I think 'i'
is more than enough both here and in the loops over dsc_to_enc_id.
> + if (pp_to_enc_id[pp_ndx] == enc_id)
> + return pp_ndx;
> + }
> +
> + return -ENAVAIL;
> +}
> +
> +static int _dpu_rm_pingpong_dsc_check(int dsc_ndx, int pp_ndx)
Same here. It is _idx or _index.
> +{
> +
> + /*
> +* dsc even index must be mapped to pingpong even index
DSC, PINGPONG. I think I had this comment for v3. You fixed the commit
message. What stopped you from fixing the comment?
DSC with even index must be used with the PINGPONG with even index
> +* dsc odd index must be mapped to pingpong odd index
> +*/
> + if ((dsc_ndx & 0x01) != (pp_ndx & 0x01))
> + return -ENAVAIL;
> +
> + return 0;
> +}
> +
> +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm,
Still _single. Even though it can get a request for two DSC blocks.
>struct dpu_global_state *global_state,
> - struct drm_encoder *enc,
> + uint32_t enc_id,
> + int *dsc_id,
>const struct msm_display_topology *top)
Wrong indentation.
> {
> - int num_dsc = top->num_dsc;
> - int i;
> + int num_dsc = 0;
> + uint32_t *pp_to_enc_id = global_state->pingpong_to_enc_id;
> + uint32_t *dsc_enc_id = global_state->dsc_to_enc_id;
> + int pp_max = PINGPONG_MAX - PINGPONG_0;
> + int pp_ndx;
> + int dsc_ndx;
> + int ret;
>
> - /* check if DSC required are allocated or not */
> - for (i = 0; i < num_dsc; i++) {
> - if (!rm->dsc_blks[i]) {
> - DPU_ERROR("DSC %d does not exist\n", i);
> - return -EIO;
> - }
> + for (dsc_ndx = 0; dsc_ndx < ARRAY_SIZE(rm->dsc_blks); dsc_ndx++) {
> + if (!rm->dsc_blks[dsc_ndx])
> + continue;
> +
> + if (reserved_by_other(dsc_enc_id, dsc_ndx, enc_id))
> + continue;
> +
> + pp_ndx = _dpu_rm_pingpong_next_index(0, enc_id, pp_to_enc_id,
> pp_max);
Error here.
> + if (pp_ndx < 0)
> + return -ENAVAIL;
> +
> + ret = _dpu_rm_pingpong_dsc_check(dsc_ndx, pp_ndx);
> + if (ret)
> + return -ENAVAIL;
> +
> + dsc_id[num_dsc++] = DSC_0 + dsc_ndx;/* found */
The comment is useless.
> +
> + if (num_dsc >= top->num_dsc)
> + break;
Can this go to the loop condition please.
> + }
>
> - if (global_state->dsc_to_enc_id[i]) {
> - DPU_ERROR("DSC %d is already allocated\n", i);
Re: [PATCH] drm/tegra: include drm/drm_edid.h only where needed
Hi Jani,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-exynos/exynos-drm-next drm-intel/for-linux-next
drm-intel/for-linux-next-fixes drm-misc/drm-misc-next drm-tip/drm-tip
linus/master v6.7-rc5 next-20231212]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-tegra-include-drm-drm_edid-h-only-where-needed/20231212-222603
base: git://anongit.freedesktop.org/drm/drm drm-next
patch link:
https://lore.kernel.org/r/20231212142409.3826544-1-jani.nikula%40intel.com
patch subject: [PATCH] drm/tegra: include drm/drm_edid.h only where needed
config: s390-allmodconfig
(https://download.01.org/0day-ci/archive/20231213/202312130548.frtzxidd-...@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231213/202312130548.frtzxidd-...@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/202312130548.frtzxidd-...@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/tegra/sor.c: In function
'tegra_sor_hdmi_setup_avi_infoframe':
>> drivers/gpu/drm/tegra/sor.c:1928:15: error: implicit declaration of function
>> 'drm_hdmi_avi_infoframe_from_display_mode'
>> [-Werror=implicit-function-declaration]
1928 | err = drm_hdmi_avi_infoframe_from_display_mode(,
| ^~~~
cc1: some warnings being treated as errors
vim +/drm_hdmi_avi_infoframe_from_display_mode +1928 drivers/gpu/drm/tegra/sor.c
459cc2c6800b54 Thierry Reding 2015-07-30 1911
459cc2c6800b54 Thierry Reding 2015-07-30 1912 static int
459cc2c6800b54 Thierry Reding 2015-07-30 1913
tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
459cc2c6800b54 Thierry Reding 2015-07-30 1914
const struct drm_display_mode *mode)
459cc2c6800b54 Thierry Reding 2015-07-30 1915 {
459cc2c6800b54 Thierry Reding 2015-07-30 1916 u8
buffer[HDMI_INFOFRAME_SIZE(AVI)];
459cc2c6800b54 Thierry Reding 2015-07-30 1917 struct
hdmi_avi_infoframe frame;
459cc2c6800b54 Thierry Reding 2015-07-30 1918 u32 value;
459cc2c6800b54 Thierry Reding 2015-07-30 1919 int err;
459cc2c6800b54 Thierry Reding 2015-07-30 1920
459cc2c6800b54 Thierry Reding 2015-07-30 1921 /* disable AVI
infoframe */
459cc2c6800b54 Thierry Reding 2015-07-30 1922 value =
tegra_sor_readl(sor, SOR_HDMI_AVI_INFOFRAME_CTRL);
459cc2c6800b54 Thierry Reding 2015-07-30 1923 value &=
~INFOFRAME_CTRL_SINGLE;
459cc2c6800b54 Thierry Reding 2015-07-30 1924 value &=
~INFOFRAME_CTRL_OTHER;
459cc2c6800b54 Thierry Reding 2015-07-30 1925 value &=
~INFOFRAME_CTRL_ENABLE;
459cc2c6800b54 Thierry Reding 2015-07-30 1926 tegra_sor_writel(sor,
value, SOR_HDMI_AVI_INFOFRAME_CTRL);
459cc2c6800b54 Thierry Reding 2015-07-30 1927
13d0add333afea Ville Syrjälä 2019-01-08 @1928 err =
drm_hdmi_avi_infoframe_from_display_mode(,
13d0add333afea Ville Syrjälä 2019-01-08 1929
>output.connector, mode);
459cc2c6800b54 Thierry Reding 2015-07-30 1930 if (err < 0) {
459cc2c6800b54 Thierry Reding 2015-07-30 1931
dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
459cc2c6800b54 Thierry Reding 2015-07-30 1932 return err;
459cc2c6800b54 Thierry Reding 2015-07-30 1933 }
459cc2c6800b54 Thierry Reding 2015-07-30 1934
459cc2c6800b54 Thierry Reding 2015-07-30 1935 err =
hdmi_avi_infoframe_pack(, buffer, sizeof(buffer));
459cc2c6800b54 Thierry Reding 2015-07-30 1936 if (err < 0) {
459cc2c6800b54 Thierry Reding 2015-07-30 1937
dev_err(sor->dev, "failed to pack AVI infoframe: %d\n", err);
459cc2c6800b54 Thierry Reding 2015-07-30 1938 return err;
459cc2c6800b54 Thierry Reding 2015-07-30 1939 }
459cc2c6800b54 Thierry Reding 2015-07-30 1940
459cc2c6800b54 Thierry Reding 2015-07-30 1941
tegra_sor_hdmi_write_infopack(sor, buffer, err);
459cc2c6800b54 Thierry Reding 2015-07-30 1942
459cc2c6800b54 Thierry Reding 2015-07-30 1943 /* enable AVI infoframe
*/
459cc2c6800b54 Thierry Reding 2015-07-30 1944 value =
tegra_sor_readl(sor, SOR_HDMI_AVI_INFOFRAME_CTRL);
459cc2c6800b54 Thierry Reding 2015-07-30 1945 value |=
INFOFRAME_CTRL_CHECKSUM_ENABLE;
459cc2c6800b54 T
Re: [PATCH v4 07/15] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block
On Tue, 12 Dec 2023 at 22:53, Abhinav Kumar wrote: > > CDM block comes with its own set of registers and operations > which can be done. In-line with other hardware blocks, this > change adds the dpu_hw_cdm abstraction for the CDM block. > > changes in v4: > - used FIELD_PREP() for dpu_hw_cdm_setup_cdwn() operations > - change to lowercase hex in dpu_hw_cdm_bind_pingpong_blk() > - move disable assignment inside else in > dpu_hw_cdm_bind_pingpong_blk() > > changes in v3: > - fix commit text from sub-blk to blk for CDM > - fix kbot issue for missing static for dpu_hw_cdm_enable() > - fix kbot issue for incorrect documentation style > - add more documentation for enums and struct in dpu_hw_cdm.h > - drop "enable" parameter from bind_pingpong_blk() as we can > just use PINGPONG_NONE for disable cases > - drop unnecessary bit operation for zero value of cdm_cfg > > changes in v2: > - replace bit magic with relevant defines > - use drmm_kzalloc instead of kzalloc/free > - some formatting fixes > - inline _setup_cdm_ops() > - protect bind_pingpong_blk with core_rev check > - drop setup_csc_data() and setup_cdwn() ops as they > are merged into enable() > > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202312101815.b3zh7pfy-...@intel.com/ > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/Makefile| 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 245 > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 142 > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > 4 files changed, 389 insertions(+) > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v4] drm/msm/dpu: improve DSC allocation
At DSC V1.1 DCE (Display Compression Engine) contains a DSC encoder.
However, at DSC V1.2 DCE consists of two DSC encoders, one has an odd
index and another one has an even index. Each encoder can work
independently. But only two DSC encoders from same DCE can be paired
to work together to support DSC merge mode at DSC V1.2. For DSC V1.1
two consecutive DSC encoders (start with even index) have to be paired
to support DSC merge mode. In addition, the DSC with even index have
to be mapped to even PINGPONG index and DSC with odd index have to be
mapped to odd PINGPONG index at its data path in regardless of DSC
V1.1 or V1.2. This patch improves DSC allocation mechanism with
consideration of those factors.
Changes in V4:
-- rework commit message
-- use reserved_by_other()
-- add _dpu_rm_pingpong_next_index()
-- revise _dpu_rm_pingpong_dsc_check()
Changes in V3:
-- add dpu_rm_pingpong_dsc_check()
-- for pair allocation use i += 2 at for loop
Changes in V2:
-- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and
_dpu_rm_reserve_dsc_pair()
Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 178 ++---
1 file changed, 163 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643..15317b6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -461,29 +461,177 @@ static int _dpu_rm_reserve_ctls(
return 0;
}
-static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
+static int _dpu_rm_pingpong_next_index(int start,
+uint32_t enc_id,
+uint32_t *pp_to_enc_id,
+int pp_max)
+{
+ int pp_ndx;
+
+ for (pp_ndx = start; pp_ndx < pp_max; pp_ndx++) {
+ if (pp_to_enc_id[pp_ndx] == enc_id)
+ return pp_ndx;
+ }
+
+ return -ENAVAIL;
+}
+
+static int _dpu_rm_pingpong_dsc_check(int dsc_ndx, int pp_ndx)
+{
+
+ /*
+* dsc even index must be mapped to pingpong even index
+* dsc odd index must be mapped to pingpong odd index
+*/
+ if ((dsc_ndx & 0x01) != (pp_ndx & 0x01))
+ return -ENAVAIL;
+
+ return 0;
+}
+
+static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm,
struct dpu_global_state *global_state,
- struct drm_encoder *enc,
+ uint32_t enc_id,
+ int *dsc_id,
const struct msm_display_topology *top)
{
- int num_dsc = top->num_dsc;
- int i;
+ int num_dsc = 0;
+ uint32_t *pp_to_enc_id = global_state->pingpong_to_enc_id;
+ uint32_t *dsc_enc_id = global_state->dsc_to_enc_id;
+ int pp_max = PINGPONG_MAX - PINGPONG_0;
+ int pp_ndx;
+ int dsc_ndx;
+ int ret;
- /* check if DSC required are allocated or not */
- for (i = 0; i < num_dsc; i++) {
- if (!rm->dsc_blks[i]) {
- DPU_ERROR("DSC %d does not exist\n", i);
- return -EIO;
- }
+ for (dsc_ndx = 0; dsc_ndx < ARRAY_SIZE(rm->dsc_blks); dsc_ndx++) {
+ if (!rm->dsc_blks[dsc_ndx])
+ continue;
+
+ if (reserved_by_other(dsc_enc_id, dsc_ndx, enc_id))
+ continue;
+
+ pp_ndx = _dpu_rm_pingpong_next_index(0, enc_id, pp_to_enc_id,
pp_max);
+ if (pp_ndx < 0)
+ return -ENAVAIL;
+
+ ret = _dpu_rm_pingpong_dsc_check(dsc_ndx, pp_ndx);
+ if (ret)
+ return -ENAVAIL;
+
+ dsc_id[num_dsc++] = DSC_0 + dsc_ndx;/* found */
+
+ if (num_dsc >= top->num_dsc)
+ break;
+ }
- if (global_state->dsc_to_enc_id[i]) {
- DPU_ERROR("DSC %d is already allocated\n", i);
- return -EIO;
+ if (num_dsc < top->num_dsc) {
+ DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
+ num_dsc, top->num_dsc);
+ return -ENAVAIL;
+ }
+
+ return 0;
+}
+
+static int _dpu_rm_reserve_dsc_pair(struct dpu_rm *rm,
+ struct dpu_global_state *global_state,
+ uint32_t enc_id,
+ int *dsc_id,
+ const struct msm_display_topology *top)
+{
+ int num_dsc = 0;
+ uint32_t *pp_to_enc_id = global_state->pingpong_to_enc_id;
+ uint32_t *dsc_enc_id = global_state->dsc_to_enc_id;
+ int pp_max = PINGPONG_MAX - PINGPONG_0;
+ int start_pp_ndx = 0;
+ int dsc_ndx, pp_ndx;
Re: [PATCH v3 01/15] drm/msm/dpu: add formats check for writeback encoder
On Tue, 12 Dec 2023 at 19:17, Abhinav Kumar wrote:
>
>
>
> On 12/11/2023 10:40 PM, Dmitry Baryshkov wrote:
> > On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar
> > wrote:
> >>
> >> In preparation for adding more formats to dpu writeback add
> >> format validation to it to fail any unsupported formats.
> >>
> >> changes in v3:
> >> - rebase on top of msm-next
> >> - replace drm_atomic_helper_check_wb_encoder_state() with
> >>drm_atomic_helper_check_wb_connector_state() due to the
> >>rebase
> >>
> >> changes in v2:
> >> - correct some grammar in the commit text
> >>
> >> Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
> >> writeback")
> >> Signed-off-by: Abhinav Kumar
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >> index bb94909caa25..425415d45ec1 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >> @@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check(
> >> {
> >> struct drm_framebuffer *fb;
> >> const struct drm_display_mode *mode = _state->mode;
> >> + int ret;
> >>
> >> DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
> >> phys_enc->hw_wb->idx, mode->name, mode->hdisplay,
> >> mode->vdisplay);
> >> @@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check(
> >> return -EINVAL;
> >> }
> >>
> >> + ret =
> >> drm_atomic_helper_check_wb_connector_state(conn_state->connector,
> >> conn_state->state);
> >> + if (ret < 0) {
> >> + DPU_ERROR("invalid pixel format %p4cc\n",
> >> >format->format);
> >> + return ret;
> >> + }
> >
> > There is no guarantee that there will be no other checks added to this
> > helper. So, I think this message is incorrect. If you wish, you can
> > promote the level of the message in the helper itself.
> > On the other hand, we rarely print such messages by default. Most of
> > the checks use drm_dbg.
> >
>
> hmm...actually drm_atomic_helper_check_wb_connector_state() already has
> a debug message to indicate invalid pixel formats.
>
> You are right, i should perhaps just say that "atomic_check failed" or
> something.
>
> I can make this a DPU_DEBUG. Actually I didnt know that we are not
> supposed to print out atomic_check() errors. Is it perhaps because its
> okay for check to fail?
There are no messages by default there, because otherwise it is so
easy for the user to overspam the dmesg and thus syslog / journal. DoS
on the plate.
>
> But then we would not know why it failed.
Think about the user of X11. They don't see the console. And by
default in the contemporary distros they won't be able to check dmesg.
So if a commit fails, they have to deduce anyway, why did it fail.
>
> >> +
> >> return 0;
> >> }
> >>
> >> --
> >> 2.40.1
> >>
> >
> >
--
With best wishes
Dmitry
Re: [PATCH v4 01/15] drm/msm/dpu: add formats check for writeback encoder
On Tue, 12 Dec 2023 at 22:53, Abhinav Kumar wrote:
>
> In preparation for adding more formats to dpu writeback add
> format validation to it to fail any unsupported formats.
>
> changes in v4:
> - change the failure message of the API
> drm_atomic_helper_check_wb_connector_state() to a generic
> one in case it checks more errors later and moreoever it
> already has debug message to indicate its failure
> - change the corresponding DPU_ERROR to DPU_DEBUG in-line with
> other atomic_check failure messages
>
> changes in v3:
> - rebase on top of msm-next
> - replace drm_atomic_helper_check_wb_encoder_state() with
> drm_atomic_helper_check_wb_connector_state() due to the
> rebase
>
> changes in v2:
> - correct some grammar in the commit text
>
> Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
> writeback")
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> index bb94909caa25..4953d87affa1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> @@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check(
> {
> struct drm_framebuffer *fb;
> const struct drm_display_mode *mode = _state->mode;
> + int ret;
>
> DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
> phys_enc->hw_wb->idx, mode->name, mode->hdisplay,
> mode->vdisplay);
> @@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check(
> return -EINVAL;
> }
>
> + ret =
> drm_atomic_helper_check_wb_connector_state(conn_state->connector,
> conn_state->state);
> + if (ret < 0) {
> + DPU_DEBUG("wb check connector state failed ret = %d\n", ret);
We already have a debug message in
drm_atomic_helper_check_wb_connector_state(). Can we please drop this
one?
> + return ret;
> + }
> +
> return 0;
> }
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
Re: [PATCH v3 07/15] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block
On Tue, 12 Dec 2023 at 19:51, Abhinav Kumar wrote:
>
>
>
> On 12/12/2023 1:40 AM, Dmitry Baryshkov wrote:
> > On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar
> > wrote:
> >>
> >> CDM block comes with its own set of registers and operations
> >> which can be done. In-line with other hardware blocks, this
> >> change adds the dpu_hw_cdm abstraction for the CDM block.
> >>
> >> changes in v3:
> >> - fix commit text from sub-blk to blk for CDM
> >> - fix kbot issue for missing static for dpu_hw_cdm_enable()
> >> - fix kbot issue for incorrect documentation style
> >> - add more documentation for enums and struct in dpu_hw_cdm.h
> >> - drop "enable" parameter from bind_pingpong_blk() as we can
> >>just use PINGPONG_NONE for disable cases
> >> - drop unnecessary bit operation for zero value of cdm_cfg
> >>
> >> changes in v2:
> >> - replace bit magic with relevant defines
> >> - use drmm_kzalloc instead of kzalloc/free
> >> - some formatting fixes
> >> - inline _setup_cdm_ops()
> >> - protect bind_pingpong_blk with core_rev check
> >> - drop setup_csc_data() and setup_cdwn() ops as they
> >>are merged into enable()
> >>
> >> Reported-by: kernel test robot
> >> Closes:
> >> https://lore.kernel.org/oe-kbuild-all/202312101815.b3zh7pfy-...@intel.com/
> >> Signed-off-by: Abhinav Kumar
> >> ---
> >> drivers/gpu/drm/msm/Makefile| 1 +
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 263
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
> >> 4 files changed, 395 insertions(+)
> >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
> >>
> >> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> >> index 49671364fdcf..b1173128b5b9 100644
> >> --- a/drivers/gpu/drm/msm/Makefile
> >> +++ b/drivers/gpu/drm/msm/Makefile
> >> @@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
> >> disp/dpu1/dpu_encoder_phys_wb.o \
> >> disp/dpu1/dpu_formats.o \
> >> disp/dpu1/dpu_hw_catalog.o \
> >> + disp/dpu1/dpu_hw_cdm.o \
> >> disp/dpu1/dpu_hw_ctl.o \
> >> disp/dpu1/dpu_hw_dsc.o \
> >> disp/dpu1/dpu_hw_dsc_1_2.o \
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> new file mode 100644
> >> index ..4976f8a05ce7
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> @@ -0,0 +1,263 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
> >> + */
> >> +
> >> +#include
> >> +
> >> +#include "dpu_hw_mdss.h"
> >> +#include "dpu_hw_util.h"
> >> +#include "dpu_hw_catalog.h"
> >> +#include "dpu_hw_cdm.h"
> >> +#include "dpu_kms.h"
> >> +
> >> +#define CDM_CSC_10_OPMODE 0x000
> >> +#define CDM_CSC_10_BASE0x004
> >> +
> >> +#define CDM_CDWN2_OP_MODE 0x100
> >> +#define CDM_CDWN2_CLAMP_OUT0x104
> >> +#define CDM_CDWN2_PARAMS_3D_0 0x108
> >> +#define CDM_CDWN2_PARAMS_3D_1 0x10C
> >> +#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
> >> +#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
> >> +#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
> >> +#define CDM_CDWN2_COEFF_OFFSITE_H_00x11C
> >> +#define CDM_CDWN2_COEFF_OFFSITE_H_10x120
> >> +#define CDM_CDWN2_COEFF_OFFSITE_H_20x124
> >> +#define CDM_CDWN2_COEFF_COSITE_V 0x128
> >> +#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
> >> +#define CDM_CDWN2_OUT_SIZE 0x130
> >> +
> >> +#define CDM_HDMI_PACK_OP_MODE 0x200
> >> +#define CDM_CSC_10_MATRIX_COEFF_0 0x004
> >> +
> >> +#define CDM_MUX0x224
> >> +
> >> +/* CDM CDWN2 sub-block bit definitions */
> >> +#define CDM_CDWN2_OP_MODE_EN BIT(0)
> >> +#define CDM_CDWN2_OP_MODE_ENABLE_HBIT(1)
> >> +#define CDM_CDWN2_OP_MODE_ENABLE_VBIT(2)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_H_AVGBIT(3)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_V_AVGBIT(5)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6)
> >> +#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITEGENMASK(4, 3)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITEGENMASK(6, 5)
> >
> > I think it might be easier to define
> >
> > enum {
> >CDM_CDWN2_METHOD_DROP_PIXEL = 0,
> >CDM_CDWN2_METHOD_AVG = 1,
> >CDM_CDWN2_METHOD_ = 2,
> >CDM_CDWN2_METHOD_DROP_PIXEL = 3,
> > };
> >
> > then use FIELD_PREP()
> >
>
> Ok, let me explore that option.
>
> We already
Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips
Hello Thierry,
On Mon, Dec 11, 2023 at 12:33:04PM +0100, Thierry Reding wrote:
> On Fri, Dec 08, 2023 at 07:50:33PM +0100, Uwe Kleine-König wrote:
> > You don't need to touch all drivers because you didn't change struct
> > pwm_chip::dev yet. (If you really want, you don't need to change that,
> > but then you have some duplication as chip->dev holds the same value as
> > priv->dev.parent in the end.)
>
> I don't think that's a problem. These are for two logically separate
> things, after all.
How are they different? I'd say one is the initializer for the other and
(ideally) unused after that. With that interpretation they are indeed
different, but then it's ugly that the initializer keeps staying around.
> Duplication can also sometimes be useful to simplify
> things. There are plently of cases where we use local variables for the
> same reason.
local variables go away though after the respective function is left.
chip->dev and its copy priv->dev.parent stay around for the full
lifetime of the chip.
> > > @@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const
> > > char *name)
> > >
> > > static int pwm_device_request(struct pwm_device *pwm, const char *label)
> > > {
> > > + struct pwm_chip *chip = pwm->priv->chip;
> >
> > With my approach getting the chip of a struct pwm_device is only one
> > pointer dereference away. You need two.
>
> None of the functions here are called very often, so even if this isn't
> optimized away it would hardly matter.
I'd say pwm_apply_state() at least matters. Also I think that making a
slow path quicker is a good thing.
I wonder how we'll converge to an approach that can go into the
mainline given that we both have our strong opinions.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH] fbdev/simplefb: change loglevel when the power domains cannot be parsed
On Tue, Dec 12, 2023 at 02:57:54PM -0500, Brian Masney wrote:
> When the power domains cannot be parsed, the message is incorrectly
> logged as an info message. Let's change this to an error since an error
> is returned.
>
> Fixes: 92a511a568e4 ("fbdev/simplefb: Add support for generic power-domains")
> Signed-off-by: Brian Masney
Acked-by: Andrew Halaney
> ---
> drivers/video/fbdev/simplefb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 6f58ee276ad1..028a56525047 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -470,7 +470,7 @@ static int simplefb_attach_genpds(struct simplefb_par
> *par,
> if (err == -ENOENT)
> return 0;
>
> - dev_info(dev, "failed to parse power-domains: %d\n", err);
> + dev_err(dev, "failed to parse power-domains: %d\n", err);
> return err;
> }
>
> --
> 2.43.0
>
[PATCH v4 13/15] drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output
Reserve CDM blocks for writeback if the format of the output fb
is YUV. At the moment, the reservation is done only for writeback
but can easily be extended by relaxing the checks once other
interfaces are ready to output YUV.
changes in v3:
- squash CDM disable during encoder cleanup into this change
changes in v2:
- use needs_cdm from topology struct
- drop fb related checks from atomic_mode_set()
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 889e9bb42715..989ee8c0e5b4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include "msm_drv.h"
#include "dpu_kms.h"
@@ -26,6 +27,7 @@
#include "dpu_hw_dspp.h"
#include "dpu_hw_dsc.h"
#include "dpu_hw_merge3d.h"
+#include "dpu_hw_cdm.h"
#include "dpu_formats.h"
#include "dpu_encoder_phys.h"
#include "dpu_crtc.h"
@@ -582,6 +584,7 @@ static int dpu_encoder_virt_atomic_check(
struct drm_display_mode *adj_mode;
struct msm_display_topology topology;
struct dpu_global_state *global_state;
+ struct drm_framebuffer *fb;
struct drm_dsc_config *dsc;
int i = 0;
int ret = 0;
@@ -622,6 +625,22 @@ static int dpu_encoder_virt_atomic_check(
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode,
crtc_state, dsc);
+ /*
+* Use CDM only for writeback at the moment as other interfaces cannot
handle it.
+* if writeback itself cannot handle cdm for some reason it will fail
in its atomic_check()
+* earlier.
+*/
+ if (dpu_enc->disp_info.intf_type == INTF_WB &&
conn_state->writeback_job) {
+ fb = conn_state->writeback_job->fb;
+
+ if (fb &&
DPU_FORMAT_IS_YUV(to_dpu_format(msm_framebuffer_format(fb
+ topology.needs_cdm = true;
+ if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm)
+ crtc_state->mode_changed = true;
+ else if (!topology.needs_cdm && dpu_enc->cur_master->hw_cdm)
+ crtc_state->mode_changed = true;
+ }
+
/*
* Release and Allocate resources on every modeset
* Dont allocate when active is false.
@@ -1062,6 +1081,15 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
dpu_enc->dsc_mask = dsc_mask;
+ if (dpu_enc->disp_info.intf_type == INTF_WB &&
conn_state->writeback_job) {
+ struct dpu_hw_blk *hw_cdm = NULL;
+
+ dpu_rm_get_assigned_resources(_kms->rm, global_state,
+ drm_enc->base.id, DPU_HW_BLK_CDM,
+ _cdm, 1);
+ dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) :
NULL;
+ }
+
cstate = to_dpu_crtc_state(crtc_state);
for (i = 0; i < num_lm; i++) {
@@ -2050,6 +2078,15 @@ void dpu_encoder_helper_phys_cleanup(struct
dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->merge_3d->idx);
}
+ if (phys_enc->hw_cdm) {
+ if (phys_enc->hw_cdm->ops.bind_pingpong_blk && phys_enc->hw_pp)
+
phys_enc->hw_cdm->ops.bind_pingpong_blk(phys_enc->hw_cdm,
+ PINGPONG_NONE);
+ if (phys_enc->hw_ctl->ops.update_pending_flush_cdm)
+
phys_enc->hw_ctl->ops.update_pending_flush_cdm(phys_enc->hw_ctl,
+
phys_enc->hw_cdm->idx);
+ }
+
if (dpu_enc->dsc) {
dpu_encoder_unprep_dsc(dpu_enc);
dpu_enc->dsc = NULL;
--
2.40.1
[PATCH v4 14/15] drm/msm/dpu: introduce separate wb2_format arrays for rgb and yuv
Lets rename the existing wb2_formats array wb2_formats_rgb to indicate
that it has only RGB formats and can be used on any chipset having a WB
block.
Introduce a new wb2_formats_rgb_yuv array to the catalog to
indicate support for YUV formats to writeback in addition to RGB.
Chipsets which have support for CDM block will use the newly added
wb2_formats_rgb_yuv array.
changes in v3:
- change type of wb2_formats_rgb/wb2_formats_rgb_yuv to u32
to fix checkpatch warnings
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
.../msm/disp/dpu1/catalog/dpu_10_0_sm8650.h | 4 +-
.../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h| 4 +-
.../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h| 4 +-
.../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h| 4 +-
.../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h| 4 +-
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 37 ++-
6 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
index 04d2a73dd942..eb5dfff2ec4f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
@@ -341,8 +341,8 @@ static const struct dpu_wb_cfg sm8650_wb[] = {
.name = "wb_2", .id = WB_2,
.base = 0x65000, .len = 0x2c8,
.features = WB_SM8250_MASK,
- .format_list = wb2_formats,
- .num_formats = ARRAY_SIZE(wb2_formats),
+ .format_list = wb2_formats_rgb,
+ .num_formats = ARRAY_SIZE(wb2_formats_rgb),
.xin_id = 6,
.vbif_idx = VBIF_RT,
.maxlinewidth = 4096,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
index 58b0f50518c8..a57d50b1f028 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
@@ -336,8 +336,8 @@ static const struct dpu_wb_cfg sm8250_wb[] = {
.name = "wb_2", .id = WB_2,
.base = 0x65000, .len = 0x2c8,
.features = WB_SM8250_MASK,
- .format_list = wb2_formats,
- .num_formats = ARRAY_SIZE(wb2_formats),
+ .format_list = wb2_formats_rgb_yuv,
+ .num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv),
.clk_ctrl = DPU_CLK_CTRL_WB2,
.xin_id = 6,
.vbif_idx = VBIF_RT,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
index bcfedfc8251a..7382ebb6e5b2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
@@ -157,8 +157,8 @@ static const struct dpu_wb_cfg sc7180_wb[] = {
.name = "wb_2", .id = WB_2,
.base = 0x65000, .len = 0x2c8,
.features = WB_SM8250_MASK,
- .format_list = wb2_formats,
- .num_formats = ARRAY_SIZE(wb2_formats),
+ .format_list = wb2_formats_rgb,
+ .num_formats = ARRAY_SIZE(wb2_formats_rgb),
.clk_ctrl = DPU_CLK_CTRL_WB2,
.xin_id = 6,
.vbif_idx = VBIF_RT,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
index 19c2b7454796..2f153e0b5c6a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
@@ -169,8 +169,8 @@ static const struct dpu_wb_cfg sc7280_wb[] = {
.name = "wb_2", .id = WB_2,
.base = 0x65000, .len = 0x2c8,
.features = WB_SM8250_MASK,
- .format_list = wb2_formats,
- .num_formats = ARRAY_SIZE(wb2_formats),
+ .format_list = wb2_formats_rgb_yuv,
+ .num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv),
.clk_ctrl = DPU_CLK_CTRL_WB2,
.xin_id = 6,
.vbif_idx = VBIF_RT,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index bf56265967c0..ad48defa154f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -315,8 +315,8 @@ static const struct dpu_wb_cfg sm8550_wb[] = {
.name = "wb_2", .id = WB_2,
.base = 0x65000, .len = 0x2c8,
.features = WB_SM8250_MASK,
- .format_list = wb2_formats,
- .num_formats = ARRAY_SIZE(wb2_formats),
+ .format_list = wb2_formats_rgb,
+ .num_formats = ARRAY_SIZE(wb2_formats_rgb),
.xin_id = 6,
.vbif_idx = VBIF_RT,
[PATCH v4 15/15] drm/msm/dpu: add cdm blocks to dpu snapshot
Now that CDM block support has been added to DPU lets also add its entry to the DPU snapshot to help debugging. Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index dc24fe4bb3b0..59647ad19906 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -947,6 +947,10 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k } } + if (cat->cdm) + msm_disp_snapshot_add_block(disp_state, cat->cdm->len, + dpu_kms->mmio + cat->cdm->base, cat->cdm->name); + pm_runtime_put_sync(_kms->pdev->dev); } -- 2.40.1
[PATCH v4 11/15] drm/msm/dpu: add an API to setup the CDM block for writeback
Add an API dpu_encoder_helper_phys_setup_cdm() which can be used by
the writeback encoder to setup the CDM block.
Currently, this is defined and used within the writeback's physical
encoder layer however, the function can be modified to be used to setup
the CDM block even for non-writeback interfaces.
Until those modifications are planned and made, keep it local to
writeback.
changes in v3:
- call bind_pingpong_blk() directly as disable() is dropped
- add dpu_csc10_rgb2yuv_601l to dpu_hw_util.h and use it
- fix kbot error on the function doc
- document that dpu_encoder_helper_phys_setup_cdm() doesn't handle
DPU_CHROMA_H1V2
changes in v2:
- add the RGB2YUV CSC matrix to dpu util as needed by CDM
- use dpu_hw_get_csc_cfg() to get and program CSC
- drop usage of setup_csc_data() and setup_cdwn() cdm ops
as they both have been merged into enable()
- drop reduntant hw_cdm and hw_pp checks
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202312102149.qmbcdsg2-...@intel.com/
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 ++
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 93 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 14 +++
3 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index e2934a6702d1..bdb89cbbcfb8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -14,8 +14,10 @@
#include "dpu_hw_intf.h"
#include "dpu_hw_wb.h"
#include "dpu_hw_pingpong.h"
+#include "dpu_hw_cdm.h"
#include "dpu_hw_ctl.h"
#include "dpu_hw_top.h"
+#include "dpu_hw_util.h"
#include "dpu_encoder.h"
#include "dpu_crtc.h"
@@ -150,6 +152,7 @@ enum dpu_intr_idx {
* @hw_pp: Hardware interface to the ping pong registers
* @hw_intf: Hardware interface to the intf registers
* @hw_wb: Hardware interface to the wb registers
+ * @hw_cdm:Hardware interface to the CDM registers
* @dpu_kms: Pointer to the dpu_kms top level
* @cached_mode: DRM mode cached at mode_set time, acted on in enable
* @enabled: Whether the encoder has enabled and running a mode
@@ -178,6 +181,7 @@ struct dpu_encoder_phys {
struct dpu_hw_pingpong *hw_pp;
struct dpu_hw_intf *hw_intf;
struct dpu_hw_wb *hw_wb;
+ struct dpu_hw_cdm *hw_cdm;
struct dpu_kms *dpu_kms;
struct drm_display_mode cached_mode;
enum dpu_enc_split_role split_role;
@@ -207,6 +211,7 @@ static inline int dpu_encoder_phys_inc_pending(struct
dpu_encoder_phys *phys)
* @wbirq_refcount: Reference count of writeback interrupt
* @wb_done_timeout_cnt: number of wb done irq timeout errors
* @wb_cfg: writeback block config to store fb related details
+ * @cdm_cfg: cdm block config needed to store writeback block's CDM
configuration
* @wb_conn: backpointer to writeback connector
* @wb_job: backpointer to current writeback job
* @dest: dpu buffer layout for current writeback output buffer
@@ -216,6 +221,7 @@ struct dpu_encoder_phys_wb {
atomic_t wbirq_refcount;
int wb_done_timeout_cnt;
struct dpu_hw_wb_cfg wb_cfg;
+ struct dpu_hw_cdm_cfg cdm_cfg;
struct drm_writeback_connector *wb_conn;
struct drm_writeback_job *wb_job;
struct dpu_hw_fmt_layout dest;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index f1b63dd9293c..3e5dbaa9c896 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -259,6 +259,96 @@ static void dpu_encoder_phys_wb_setup_ctl(struct
dpu_encoder_phys *phys_enc)
}
}
+/**
+ * dpu_encoder_helper_phys_setup_cdm - setup chroma down sampling block
+ * This API does not handle
DPU_CHROMA_H1V2.
+ * @phys_enc:Pointer to physical encoder
+ */
+static void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys
*phys_enc)
+{
+ struct dpu_hw_cdm *hw_cdm;
+ struct dpu_hw_cdm_cfg *cdm_cfg;
+ struct dpu_hw_pingpong *hw_pp;
+ struct dpu_encoder_phys_wb *wb_enc;
+ const struct msm_format *format;
+ const struct dpu_format *dpu_fmt;
+ struct drm_writeback_job *wb_job;
+ int ret;
+
+ if (!phys_enc)
+ return;
+
+ wb_enc = to_dpu_encoder_phys_wb(phys_enc);
+ cdm_cfg = _enc->cdm_cfg;
+ hw_pp = phys_enc->hw_pp;
+ hw_cdm = phys_enc->hw_cdm;
+ wb_job = wb_enc->wb_job;
+
+ format = msm_framebuffer_format(wb_enc->wb_job->fb);
+ dpu_fmt = dpu_get_dpu_format_ext(format->pixel_format,
wb_job->fb->modifier);
+
+ if (!hw_cdm)
+
[PATCH v4 12/15] drm/msm/dpu: plug-in the cdm related bits to writeback setup
To setup and enable CDM block for the writeback pipeline, lets
add the pieces together to set the active bits and the flush
bits for the CDM block.
changes in v2:
- passed the cdm idx to update_pending_flush_cdm()
(have retained the R-b as its a minor change)
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 3e5dbaa9c896..1a23cdd02038 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -214,6 +214,7 @@ static void dpu_encoder_phys_wb_setup_ctl(struct
dpu_encoder_phys *phys_enc)
{
struct dpu_hw_wb *hw_wb;
struct dpu_hw_ctl *ctl;
+ struct dpu_hw_cdm *hw_cdm;
if (!phys_enc) {
DPU_ERROR("invalid encoder\n");
@@ -222,6 +223,7 @@ static void dpu_encoder_phys_wb_setup_ctl(struct
dpu_encoder_phys *phys_enc)
hw_wb = phys_enc->hw_wb;
ctl = phys_enc->hw_ctl;
+ hw_cdm = phys_enc->hw_cdm;
if (test_bit(DPU_CTL_ACTIVE_CFG, >caps->features) &&
(phys_enc->hw_ctl &&
@@ -238,6 +240,9 @@ static void dpu_encoder_phys_wb_setup_ctl(struct
dpu_encoder_phys *phys_enc)
if (mode_3d && hw_pp && hw_pp->merge_3d)
intf_cfg.merge_3d = hw_pp->merge_3d->idx;
+ if (hw_cdm)
+ intf_cfg.cdm = hw_cdm->idx;
+
if (phys_enc->hw_pp->merge_3d &&
phys_enc->hw_pp->merge_3d->ops.setup_3d_mode)
phys_enc->hw_pp->merge_3d->ops.setup_3d_mode(phys_enc->hw_pp->merge_3d,
mode_3d);
@@ -418,6 +423,7 @@ static void _dpu_encoder_phys_wb_update_flush(struct
dpu_encoder_phys *phys_enc)
struct dpu_hw_wb *hw_wb;
struct dpu_hw_ctl *hw_ctl;
struct dpu_hw_pingpong *hw_pp;
+ struct dpu_hw_cdm *hw_cdm;
u32 pending_flush = 0;
if (!phys_enc)
@@ -426,6 +432,7 @@ static void _dpu_encoder_phys_wb_update_flush(struct
dpu_encoder_phys *phys_enc)
hw_wb = phys_enc->hw_wb;
hw_pp = phys_enc->hw_pp;
hw_ctl = phys_enc->hw_ctl;
+ hw_cdm = phys_enc->hw_cdm;
DPU_DEBUG("[wb:%d]\n", hw_wb->idx - WB_0);
@@ -441,6 +448,9 @@ static void _dpu_encoder_phys_wb_update_flush(struct
dpu_encoder_phys *phys_enc)
hw_ctl->ops.update_pending_flush_merge_3d(hw_ctl,
hw_pp->merge_3d->idx);
+ if (hw_cdm && hw_ctl->ops.update_pending_flush_cdm)
+ hw_ctl->ops.update_pending_flush_cdm(hw_ctl, hw_cdm->idx);
+
if (hw_ctl->ops.get_pending_flush)
pending_flush = hw_ctl->ops.get_pending_flush(hw_ctl);
--
2.40.1
[PATCH v4 07/15] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block
CDM block comes with its own set of registers and operations
which can be done. In-line with other hardware blocks, this
change adds the dpu_hw_cdm abstraction for the CDM block.
changes in v4:
- used FIELD_PREP() for dpu_hw_cdm_setup_cdwn() operations
- change to lowercase hex in dpu_hw_cdm_bind_pingpong_blk()
- move disable assignment inside else in dpu_hw_cdm_bind_pingpong_blk()
changes in v3:
- fix commit text from sub-blk to blk for CDM
- fix kbot issue for missing static for dpu_hw_cdm_enable()
- fix kbot issue for incorrect documentation style
- add more documentation for enums and struct in dpu_hw_cdm.h
- drop "enable" parameter from bind_pingpong_blk() as we can
just use PINGPONG_NONE for disable cases
- drop unnecessary bit operation for zero value of cdm_cfg
changes in v2:
- replace bit magic with relevant defines
- use drmm_kzalloc instead of kzalloc/free
- some formatting fixes
- inline _setup_cdm_ops()
- protect bind_pingpong_blk with core_rev check
- drop setup_csc_data() and setup_cdwn() ops as they
are merged into enable()
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202312101815.b3zh7pfy-...@intel.com/
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/Makefile| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 245
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 142
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
4 files changed, 389 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 49671364fdcf..b1173128b5b9 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_encoder_phys_wb.o \
disp/dpu1/dpu_formats.o \
disp/dpu1/dpu_hw_catalog.o \
+ disp/dpu1/dpu_hw_cdm.o \
disp/dpu1/dpu_hw_ctl.o \
disp/dpu1/dpu_hw_dsc.o \
disp/dpu1/dpu_hw_dsc_1_2.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
new file mode 100644
index ..050c6aa40720
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, The Linux Foundation. All rights reserved.
+ */
+
+#include
+
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_util.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hw_cdm.h"
+#include "dpu_kms.h"
+
+#define CDM_CSC_10_OPMODE 0x000
+#define CDM_CSC_10_BASE0x004
+
+#define CDM_CDWN2_OP_MODE 0x100
+#define CDM_CDWN2_CLAMP_OUT0x104
+#define CDM_CDWN2_PARAMS_3D_0 0x108
+#define CDM_CDWN2_PARAMS_3D_1 0x10C
+#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
+#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
+#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
+#define CDM_CDWN2_COEFF_OFFSITE_H_00x11C
+#define CDM_CDWN2_COEFF_OFFSITE_H_10x120
+#define CDM_CDWN2_COEFF_OFFSITE_H_20x124
+#define CDM_CDWN2_COEFF_COSITE_V 0x128
+#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
+#define CDM_CDWN2_OUT_SIZE 0x130
+
+#define CDM_HDMI_PACK_OP_MODE 0x200
+#define CDM_CSC_10_MATRIX_COEFF_0 0x004
+
+#define CDM_MUX0x224
+
+/* CDM CDWN2 sub-block bit definitions */
+#define CDM_CDWN2_OP_MODE_EN BIT(0)
+#define CDM_CDWN2_OP_MODE_ENABLE_HBIT(1)
+#define CDM_CDWN2_OP_MODE_ENABLE_VBIT(2)
+#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
+#define CDM_CDWN2_V_PIXEL_METHOD_MASK GENMASK(6, 5)
+#define CDM_CDWN2_H_PIXEL_METHOD_MASK GENMASK(4, 3)
+
+/* CDM CSC10 sub-block bit definitions */
+#define CDM_CSC10_OP_MODE_EN BIT(0)
+#define CDM_CSC10_OP_MODE_SRC_FMT_YUV BIT(1)
+#define CDM_CSC10_OP_MODE_DST_FMT_YUV BIT(2)
+
+/* CDM HDMI pack sub-block bit definitions */
+#define CDM_HDMI_PACK_OP_MODE_EN BIT(0)
+
+/*
+ * Horizontal coefficients for cosite chroma downscale
+ * s13 representation of coefficients
+ */
+static u32 cosite_h_coeff[] = {0x0016, 0x01cc, 0x019e};
+
+/*
+ * Horizontal coefficients for offsite chroma downscale
+ */
+static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046};
+
+/*
+ * Vertical coefficients for cosite chroma downscale
+ */
+static u32 cosite_v_coeff[] = {0x00080004};
+/*
+ * Vertical coefficients for offsite chroma downscale
+ */
+static u32 offsite_v_coeff[] = {0x00060002};
+
+static int dpu_hw_cdm_setup_cdwn(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg
*cfg)
+{
+ struct dpu_hw_blk_reg_map *c = >hw;
+
[PATCH v4 10/15] drm/msm/dpu: add CDM related logic to dpu_hw_ctl layer
CDM block will need its own logic to program the flush and active
bits in the dpu_hw_ctl layer.
Make necessary changes in dpu_hw_ctl to support CDM programming.
changes in v3:
- drop unused cdm_active as reported by kbot
- retained the R-b as its a trivial change
changes in v2:
- remove unused empty line
- pass in cdm_num to update_pending_flush_cdm()
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202312102047.s0i69pcs-...@intel.com/
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 33 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 12
2 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index e7b680a151d6..e76565c3e6a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -32,11 +32,13 @@
#define CTL_DSC_ACTIVE0x0E8
#define CTL_WB_ACTIVE 0x0EC
#define CTL_INTF_ACTIVE 0x0F4
+#define CTL_CDM_ACTIVE0x0F8
#define CTL_FETCH_PIPE_ACTIVE 0x0FC
#define CTL_MERGE_3D_FLUSH0x100
#define CTL_DSC_FLUSH0x104
#define CTL_WB_FLUSH 0x108
#define CTL_INTF_FLUSH0x110
+#define CTL_CDM_FLUSH0x114
#define CTL_INTF_MASTER 0x134
#define CTL_DSPP_n_FLUSH(n) ((0x13C) + ((n) * 4))
@@ -46,6 +48,7 @@
#define DPU_REG_RESET_TIMEOUT_US2000
#define MERGE_3D_IDX 23
#define DSC_IDX22
+#define CDM_IDX 26
#define INTF_IDX 31
#define WB_IDX 16
#define DSPP_IDX 29 /* From DPU hw rev 7.x.x */
@@ -107,6 +110,7 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct
dpu_hw_ctl *ctx)
ctx->pending_wb_flush_mask = 0;
ctx->pending_merge_3d_flush_mask = 0;
ctx->pending_dsc_flush_mask = 0;
+ ctx->pending_cdm_flush_mask = 0;
memset(ctx->pending_dspp_flush_mask, 0,
sizeof(ctx->pending_dspp_flush_mask));
@@ -151,6 +155,10 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct
dpu_hw_ctl *ctx)
DPU_REG_WRITE(>hw, CTL_DSC_FLUSH,
ctx->pending_dsc_flush_mask);
+ if (ctx->pending_flush_mask & BIT(CDM_IDX))
+ DPU_REG_WRITE(>hw, CTL_CDM_FLUSH,
+ ctx->pending_cdm_flush_mask);
+
DPU_REG_WRITE(>hw, CTL_FLUSH, ctx->pending_flush_mask);
}
@@ -282,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_wb(struct
dpu_hw_ctl *ctx,
}
}
+static void dpu_hw_ctl_update_pending_flush_cdm(struct dpu_hw_ctl *ctx, enum
dpu_cdm cdm_num)
+{
+ /* update pending flush only if CDM_0 is flushed */
+ if (cdm_num == CDM_0)
+ ctx->pending_flush_mask |= BIT(CDM_IDX);
+}
+
static void dpu_hw_ctl_update_pending_flush_wb_v1(struct dpu_hw_ctl *ctx,
enum dpu_wb wb)
{
@@ -310,6 +325,12 @@ static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct
dpu_hw_ctl *ctx,
ctx->pending_flush_mask |= BIT(DSC_IDX);
}
+static void dpu_hw_ctl_update_pending_flush_cdm_v1(struct dpu_hw_ctl *ctx,
enum dpu_cdm cdm_num)
+{
+ ctx->pending_cdm_flush_mask |= BIT(cdm_num - CDM_0);
+ ctx->pending_flush_mask |= BIT(CDM_IDX);
+}
+
static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
enum dpu_dspp dspp, u32 dspp_sub_blk)
{
@@ -543,6 +564,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if (cfg->dsc)
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
+
+ if (cfg->cdm)
+ DPU_REG_WRITE(c, CTL_CDM_ACTIVE, cfg->cdm);
}
static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
@@ -586,6 +610,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl
*ctx,
u32 wb_active = 0;
u32 merge3d_active = 0;
u32 dsc_active;
+ u32 cdm_active;
/*
* This API resets each portion of the CTL path namely,
@@ -621,6 +646,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl
*ctx,
dsc_active &= ~cfg->dsc;
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
}
+
+ if (cfg->cdm) {
+ cdm_active = DPU_REG_READ(c, CTL_CDM_ACTIVE);
+ cdm_active &= ~cfg->cdm;
+ DPU_REG_WRITE(c, CTL_CDM_ACTIVE, cdm_active);
+ }
}
static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
@@ -654,12 +685,14 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
ops->update_pending_flush_wb =
dpu_hw_ctl_update_pending_flush_wb_v1;
ops->update_pending_flush_dsc =
dpu_hw_ctl_update_pending_flush_dsc_v1;
+ ops->update_pending_flush_cdm =
[PATCH v4 08/15] drm/msm/dpu: add cdm blocks to RM
Add the RM APIs necessary to initialize and allocate CDM
blocks to be used by the rest of the DPU pipeline.
changes in v2:
- treat cdm_init() failure as fatal
- fixed the commit text
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 13 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 0bb28cf4a6cb..7ed476b96304 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -8,6 +8,7 @@
#include "dpu_kms.h"
#include "dpu_hw_lm.h"
#include "dpu_hw_ctl.h"
+#include "dpu_hw_cdm.h"
#include "dpu_hw_pingpong.h"
#include "dpu_hw_sspp.h"
#include "dpu_hw_intf.h"
@@ -176,6 +177,18 @@ int dpu_rm_init(struct drm_device *dev,
rm->hw_sspp[sspp->id - SSPP_NONE] = hw;
}
+ if (cat->cdm) {
+ struct dpu_hw_cdm *hw;
+
+ hw = dpu_hw_cdm_init(dev, cat->cdm, mmio, cat->mdss_ver);
+ if (IS_ERR(hw)) {
+ rc = PTR_ERR(hw);
+ DPU_ERROR("failed cdm object creation: err %d\n", rc);
+ goto fail;
+ }
+ rm->cdm_blk = >base;
+ }
+
return 0;
fail:
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 36752d837be4..e3f83ebc656b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -22,6 +22,7 @@ struct dpu_global_state;
* @hw_wb: array of wb hardware resources
* @dspp_blks: array of dspp hardware resources
* @hw_sspp: array of sspp hardware resources
+ * @cdm_blk: cdm hardware resource
*/
struct dpu_rm {
struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0];
@@ -33,6 +34,7 @@ struct dpu_rm {
struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE];
+ struct dpu_hw_blk *cdm_blk;
};
/**
--
2.40.1
[PATCH v4 06/15] drm/msm/dpu: add cdm blocks to sm8250 dpu_hw_catalog
Add CDM blocks to the sm8250 dpu_hw_catalog to support
YUV format output from writeback block.
changes in v2:
- re-use the cdm definition from sc7280
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
index 2359c16e9206..58b0f50518c8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
@@ -384,6 +384,7 @@ const struct dpu_mdss_cfg dpu_sm8250_cfg = {
.mdss_ver = _mdss_ver,
.caps = _dpu_caps,
.mdp = _mdp,
+ .cdm = _cdm,
.ctl_count = ARRAY_SIZE(sm8250_ctl),
.ctl = sm8250_ctl,
.sspp_count = ARRAY_SIZE(sm8250_sspp),
--
2.40.1
[PATCH v4 09/15] drm/msm/dpu: add support to allocate CDM from RM
Even though there is usually only one CDM block, it can be
used by either HDMI, DisplayPort OR Writeback interfaces.
Hence its allocation needs to be tracked properly by the
resource manager to ensure appropriate availability of the
block.
changes in v2:
- move needs_cdm to topology struct
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 38 +++--
drivers/gpu/drm/msm/msm_drv.h | 2 ++
4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 9db4cf61bd29..5df545904057 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -98,6 +98,7 @@ enum dpu_hw_blk_type {
DPU_HW_BLK_DSPP,
DPU_HW_BLK_MERGE_3D,
DPU_HW_BLK_DSC,
+ DPU_HW_BLK_CDM,
DPU_HW_BLK_MAX,
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index df6271017b80..a0cd36e45a01 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -135,6 +135,7 @@ struct dpu_global_state {
uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
+ uint32_t cdm_to_enc_id;
};
struct dpu_global_state
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 7ed476b96304..b58a9c2ae326 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -435,6 +435,26 @@ static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
return 0;
}
+static int _dpu_rm_reserve_cdm(struct dpu_rm *rm,
+ struct dpu_global_state *global_state,
+ struct drm_encoder *enc)
+{
+ /* try allocating only one CDM block */
+ if (!rm->cdm_blk) {
+ DPU_ERROR("CDM block does not exist\n");
+ return -EIO;
+ }
+
+ if (global_state->cdm_to_enc_id) {
+ DPU_ERROR("CDM_0 is already allocated\n");
+ return -EIO;
+ }
+
+ global_state->cdm_to_enc_id = enc->base.id;
+
+ return 0;
+}
+
static int _dpu_rm_make_reservation(
struct dpu_rm *rm,
struct dpu_global_state *global_state,
@@ -460,6 +480,14 @@ static int _dpu_rm_make_reservation(
if (ret)
return ret;
+ if (reqs->topology.needs_cdm) {
+ ret = _dpu_rm_reserve_cdm(rm, global_state, enc);
+ if (ret) {
+ DPU_ERROR("unable to find CDM blk\n");
+ return ret;
+ }
+ }
+
return ret;
}
@@ -470,9 +498,9 @@ static int _dpu_rm_populate_requirements(
{
reqs->topology = req_topology;
- DRM_DEBUG_KMS("num_lm: %d num_dsc: %d num_intf: %d\n",
+ DRM_DEBUG_KMS("num_lm: %d num_dsc: %d num_intf: %d cdm: %d\n",
reqs->topology.num_lm, reqs->topology.num_dsc,
- reqs->topology.num_intf);
+ reqs->topology.num_intf, reqs->topology.needs_cdm);
return 0;
}
@@ -501,6 +529,7 @@ void dpu_rm_release(struct dpu_global_state *global_state,
ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
_dpu_rm_clear_mapping(global_state->dspp_to_enc_id,
ARRAY_SIZE(global_state->dspp_to_enc_id), enc->base.id);
+ _dpu_rm_clear_mapping(_state->cdm_to_enc_id, 1, enc->base.id);
}
int dpu_rm_reserve(
@@ -574,6 +603,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
hw_to_enc_id = global_state->dsc_to_enc_id;
max_blks = ARRAY_SIZE(rm->dsc_blks);
break;
+ case DPU_HW_BLK_CDM:
+ hw_blks = >cdm_blk;
+ hw_to_enc_id = _state->cdm_to_enc_id;
+ max_blks = 1;
+ break;
default:
DPU_ERROR("blk type %d not managed by rm\n", type);
return 0;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index c0446fa66b98..16a7cbc0b7dd 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -90,12 +90,14 @@ enum msm_event_wait {
* @num_intf: number of interfaces the panel is mounted on
* @num_dspp: number of dspp blocks used
* @num_dsc: number of Display Stream Compression (DSC) blocks used
+ * @needs_cdm:indicates whether cdm block is needed for this display
topology
*/
struct msm_display_topology {
u32 num_lm;
u32 num_intf;
u32 num_dspp;
u32 num_dsc;
+ bool needs_cdm;
};
/* Commit/Event thread specific structure */
--
2.40.1
[PATCH v4 05/15] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
Add CDM blocks to the sc7280 dpu_hw_catalog to support
YUV format output from writeback block.
changes in v3:
- change the comment from sub-blk to clk for CDM
changes in v2:
- remove explicit zero assignment for features
- move sc7280_cdm to dpu_hw_catalog from the sc7280
catalog file as its definition can be re-used
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +
4 files changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
index 209675de6742..19c2b7454796 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
@@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
.mdss_ver = _mdss_ver,
.caps = _dpu_caps,
.mdp = _mdp,
+ .cdm = _cdm,
.ctl_count = ARRAY_SIZE(sc7280_ctl),
.ctl = sc7280_ctl,
.sspp_count = ARRAY_SIZE(sc7280_sspp),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d52aae54bbd5..b304bebedb84 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
.ctl = {.name = "ctl", .base = 0xF80, .len = 0x10},
};
+/*
+ * CDM block config
+ */
+static const struct dpu_cdm_cfg sc7280_cdm = {
+ .name = "cdm_0",
+ .id = CDM_0,
+ .len = 0x228,
+ .base = 0x79200,
+};
+
/*
* VBIF sub blocks config
*/
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index e3c0d007481b..ba82ef4560a6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -682,6 +682,17 @@ struct dpu_vbif_cfg {
u32 memtype[MAX_XIN_COUNT];
};
+/**
+ * struct dpu_cdm_cfg - information of chroma down blocks
+ * @name string name for debug purposes
+ * @id enum identifying this block
+ * @base register offset of this block
+ * @features bit mask identifying sub-blocks/features
+ */
+struct dpu_cdm_cfg {
+ DPU_HW_BLK_INFO;
+};
+
/**
* Define CDP use cases
* @DPU_PERF_CDP_UDAGE_RT: real-time use cases
@@ -805,6 +816,8 @@ struct dpu_mdss_cfg {
u32 wb_count;
const struct dpu_wb_cfg *wb;
+ const struct dpu_cdm_cfg *cdm;
+
u32 ad_count;
u32 dspp_count;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index a6702b2bfc68..f319c8232ea5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -185,6 +185,11 @@ enum dpu_dsc {
DSC_MAX
};
+enum dpu_cdm {
+ CDM_0 = 1,
+ CDM_MAX
+};
+
enum dpu_pingpong {
PINGPONG_NONE,
PINGPONG_0,
--
2.40.1
[PATCH v4 04/15] drm/msm/dpu: move csc matrices to dpu_hw_util
Since the type and usage of CSC matrices is spanning across DPU
lets introduce a helper to the dpu_hw_util to return the CSC
corresponding to the request type. This will help to add more
supported CSC types such as the RGB to YUV one which is used in
the case of CDM.
changes in v3:
- drop the extra wrapper and export the matrices directly
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 30
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
index fe083b2e5696..aa50005042d1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -19,6 +19,36 @@
#define MISR_CTRL_STATUS_CLEAR BIT(10)
#define MISR_CTRL_FREE_RUN_MASK BIT(31)
+static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
+ {
+ /* S15.16 format */
+ 0x00012A00, 0x, 0x00019880,
+ 0x00012A00, 0x9B80, 0x3000,
+ 0x00012A00, 0x00020480, 0x,
+ },
+ /* signed bias */
+ { 0xfff0, 0xff80, 0xff80,},
+ { 0x0, 0x0, 0x0,},
+ /* unsigned clamp */
+ { 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,},
+ { 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,},
+};
+
+static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
+ {
+ /* S15.16 format */
+ 0x00012A00, 0x, 0x00019880,
+ 0x00012A00, 0x9B80, 0x3000,
+ 0x00012A00, 0x00020480, 0x,
+ },
+ /* signed bias */
+ { 0xffc0, 0xfe00, 0xfe00,},
+ { 0x0, 0x0, 0x0,},
+ /* unsigned clamp */
+ { 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,},
+ { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
+};
+
/*
* This is the common struct maintained by each sub block
* for mapping the register offsets in this block to the
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 3235ab132540..ff975ad51145 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -21,6 +21,7 @@
#include "dpu_kms.h"
#include "dpu_formats.h"
#include "dpu_hw_sspp.h"
+#include "dpu_hw_util.h"
#include "dpu_trace.h"
#include "dpu_crtc.h"
#include "dpu_vbif.h"
@@ -508,36 +509,6 @@ static void _dpu_plane_setup_pixel_ext(struct
dpu_hw_scaler3_cfg *scale_cfg,
}
}
-static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
- {
- /* S15.16 format */
- 0x00012A00, 0x, 0x00019880,
- 0x00012A00, 0x9B80, 0x3000,
- 0x00012A00, 0x00020480, 0x,
- },
- /* signed bias */
- { 0xfff0, 0xff80, 0xff80,},
- { 0x0, 0x0, 0x0,},
- /* unsigned clamp */
- { 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,},
- { 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,},
-};
-
-static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
- {
- /* S15.16 format */
- 0x00012A00, 0x, 0x00019880,
- 0x00012A00, 0x9B80, 0x3000,
- 0x00012A00, 0x00020480, 0x,
- },
- /* signed bias */
- { 0xffc0, 0xfe00, 0xfe00,},
- { 0x0, 0x0, 0x0,},
- /* unsigned clamp */
- { 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,},
- { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
-};
-
static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_sw_pipe *pipe,
const struct dpu_format
*fmt)
{
--
2.40.1
[PATCH v4 02/15] drm/msm/dpu: rename dpu_encoder_phys_wb_setup_cdp to match its functionality
dpu_encoder_phys_wb_setup_cdp() is not programming the chroma down
prefetch block. Its setting up the display ctl path for writeback.
Hence rename it to dpu_encoder_phys_wb_setup_ctl() to match what its
actually doing.
Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
writeback")
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 4953d87affa1..f1b63dd9293c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -207,10 +207,10 @@ static void dpu_encoder_phys_wb_setup_fb(struct
dpu_encoder_phys *phys_enc,
}
/**
- * dpu_encoder_phys_wb_setup_cdp - setup chroma down prefetch block
+ * dpu_encoder_phys_wb_setup_ctl - setup wb pipeline for ctl path
* @phys_enc:Pointer to physical encoder
*/
-static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys *phys_enc)
+static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc)
{
struct dpu_hw_wb *hw_wb;
struct dpu_hw_ctl *ctl;
@@ -382,7 +382,7 @@ static void dpu_encoder_phys_wb_setup(
dpu_encoder_phys_wb_setup_fb(phys_enc, fb);
- dpu_encoder_phys_wb_setup_cdp(phys_enc);
+ dpu_encoder_phys_wb_setup_ctl(phys_enc);
}
--
2.40.1
[PATCH v4 03/15] drm/msm/dpu: fix writeback programming for YUV cases
For YUV cases, setting the required format bits was missed out in the register programming. Lets fix it now in preparation of adding YUV formats support for writeback. changes in v2: - dropped the fixes tag as its not a fix but adding new functionality Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c index ed0e80616129..e75995f7fcea 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c @@ -89,6 +89,9 @@ static void dpu_hw_wb_setup_format(struct dpu_hw_wb *ctx, dst_format |= BIT(14); /* DST_ALPHA_X */ } + if (DPU_FORMAT_IS_YUV(fmt)) + dst_format |= BIT(15); + pattern = (fmt->element[3] << 24) | (fmt->element[2] << 16) | (fmt->element[1] << 8) | -- 2.40.1
[PATCH v4 01/15] drm/msm/dpu: add formats check for writeback encoder
In preparation for adding more formats to dpu writeback add
format validation to it to fail any unsupported formats.
changes in v4:
- change the failure message of the API
drm_atomic_helper_check_wb_connector_state() to a generic
one in case it checks more errors later and moreoever it
already has debug message to indicate its failure
- change the corresponding DPU_ERROR to DPU_DEBUG in-line with
other atomic_check failure messages
changes in v3:
- rebase on top of msm-next
- replace drm_atomic_helper_check_wb_encoder_state() with
drm_atomic_helper_check_wb_connector_state() due to the
rebase
changes in v2:
- correct some grammar in the commit text
Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
writeback")
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index bb94909caa25..4953d87affa1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check(
{
struct drm_framebuffer *fb;
const struct drm_display_mode *mode = _state->mode;
+ int ret;
DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
phys_enc->hw_wb->idx, mode->name, mode->hdisplay,
mode->vdisplay);
@@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check(
return -EINVAL;
}
+ ret = drm_atomic_helper_check_wb_connector_state(conn_state->connector,
conn_state->state);
+ if (ret < 0) {
+ DPU_DEBUG("wb check connector state failed ret = %d\n", ret);
+ return ret;
+ }
+
return 0;
}
--
2.40.1
[PATCH v4 00/15] Add CDM support for MSM writeback
Chroma Down Sampling (CDM) block is a hardware block in the DPU pipeline which among other things has a CSC block that can convert RGB input from the DPU to YUV data. This block can be used with either HDMI, DP or writeback interface. In this series, lets first add the support for CDM block to be used with writeback and then follow-up with support for other interfaces such as DP. This was validated by adding support to pass custom output format to the IGT's kms_writeback test-case, specifically only for the output dump test-case [1]. The usage for this is: ./kms_writeback -d -f So for NV12, this can be verified with the below command: ./kms_writeback -d -f NV12 [1] : https://patchwork.freedesktop.org/series/122125/ changes in v4: - change the debug message when drm_atomic_helper_check_wb_connector_state() returns an error - used FIELD_PREP() for dpu_hw_cdm_setup_cdwn() operations - change to lowercase hex in dpu_hw_cdm_bind_pingpong_blk() - move disable assignment inside else in dpu_hw_cdm_bind_pingpong_blk() changes in v3: - rebase on top of msm-next - drop the extra wrapper and export the CSC matrices directly - fixes in commit text as requested - fixes for kbot errors as reported - drop "enable" parameter from bind_pingpong_blk() as we can just use PINGPONG_NONE for disable cases - squash cdm changes in encoder cleanup to the change which allocates cdm changes in v2: - rebased on top of current msm-next-lumag - fix commit text of some of the patches - move csc matrices to dpu_hw_util as they span across DPU - move cdm blk define to dpu_hw_catalog as its common across chipsets - remove bit magic in dpu_hw_cdm with relevant defines - use drmm_kzalloc instead of kzalloc/free - protect bind_pingpong_blk with core_rev check - drop setup_csc_data() and setup_cdwn() ops as they are merged into enable() - protect bind_pingpong_blk with core_rev check - drop setup_csc_data() and setup_cdwn() ops as they are merged into enable() - move needs_cdm to topology struct - call update_pending_flush_cdm even when bind_pingpong_blk is not present - drop usage of setup_csc_data() and setup_cdwn() cdm ops as they both have been merged into enable() - drop reduntant hw_cdm and hw_pp checks - drop fb related checks from dpu_encoder::atomic_mode_set() - introduce separate wb2_format arrays for rgb and yuv Abhinav Kumar (15): drm/msm/dpu: add formats check for writeback encoder drm/msm/dpu: rename dpu_encoder_phys_wb_setup_cdp to match its functionality drm/msm/dpu: fix writeback programming for YUV cases drm/msm/dpu: move csc matrices to dpu_hw_util drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog drm/msm/dpu: add cdm blocks to sm8250 dpu_hw_catalog drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block drm/msm/dpu: add cdm blocks to RM drm/msm/dpu: add support to allocate CDM from RM drm/msm/dpu: add CDM related logic to dpu_hw_ctl layer drm/msm/dpu: add an API to setup the CDM block for writeback drm/msm/dpu: plug-in the cdm related bits to writeback setup drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output drm/msm/dpu: introduce separate wb2_format arrays for rgb and yuv drm/msm/dpu: add cdm blocks to dpu snapshot drivers/gpu/drm/msm/Makefile | 1 + .../msm/disp/dpu1/catalog/dpu_10_0_sm8650.h | 4 +- .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h| 5 +- .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h| 4 +- .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h| 5 +- .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 +++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 + .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 114 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 47 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c| 245 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h| 142 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 33 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h| 12 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 7 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 44 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 51 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h| 2 + drivers/gpu/drm/msm/msm_drv.h | 2 + 24 files changed, 771 insertions(+), 46 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c create mode
Re: (subset) [PATCH v1 1/3] dt-bindings: drm: rockchip: convert inno_hdmi-rockchip.txt to yaml
On Mon, 4 Dec 2023 18:39:03 +0100, Johan Jonker wrote: > Convert inno_hdmi-rockchip.txt to yaml. > > Applied, thanks! [2/3] ARM: dts: rockchip: rk3036: fix hdmi ports node commit: 27ded76ef0fcfcf939914532aae575cf23c221b4 [3/3] ARM: dts: rockchip: rk3036-kylin: add hdmi-connector node commit: 569b26af7919c15a8ce231b4fae29cfbd811f144 And changed the connector-type to "a" :-) Best regards, -- Heiko Stuebner
Re: [PATCH] dt-bindings: panel-simple-dsi: move LG 5" HD TFT LCD panel into DSI yaml
Good catch, thank you! Sent v2.
David
On 12/12/2023 20:41, Jessica Zhang wrote:
On 12/12/2023 9:53 AM, David Heidelberg wrote:
Originally was in the panel-simple, but belongs to panel-simple-dsi.
See arch/arm/boot/dts/nvidia/tegra114-roth.dts for more details.
Fixes:
```
arch/arm/boot/dts/tegra114-roth.dt.yaml: panel@0: 'reg' does not
match any of the regexes: 'pinctrl-[0-9]+'
From schema:
Documentation/devicetree/bindings/display/panel/panel-simple.yaml
```
Hi David,
Would "Fixes: 310abcea76e9 ("dt-bindings: display: convert simple lg
panels to DT Schema")" be appropriate here?
Thanks,
Jessica Zhang
Signed-off-by: David Heidelberg
---
.../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
.../devicetree/bindings/display/panel/panel-simple.yaml | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 73674baea75d..f9160d7bac3c 100644
---
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++
b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -42,6 +42,8 @@ properties:
- lg,acx467akm-7
# LG Corporation 7" WXGA TFT LCD panel
- lg,ld070wx3-sl01
+ # LG Corporation 5" HD TFT LCD panel
+ - lg,lh500wx1-sd03
# One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
- osddisplays,osd101t2587-53ts
# Panasonic 10" WUXGA TFT LCD panel
diff --git
a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 2021aa82871a..634a10c6f2dd 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -212,8 +212,6 @@ properties:
- lemaker,bl035-rgb-002
# LG 7" (800x480 pixels) TFT LCD panel
- lg,lb070wv8
- # LG Corporation 5" HD TFT LCD panel
- - lg,lh500wx1-sd03
# LG LP079QX1-SP0V 7.9" (1536x2048 pixels) TFT LCD panel
- lg,lp079qx1-sp0v
# LG 9.7" (2048x1536 pixels) TFT LCD panel
--
2.43.0
--
David Heidelberg
[PATCH v2] dt-bindings: panel-simple-dsi: move LG 5" HD TFT LCD panel into DSI yaml
Originally was in the panel-simple, but belongs to panel-simple-dsi.
See arch/arm/boot/dts/nvidia/tegra114-roth.dts for more details.
Resolves the following warning:
```
arch/arm/boot/dts/tegra114-roth.dt.yaml: panel@0: 'reg' does not match any of
the regexes: 'pinctrl-[0-9]+'
From schema:
Documentation/devicetree/bindings/display/panel/panel-simple.yaml
```
Fixes: 310abcea76e9 ("dt-bindings: display: convert simple lg panels to DT
Schema")
Signed-off-by: David Heidelberg
---
v2: added Fixes tag (thx to Jessica)
.../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
.../devicetree/bindings/display/panel/panel-simple.yaml | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 73674baea75d..f9160d7bac3c 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -42,6 +42,8 @@ properties:
- lg,acx467akm-7
# LG Corporation 7" WXGA TFT LCD panel
- lg,ld070wx3-sl01
+# LG Corporation 5" HD TFT LCD panel
+ - lg,lh500wx1-sd03
# One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
- osddisplays,osd101t2587-53ts
# Panasonic 10" WUXGA TFT LCD panel
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 2021aa82871a..634a10c6f2dd 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -212,8 +212,6 @@ properties:
- lemaker,bl035-rgb-002
# LG 7" (800x480 pixels) TFT LCD panel
- lg,lb070wv8
-# LG Corporation 5" HD TFT LCD panel
- - lg,lh500wx1-sd03
# LG LP079QX1-SP0V 7.9" (1536x2048 pixels) TFT LCD panel
- lg,lp079qx1-sp0v
# LG 9.7" (2048x1536 pixels) TFT LCD panel
--
2.43.0
Re: [PATCH 00/31] Fix and improve Rockchip RK3128 support
Hi Alex, Am Dienstag, 29. August 2023, 19:16:16 CET schrieb Alex Bee: > this series fixes some issues I found when testing my "new" RK3128 board > with the mainline kernel and adds some core functionality like SMP bringup, > usb and networking. before I discard this thread, is everything from here migrated to separate serieses or are there patches in this big pile that are still relevant on their own, that haven't been applied? [I lost track of what did go where ;-) ] Thanks Heiko
[PATCH] fbdev/simplefb: change loglevel when the power domains cannot be parsed
When the power domains cannot be parsed, the message is incorrectly
logged as an info message. Let's change this to an error since an error
is returned.
Fixes: 92a511a568e4 ("fbdev/simplefb: Add support for generic power-domains")
Signed-off-by: Brian Masney
---
drivers/video/fbdev/simplefb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 6f58ee276ad1..028a56525047 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -470,7 +470,7 @@ static int simplefb_attach_genpds(struct simplefb_par *par,
if (err == -ENOENT)
return 0;
- dev_info(dev, "failed to parse power-domains: %d\n", err);
+ dev_err(dev, "failed to parse power-domains: %d\n", err);
return err;
}
--
2.43.0
Re: [PATCH] dt-bindings: panel-simple-dsi: move LG 5" HD TFT LCD panel into DSI yaml
On 12/12/2023 9:53 AM, David Heidelberg wrote:
Originally was in the panel-simple, but belongs to panel-simple-dsi.
See arch/arm/boot/dts/nvidia/tegra114-roth.dts for more details.
Fixes:
```
arch/arm/boot/dts/tegra114-roth.dt.yaml: panel@0: 'reg' does not match any of
the regexes: 'pinctrl-[0-9]+'
From schema:
Documentation/devicetree/bindings/display/panel/panel-simple.yaml
```
Hi David,
Would "Fixes: 310abcea76e9 ("dt-bindings: display: convert simple lg
panels to DT Schema")" be appropriate here?
Thanks,
Jessica Zhang
Signed-off-by: David Heidelberg
---
.../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
.../devicetree/bindings/display/panel/panel-simple.yaml | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 73674baea75d..f9160d7bac3c 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -42,6 +42,8 @@ properties:
- lg,acx467akm-7
# LG Corporation 7" WXGA TFT LCD panel
- lg,ld070wx3-sl01
+# LG Corporation 5" HD TFT LCD panel
+ - lg,lh500wx1-sd03
# One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
- osddisplays,osd101t2587-53ts
# Panasonic 10" WUXGA TFT LCD panel
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 2021aa82871a..634a10c6f2dd 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -212,8 +212,6 @@ properties:
- lemaker,bl035-rgb-002
# LG 7" (800x480 pixels) TFT LCD panel
- lg,lb070wv8
-# LG Corporation 5" HD TFT LCD panel
- - lg,lh500wx1-sd03
# LG LP079QX1-SP0V 7.9" (1536x2048 pixels) TFT LCD panel
- lg,lp079qx1-sp0v
# LG 9.7" (2048x1536 pixels) TFT LCD panel
--
2.43.0
Re: [net-next v1 15/16] net: add devmem TCP documentation
On Thu, Dec 07, 2023 at 04:52:46PM -0800, Mina Almasry wrote: > Signed-off-by: Mina Almasry > --- > Documentation/networking/devmem.rst | 270 > 1 file changed, 270 insertions(+) > create mode 100644 Documentation/networking/devmem.rst > > diff --git a/Documentation/networking/devmem.rst > b/Documentation/networking/devmem.rst > new file mode 100644 > index ..ed0d9c88b708 > --- /dev/null > +++ b/Documentation/networking/devmem.rst > @@ -0,0 +1,270 @@ Hi Mina, Please consider adding an SPDX header here. And please consider adding devmem to index.rxt, as make htmldocs currently warns: .../devmem.rst: WARNING: document isn't included in any toctree
Re: [net-next v1 14/16] net: add SO_DEVMEM_DONTNEED setsockopt to release RX frags
On Thu, Dec 07, 2023 at 04:52:45PM -0800, Mina Almasry wrote:
> Add an interface for the user to notify the kernel that it is done
> reading the devmem dmabuf frags returned as cmsg. The kernel will
> drop the reference on the frags to make them available for re-use.
>
> Signed-off-by: Willem de Bruijn
> Signed-off-by: Kaiyuan Zhang
> Signed-off-by: Mina Almasry
...
> diff --git a/net/core/sock.c b/net/core/sock.c
> index fef349dd72fa..521bdc4ff260 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1051,6 +1051,41 @@ static int sock_reserve_memory(struct sock *sk, int
> bytes)
> return 0;
> }
>
> +static noinline_for_stack int
> +sock_devmem_dontneed(struct sock *sk, sockptr_t optval, unsigned int optlen)
> +{
> + struct dmabuf_token tokens[128];
Hi Mina,
I am guessing it is mostly due to the line above,
but on x86 32bit builds I see:
warning: the frame size of 1048 bytes is larger than 1024 bytes
[-Wframe-larger-than
> + unsigned int num_tokens, i, j;
> + int ret;
> +
> + if (sk->sk_type != SOCK_STREAM || sk->sk_protocol != IPPROTO_TCP)
> + return -EBADF;
> +
> + if (optlen % sizeof(struct dmabuf_token) || optlen > sizeof(tokens))
> + return -EINVAL;
> +
> + num_tokens = optlen / sizeof(struct dmabuf_token);
> + if (copy_from_sockptr(tokens, optval, optlen))
> + return -EFAULT;
> +
> + ret = 0;
> + for (i = 0; i < num_tokens; i++) {
> + for (j = 0; j < tokens[i].token_count; j++) {
> + struct page *page = xa_erase(>sk_user_pages,
> + tokens[i].token_start + j);
> +
> + if (page) {
> + if (WARN_ON_ONCE(!napi_pp_put_page(page,
> +false)))
> + page_pool_page_put_many(page, 1);
> + ret++;
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
> void sockopt_lock_sock(struct sock *sk)
> {
> /* When current->bpf_ctx is set, the setsockopt is called from
...
Re: [PATCH] ASoC: hdmi-codec: drop drm/drm_edid.h include
Hi Jani, kernel test robot noticed the following build errors: [auto build test ERROR on tiwai-sound/for-next] [also build test ERROR on tiwai-sound/for-linus linus/master v6.7-rc5 next-20231212] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/ASoC-hdmi-codec-drop-drm-drm_edid-h-include/20231212-223200 base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next patch link: https://lore.kernel.org/r/20231212143038.3828691-1-jani.nikula%40intel.com patch subject: [PATCH] ASoC: hdmi-codec: drop drm/drm_edid.h include config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20231213/202312130235.qg8owkk0-...@intel.com/config) compiler: sparc64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312130235.qg8owkk0-...@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/202312130235.qg8owkk0-...@intel.com/ All error/warnings (new ones prefixed by >>): drivers/gpu/drm/msm/dp/dp_display.c: In function 'dp_display_process_hpd_high': >> drivers/gpu/drm/msm/dp/dp_display.c:406:31: error: implicit declaration of >> function 'drm_detect_monitor_audio' [-Werror=implicit-function-declaration] 406 | dp->audio_supported = drm_detect_monitor_audio(edid); | ^~~~ cc1: some warnings being treated as errors -- drivers/gpu/drm/vc4/vc4_hdmi.c: In function 'vc4_hdmi_is_full_range': >> drivers/gpu/drm/vc4/vc4_hdmi.c:168:17: error: implicit declaration of >> function 'drm_default_rgb_quant_range' >> [-Werror=implicit-function-declaration] 168 | drm_default_rgb_quant_range(mode) == HDMI_QUANTIZATION_RANGE_FULL; | ^~~ drivers/gpu/drm/vc4/vc4_hdmi.c: In function 'vc4_hdmi_handle_hotplug': >> drivers/gpu/drm/vc4/vc4_hdmi.c:437:16: error: implicit declaration of >> function 'drm_get_edid'; did you mean 'drm_gem_evict'? >> [-Werror=implicit-function-declaration] 437 | edid = drm_get_edid(connector, vc4_hdmi->ddc); |^~~~ |drm_gem_evict >> drivers/gpu/drm/vc4/vc4_hdmi.c:437:14: warning: assignment to 'struct edid >> *' from 'int' makes pointer from integer without a cast [-Wint-conversion] 437 | edid = drm_get_edid(connector, vc4_hdmi->ddc); | ^ drivers/gpu/drm/vc4/vc4_hdmi.c: In function 'vc4_hdmi_connector_get_modes': drivers/gpu/drm/vc4/vc4_hdmi.c:508:14: warning: assignment to 'struct edid *' from 'int' makes pointer from integer without a cast [-Wint-conversion] 508 | edid = drm_get_edid(connector, vc4_hdmi->ddc); | ^ >> drivers/gpu/drm/vc4/vc4_hdmi.c:514:15: error: implicit declaration of >> function 'drm_add_edid_modes' [-Werror=implicit-function-declaration] 514 | ret = drm_add_edid_modes(connector, edid); | ^~ drivers/gpu/drm/vc4/vc4_hdmi.c: In function 'vc4_hdmi_set_avi_infoframe': >> drivers/gpu/drm/vc4/vc4_hdmi.c:910:15: error: implicit declaration of >> function 'drm_hdmi_avi_infoframe_from_display_mode'; did you mean >> 'drm_hdmi_avi_infoframe_content_type'? >> [-Werror=implicit-function-declaration] 910 | ret = drm_hdmi_avi_infoframe_from_display_mode(, | ^~~~ | drm_hdmi_avi_infoframe_content_type >> drivers/gpu/drm/vc4/vc4_hdmi.c:917:9: error: implicit declaration of >> function 'drm_hdmi_avi_infoframe_quant_range'; did you mean >> 'drm_hdmi_avi_infoframe_content_type'? >> [-Werror=implicit-function-declaration] 917 | drm_hdmi_avi_infoframe_quant_range(, | ^~ | drm_hdmi_avi_infoframe_content_type drivers/gpu/drm/vc4/vc4_hdmi.c: In function 'vc4_hdmi_sink_supports_format_bpc': >> drivers/gpu/drm/vc4/vc4_hdmi.c:1930:18: error: implicit declaration of >> function 'drm_match_cea_mode' [-Werror=implicit-function-declaration] 1930 | u8 vic = drm_match_cea_mode(mode); | ^~ >> drivers/gpu/drm/vc4/vc4_hdmi.c:1950:70: error: 'DRM_EDID_HDMI_DC_30' >> undeclared (first use in this function) 1950 | if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_mo
Re: [PATCH 1/3] drm/mxsfb: Add an entry for "fsl,imx8mq-lcdif"
Hi Lucas, On Tue, Dec 12, 2023 at 3:19 PM Lucas Stach wrote: > I don't really like this series. While we don't make any strong > guarantees in this way, it breaks booting older kernels with a new DT. I thought we needed only to guarantee that old DTs still run with newer kernels, not the other way around. > As this driver patch shows, there are no actual differences in the IP > block between i.MX6SX and i.MX8MQ, it's just the SoC integration that's > different, where in one SoC the controller is located in a power > domain, while it isn't in another. > > To me this smells like making changes to parts that don't really care > about those differences. The thing that cares about the difference is > the DT schema validation and I feel like there must be a better way to > describe this in the schema than to patch both driver and DT just to > accommodate a SoC integration difference. Is there a way to make the > power-domain property not dependent on the IP block, but the machine > compatible? Yes, this series is all about making dt-schema validation happy. If there is a better alternative, I am all ears. Rob, Krzysztof, Conor Any suggestions? Thanks
Re: [PATCH 1/3] drm/mxsfb: Add an entry for "fsl,imx8mq-lcdif"
Hi Fabio,
Am Montag, dem 11.12.2023 um 17:41 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam
>
> On i.MX6SX, the LCDIF has an associated power domain.
>
> However, i.MX8MQ does not have an LCDIF power domain.
>
> imx8mq.dtsi has the following compatible string:
>
> compatible = "fsl,imx8mq-lcdif", "fsl,imx6sx-lcdif";
>
> which causes the following dt-schema warning:
>
> imx8mq-evk.dtb: lcd-controller@3032: 'power-domains' is a required
> property
> from schema $id: http://devicetree.org/schemas/display/fsl,lcdif.yaml#
>
> To prevent this problem, add a specific fsl,imx8mq-lcdif entry in
> the driver to properly handle such a power-domain requirement difference.
>
I don't really like this series. While we don't make any strong
guarantees in this way, it breaks booting older kernels with a new DT.
As this driver patch shows, there are no actual differences in the IP
block between i.MX6SX and i.MX8MQ, it's just the SoC integration that's
different, where in one SoC the controller is located in a power
domain, while it isn't in another.
To me this smells like making changes to parts that don't really care
about those differences. The thing that cares about the difference is
the DT schema validation and I feel like there must be a better way to
describe this in the schema than to patch both driver and DT just to
accommodate a SoC integration difference. Is there a way to make the
power-domain property not dependent on the IP block, but the machine
compatible?
Regards,
Lucas
> Signed-off-by: Fabio Estevam
> ---
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index b483ef48216a..ac9ce3b45b38 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -340,6 +340,7 @@ static const struct of_device_id mxsfb_dt_ids[] = {
> { .compatible = "fsl,imx23-lcdif", .data = _devdata[MXSFB_V3], },
> { .compatible = "fsl,imx28-lcdif", .data = _devdata[MXSFB_V4], },
> { .compatible = "fsl,imx6sx-lcdif", .data = _devdata[MXSFB_V6], },
> + { .compatible = "fsl,imx8mq-lcdif", .data = _devdata[MXSFB_V6], },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxsfb_dt_ids);
[PATCH] dt-bindings: panel-simple-dsi: move LG 5" HD TFT LCD panel into DSI yaml
Originally was in the panel-simple, but belongs to panel-simple-dsi. See arch/arm/boot/dts/nvidia/tegra114-roth.dts for more details. Fixes: ``` arch/arm/boot/dts/tegra114-roth.dt.yaml: panel@0: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: Documentation/devicetree/bindings/display/panel/panel-simple.yaml ``` Signed-off-by: David Heidelberg --- .../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++ .../devicetree/bindings/display/panel/panel-simple.yaml | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml index 73674baea75d..f9160d7bac3c 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml @@ -42,6 +42,8 @@ properties: - lg,acx467akm-7 # LG Corporation 7" WXGA TFT LCD panel - lg,ld070wx3-sl01 +# LG Corporation 5" HD TFT LCD panel + - lg,lh500wx1-sd03 # One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel - osddisplays,osd101t2587-53ts # Panasonic 10" WUXGA TFT LCD panel diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 2021aa82871a..634a10c6f2dd 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -212,8 +212,6 @@ properties: - lemaker,bl035-rgb-002 # LG 7" (800x480 pixels) TFT LCD panel - lg,lb070wv8 -# LG Corporation 5" HD TFT LCD panel - - lg,lh500wx1-sd03 # LG LP079QX1-SP0V 7.9" (1536x2048 pixels) TFT LCD panel - lg,lp079qx1-sp0v # LG 9.7" (2048x1536 pixels) TFT LCD panel -- 2.43.0
Re: [PATCH v3 07/15] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block
On 12/12/2023 1:40 AM, Dmitry Baryshkov wrote:
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote:
CDM block comes with its own set of registers and operations
which can be done. In-line with other hardware blocks, this
change adds the dpu_hw_cdm abstraction for the CDM block.
changes in v3:
- fix commit text from sub-blk to blk for CDM
- fix kbot issue for missing static for dpu_hw_cdm_enable()
- fix kbot issue for incorrect documentation style
- add more documentation for enums and struct in dpu_hw_cdm.h
- drop "enable" parameter from bind_pingpong_blk() as we can
just use PINGPONG_NONE for disable cases
- drop unnecessary bit operation for zero value of cdm_cfg
changes in v2:
- replace bit magic with relevant defines
- use drmm_kzalloc instead of kzalloc/free
- some formatting fixes
- inline _setup_cdm_ops()
- protect bind_pingpong_blk with core_rev check
- drop setup_csc_data() and setup_cdwn() ops as they
are merged into enable()
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202312101815.b3zh7pfy-...@intel.com/
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/Makefile| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 263
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
4 files changed, 395 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 49671364fdcf..b1173128b5b9 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_encoder_phys_wb.o \
disp/dpu1/dpu_formats.o \
disp/dpu1/dpu_hw_catalog.o \
+ disp/dpu1/dpu_hw_cdm.o \
disp/dpu1/dpu_hw_ctl.o \
disp/dpu1/dpu_hw_dsc.o \
disp/dpu1/dpu_hw_dsc_1_2.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
new file mode 100644
index ..4976f8a05ce7
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, The Linux Foundation. All rights reserved.
+ */
+
+#include
+
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_util.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hw_cdm.h"
+#include "dpu_kms.h"
+
+#define CDM_CSC_10_OPMODE 0x000
+#define CDM_CSC_10_BASE0x004
+
+#define CDM_CDWN2_OP_MODE 0x100
+#define CDM_CDWN2_CLAMP_OUT0x104
+#define CDM_CDWN2_PARAMS_3D_0 0x108
+#define CDM_CDWN2_PARAMS_3D_1 0x10C
+#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
+#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
+#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
+#define CDM_CDWN2_COEFF_OFFSITE_H_00x11C
+#define CDM_CDWN2_COEFF_OFFSITE_H_10x120
+#define CDM_CDWN2_COEFF_OFFSITE_H_20x124
+#define CDM_CDWN2_COEFF_COSITE_V 0x128
+#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
+#define CDM_CDWN2_OUT_SIZE 0x130
+
+#define CDM_HDMI_PACK_OP_MODE 0x200
+#define CDM_CSC_10_MATRIX_COEFF_0 0x004
+
+#define CDM_MUX0x224
+
+/* CDM CDWN2 sub-block bit definitions */
+#define CDM_CDWN2_OP_MODE_EN BIT(0)
+#define CDM_CDWN2_OP_MODE_ENABLE_HBIT(1)
+#define CDM_CDWN2_OP_MODE_ENABLE_VBIT(2)
+#define CDM_CDWN2_OP_MODE_METHOD_H_AVGBIT(3)
+#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4)
+#define CDM_CDWN2_OP_MODE_METHOD_V_AVGBIT(5)
+#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6)
+#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
+#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITEGENMASK(4, 3)
+#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITEGENMASK(6, 5)
I think it might be easier to define
enum {
CDM_CDWN2_METHOD_DROP_PIXEL = 0,
CDM_CDWN2_METHOD_AVG = 1,
CDM_CDWN2_METHOD_ = 2,
CDM_CDWN2_METHOD_DROP_PIXEL = 3,
};
then use FIELD_PREP()
Ok, let me explore that option.
We already have below enum in dpu_hw_cdm.h, we can re-use that
+enum dpu_hw_cdwn_type {
+ CDM_CDWN_DISABLE,
+ CDM_CDWN_PIXEL_DROP,
+ CDM_CDWN_AVG,
+ CDM_CDWN_COSITE,
+ CDM_CDWN_OFFSITE,
+};
+#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5)
+#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3)
Why are they called foo_DROP_bar?
Because they are always used with ~
And as per the documentation 0 for this field is pixel drop.
If we are dropping the clearing altogether, then these masks will go
away too.
+
+/* CDM CSC10 sub-block bit definitions */
+#define
Re: [PATCH 3/3] drm/amd/display: Support DRM_AMD_DC_FP on RISC-V
On Mon, Dec 11, 2023 at 11:12:42PM -0800, Christoph Hellwig wrote: > On Thu, Dec 07, 2023 at 10:49:53PM -0600, Samuel Holland wrote: > > Actually tracking all possibly-FPU-tainted functions and their call sites is > > probably possible, but a much larger task. > > I think objtool should be able to do that reasonably easily, it already > does it for checking section where userspace address access is enabled > or not, which is very similar. Yeah, that might be doable. I can look into it. -- Josh
[PATCH 0/5] clk: Make clk_rate_exclusive_get() return void
Hello, clk_rate_exclusive_get() returns zero unconditionally. Most users "know" that and don't check the return value. This series fixes the four users that do error checking on the returned value and then makes function return void. Given that the changes to the drivers are simple and so merge conflicts (if any) should be easy to handle, I suggest to merge this complete series via the clk tree. Best regards Uwe Uwe Kleine-König (5): PM / devfreq: sun8i-a33-mbus: Simplify usage of clk_rate_exclusive_get() drm/meson: Simplify usage of clk_rate_exclusive_get() memory: tegra210-emc: Simplify usage of clk_rate_exclusive_get() memory: tegra30-emc: Simplify usage of clk_rate_exclusive_get() clk: Make clk_rate_exclusive_get() return void drivers/clk/clk.c | 6 ++ drivers/devfreq/sun8i-a33-mbus.c | 14 ++ drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 8 +--- drivers/memory/tegra/tegra210-emc-core.c | 6 +- drivers/memory/tegra/tegra30-emc.c| 6 +- include/linux/clk.h | 8 +++- 6 files changed, 10 insertions(+), 38 deletions(-) base-commit: bbd220ce4e29ed55ab079007cff0b550895258eb -- 2.42.0
[PATCH 5/5] clk: Make clk_rate_exclusive_get() return void
The function currently returns 0 unconditionally. This isn't very useful
and makes users create dead code error paths. So let this function
return no value. All users were adapted before to ignore the returned
value.
Also fix a few typos in the kernel doc comment for
clk_rate_exclusive_get().
Signed-off-by: Uwe Kleine-König
---
drivers/clk/clk.c | 6 ++
include/linux/clk.h | 8 +++-
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 2253c154a824..af2011c2a93b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -925,17 +925,15 @@ static void clk_core_rate_restore_protect(struct clk_core
*core, int count)
* clk_rate_exclusive_put(). Calls to this function may sleep.
* Returns 0 on success, -EERROR otherwise
*/
-int clk_rate_exclusive_get(struct clk *clk)
+void clk_rate_exclusive_get(struct clk *clk)
{
if (!clk)
- return 0;
+ return;
clk_prepare_lock();
clk_core_rate_protect(clk->core);
clk->exclusive_count++;
clk_prepare_unlock();
-
- return 0;
}
EXPORT_SYMBOL_GPL(clk_rate_exclusive_get);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 06f1b292f8a0..f88c407925f8 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -190,16 +190,14 @@ bool clk_is_match(const struct clk *p, const struct clk
*q);
*
* This function allows drivers to get exclusive control over the rate of a
* provider. It prevents any other consumer to execute, even indirectly,
- * opereation which could alter the rate of the provider or cause glitches
+ * operation which could alter the rate of the provider or cause glitches
*
- * If exlusivity is claimed more than once on clock, even by the same driver,
+ * If exclusivity is claimed more than once on clock, even by the same driver,
* the rate effectively gets locked as exclusivity can't be preempted.
*
* Must not be called from within atomic context.
- *
- * Returns success (0) or negative errno.
*/
-int clk_rate_exclusive_get(struct clk *clk);
+void clk_rate_exclusive_get(struct clk *clk);
/**
* clk_rate_exclusive_put - release exclusivity over the rate control of a
--
2.42.0
[PATCH 2/5] drm/meson: Simplify usage of clk_rate_exclusive_get()
clk_rate_exclusive_get() returns 0 unconditionally. So remove error
handling. This prepares making clk_rate_exclusive_get() return void.
Signed-off-by: Uwe Kleine-König
---
Hello,
btw, the driver fails to call clk_rate_exclusive_put() in the error
paths of dw_mipi_dsi_phy_init().
Best regards
Uwe
drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
index a6bc1bdb3d0d..49571f1ed88a 100644
--- a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
+++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
@@ -88,15 +88,9 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
}
/* Make sure the rate of the bit clock is not modified by someone else
*/
- ret = clk_rate_exclusive_get(mipi_dsi->bit_clk);
- if (ret) {
- dev_err(mipi_dsi->dev,
- "Failed to set the exclusivity on the bit clock rate
(ret %d)\n", ret);
- return ret;
- }
+ clk_rate_exclusive_get(mipi_dsi->bit_clk);
ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->mode->clock * 1000);
-
if (ret) {
dev_err(mipi_dsi->dev, "Failed to set DSI Pixel clock rate %u
(%d)\n",
mipi_dsi->mode->clock * 1000, ret);
--
2.42.0
Re: [PATCH v3 00/12] RB1/QCM2290 features
On Wed, 29 Nov 2023 15:43:57 +0100, Konrad Dybcio wrote: > This series brings: > - interconnect plumbing > - display setup > > for QCM2290/QRB2210 and > > - CAN bus controller > - HDMI display > - wifi fw variant name > > [...] Applied SMMU update to will (for-joerg/arm-smmu/updates), thanks! [05/12] iommu/arm-smmu-qcom: Add QCM2290 MDSS compatible https://git.kernel.org/will/c/28af105cb650 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev
Re: [PATCH v3 01/15] drm/msm/dpu: add formats check for writeback encoder
On 12/11/2023 10:40 PM, Dmitry Baryshkov wrote:
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote:
In preparation for adding more formats to dpu writeback add
format validation to it to fail any unsupported formats.
changes in v3:
- rebase on top of msm-next
- replace drm_atomic_helper_check_wb_encoder_state() with
drm_atomic_helper_check_wb_connector_state() due to the
rebase
changes in v2:
- correct some grammar in the commit text
Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for
writeback")
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index bb94909caa25..425415d45ec1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check(
{
struct drm_framebuffer *fb;
const struct drm_display_mode *mode = _state->mode;
+ int ret;
DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
phys_enc->hw_wb->idx, mode->name, mode->hdisplay,
mode->vdisplay);
@@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check(
return -EINVAL;
}
+ ret = drm_atomic_helper_check_wb_connector_state(conn_state->connector,
conn_state->state);
+ if (ret < 0) {
+ DPU_ERROR("invalid pixel format %p4cc\n", >format->format);
+ return ret;
+ }
There is no guarantee that there will be no other checks added to this
helper. So, I think this message is incorrect. If you wish, you can
promote the level of the message in the helper itself.
On the other hand, we rarely print such messages by default. Most of
the checks use drm_dbg.
hmm...actually drm_atomic_helper_check_wb_connector_state() already has
a debug message to indicate invalid pixel formats.
You are right, i should perhaps just say that "atomic_check failed" or
something.
I can make this a DPU_DEBUG. Actually I didnt know that we are not
supposed to print out atomic_check() errors. Is it perhaps because its
okay for check to fail?
But then we would not know why it failed.
+
return 0;
}
--
2.40.1
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
Hi Abhinav,
On Tue, 12 Dec 2023 at 08:49, Dmitry Baryshkov
wrote:
>
> On Mon, 11 Dec 2023 at 23:48, Abhinav Kumar wrote:
> >
> >
> >
> > On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote:
> > > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar
> > > wrote:
> > >>
> > >>
> > >>
> > >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote:
> > >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar
> > >>> wrote:
> >
> >
> >
> > On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
> > > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar
> > > wrote:
> > >>
> > >> Add CDM blocks to the sc7280 dpu_hw_catalog to support
> > >> YUV format output from writeback block.
> > >>
> > >> changes in v2:
> > >>- remove explicit zero assignment for features
> > >>- move sc7280_cdm to dpu_hw_catalog from the sc7280
> > >> catalog file as its definition can be re-used
> > >>
> > >> Signed-off-by: Abhinav Kumar
> > >> ---
> > >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 +
> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10
> > >> ++
> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13
> > >> +
> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +
> > >> 4 files changed, 29 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> > >> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> > >> index 209675de6742..19c2b7454796 100644
> > >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> > >> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
> > >>.mdss_ver = _mdss_ver,
> > >>.caps = _dpu_caps,
> > >>.mdp = _mdp,
> > >> + .cdm = _cdm,
> > >>.ctl_count = ARRAY_SIZE(sc7280_ctl),
> > >>.ctl = sc7280_ctl,
> > >>.sspp_count = ARRAY_SIZE(sc7280_sspp),
> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> index d52aae54bbd5..1be3156cde05 100644
> > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1
> > >> = {
> > >>.ctl = {.name = "ctl", .base = 0xF80, .len = 0x10},
> > >> };
> > >>
> > >> +/*
> > >> + * CDM sub block config
> > >
> > > Nit: it is not a subblock config.
> > >
> >
> > Ack.
> >
> > >> + */
> > >> +static const struct dpu_cdm_cfg sc7280_cdm = {
> > >
> > > I know that I have r-b'ed this patch. But then one thing occurred to
> > > me. If this definition is common to all (or almost all) platforms, can
> > > we just call it dpu_cdm or dpu_common_cdm?
> > >
> > >> + .name = "cdm_0",
> > >> + .id = CDM_0,
> > >> + .len = 0x228,
> > >> + .base = 0x79200,
> > >> +};
> >
> > hmmm almost common but not entirely ... msm8998's CDM has a
> > shorter
> > len of 0x224 :(
> > >>>
> > >>> Then sdm845_cdm?
> > >>>
> > >>
> > >> That also has a shorter cdm length.
> > >
> > > Could you please clarify. According to the downstream DT files all CDM
> > > blocks up to (but not including) sm8550 had length 0x224. SM8550 and
> > > SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers
> > > after 0x204.
> > >>
> >
> > We always list 0x4 more than the last offset.
>
> Yes, so this makes it correct for several latest DT files, which have
> qcom,sde-cdm-size = <0x220>.
> However all the previous DT files (from msm8998 to sm8450) had
> qcom,sde-cdm-size = <0x224>;
Ok, I think I got it, what you were writing about. And we can ignore
the sde-cdm-size from the DT files.
>
> >
> > In chipsets sdm845 and msm8998, I only see the last offset of CDM as
> > 0x220 but in sm8250 and sc7280, the last offset is 0x224. Hence the
> > total length is more in sc7280/sm8250 as compared to sdm845/msm8998.
> >
> > I didnt follow that you do not see any registers after 0x204.
> >
> > The CDM_MUX is the last offset which has an offset 0x224 from the base
> > address. So thats the last offset.
>
> Ack
>
> >
> > The newer chipsets have CDM_MUX and the older ones did not. Hence the
> > difference in length.
> >
> > >> BTW, sdm845 is not in this series. It will be part of RFT as we
> > >> discussed.
> > >>
> >
> > >> +
> > >> /*
> > >> * VBIF sub blocks config
> > >>
[PATCH v8 1/2] drm/i915/guc: Flush context destruction worker at suspend
When suspending, flush the context-guc-id
deregistration worker at the final stages of
intel_gt_suspend_late when we finally call gt_sanitize
that eventually leads down to __uc_sanitize so that
the deregistration worker doesn't fire off later as
we reset the GuC microcontroller.
Signed-off-by: Alan Previn
Reviewed-by: Rodrigo Vivi
Tested-by: Mousumi Jana
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 +
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h | 2 ++
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 ++
3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a259f1118c5a..9c64ae0766cc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1613,6 +1613,11 @@ static void guc_flush_submissions(struct intel_guc *guc)
spin_unlock_irqrestore(_engine->lock, flags);
}
+void intel_guc_submission_flush_work(struct intel_guc *guc)
+{
+ flush_work(>submission_state.destroyed_worker);
+}
+
static void guc_flush_destroyed_contexts(struct intel_guc *guc);
void intel_guc_submission_reset_prepare(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index c57b29cdb1a6..b6df75622d3b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -38,6 +38,8 @@ int intel_guc_wait_for_pending_msg(struct intel_guc *guc,
bool interruptible,
long timeout);
+void intel_guc_submission_flush_work(struct intel_guc *guc);
+
static inline bool intel_guc_submission_is_supported(struct intel_guc *guc)
{
return guc->submission_supported;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3872d309ed31..b8b09b1bee3e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -690,6 +690,8 @@ void intel_uc_suspend(struct intel_uc *uc)
return;
}
+ intel_guc_submission_flush_work(guc);
+
with_intel_runtime_pm(_to_gt(uc)->i915->runtime_pm, wakeref) {
err = intel_guc_suspend(guc);
if (err)
--
2.39.0
[PATCH v8 2/2] drm/i915/guc: Close deregister-context race against CT-loss
If we are at the end of suspend or very early in resume
its possible an async fence signal (via rcu_call) is triggered
to free_engines which could lead us to the execution of
the context destruction worker (after a prior worker flush).
Thus, when suspending, insert rcu_barriers at the start
of i915_gem_suspend (part of driver's suspend prepare) and
again in i915_gem_suspend_late so that all such cases have
completed and context destruction list isn't missing anything.
In destroyed_worker_func, close the race against CT-loss
by checking that CT is enabled before calling into
deregister_destroyed_contexts.
Based on testing, guc_lrc_desc_unpin may still race and fail
as we traverse the GuC's context-destroy list because the
CT could be disabled right before calling GuC's CT send function.
We've witnessed this race condition once every ~6000-8000
suspend-resume cycles while ensuring workloads that render
something onscreen is continuously started just before
we suspend (and the workload is small enough to complete
and trigger the queued engine/context free-up either very
late in suspend or very early in resume).
In such a case, we need to unroll the entire process because
guc-lrc-unpin takes a gt wakeref which only gets released in
the G2H IRQ reply that never comes through in this corner
case. Without the unroll, the taken wakeref is leaked and will
cascade into a kernel hang later at the tail end of suspend in
this function:
intel_wakeref_wait_for_idle(>wakeref)
(called by) - intel_gt_pm_wait_for_idle
(called by) - wait_for_suspend
Thus, do an unroll in guc_lrc_desc_unpin and deregister_destroyed_-
contexts if guc_lrc_desc_unpin fails due to CT send falure.
When unrolling, keep the context in the GuC's destroy-list so
it can get picked up on the next destroy worker invocation
(if suspend aborted) or get fully purged as part of a GuC
sanitization (end of suspend) or a reset flow.
Signed-off-by: Alan Previn
Signed-off-by: Anshuman Gupta
Tested-by: Mousumi Jana
Acked-by: Daniele Ceraolo Spurio
---
drivers/gpu/drm/i915/gem/i915_gem_pm.c| 10 +++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 73 +--
2 files changed, 78 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 0d812f4d787d..3b27218aabe2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -28,6 +28,13 @@ void i915_gem_suspend(struct drm_i915_private *i915)
GEM_TRACE("%s\n", dev_name(i915->drm.dev));
intel_wakeref_auto(>runtime_pm.userfault_wakeref, 0);
+ /*
+* On rare occasions, we've observed the fence completion triggers
+* free_engines asynchronously via rcu_call. Ensure those are done.
+* This path is only called on suspend, so it's an acceptable cost.
+*/
+ rcu_barrier();
+
flush_workqueue(i915->wq);
/*
@@ -160,6 +167,9 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
* machine in an unusable condition.
*/
+ /* Like i915_gem_suspend, flush tasks staged from fence triggers */
+ rcu_barrier();
+
for_each_gt(gt, i915, i)
intel_gt_suspend_late(gt);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9c64ae0766cc..cae637fc3ead 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -236,6 +236,13 @@ set_context_destroyed(struct intel_context *ce)
ce->guc_state.sched_state |= SCHED_STATE_DESTROYED;
}
+static inline void
+clr_context_destroyed(struct intel_context *ce)
+{
+ lockdep_assert_held(>guc_state.lock);
+ ce->guc_state.sched_state &= ~SCHED_STATE_DESTROYED;
+}
+
static inline bool context_pending_disable(struct intel_context *ce)
{
return ce->guc_state.sched_state & SCHED_STATE_PENDING_DISABLE;
@@ -613,6 +620,8 @@ static int guc_submission_send_busy_loop(struct intel_guc
*guc,
u32 g2h_len_dw,
bool loop)
{
+ int ret;
+
/*
* We always loop when a send requires a reply (i.e. g2h_len_dw > 0),
* so we don't handle the case where we don't get a reply because we
@@ -623,7 +632,11 @@ static int guc_submission_send_busy_loop(struct intel_guc
*guc,
if (g2h_len_dw)
atomic_inc(>outstanding_submission_g2h);
- return intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
+ ret = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
+ if (ret)
+ atomic_dec(>outstanding_submission_g2h);
+
+ return ret;
}
int intel_guc_wait_for_pending_msg(struct intel_guc *guc,
@@ -3288,12 +3301,13 @@ static void guc_context_close(struct intel_context *ce)
[PATCH v8 0/2] Resolve suspend-resume racing with GuC destroy-context-worker
This series is the result of debugging issues root caused to races between the GuC's destroyed_worker_func being triggered vs repeating suspend-resume cycles with concurrent delayed fence signals for engine-freeing. The reproduction steps require that an app is launched right before the start of the suspend cycle where it creates a new gem context and submits a tiny workload that would complete in the middle of the suspend cycle. However this app uses dma-buffer sharing or dma-fence with non-GPU objects or signals that eventually triggers a FENCE_FREE via__i915_sw_fence_notify that connects to engines_notify -> free_engines_rcu -> intel_context_put -> kref_put(>ref..) that queues the worker after the GuCs CTB has been disabled (i.e. after i915-gem's suspend-late). This sequence is a corner-case and required repeating this app->suspend->resume cycle ~1500 times across 4 identical systems to see it once. That said, based on above callstack, it is clear that merely flushing the context destruction worker, which is obviously missing and needed, isn't sufficient. Because of that, this series adds additional patches besides the obvious (Patch #1) flushing of the worker during the suspend flows. It also includes (Patch #2) closing a race between sending the context-deregistration H2G vs the CTB getting disabled in the midst of it (by detecing the failure and unrolling the guc-lrc-unpin flow) and adding an additional rcu_barrier in the gem-suspend flow to purge outstanding rcu defered tasks that may include context destruction. This patch was tested and confirmed to be reliably working after running ~1500 suspend resume cycles on 4 concurrent machines. Changes from prior revs: v7: - Rebase on latest drm-tip. v6: - Dont hold the spinlock while calling deregister_context which can take a longer time. (Rodrigo) - Fix / improve of comments. (Rodrigo) - Release the ce->guc_state.lock before calling deregister_context and retake it if we fail. (John/Daniele). v5: - Remove Patch #3 which doesnt solve this exact bug but can be a separate patch(Tvrtko). v4: - In Patch #2, change the position of the calls into rcu_barrier based on latest testing data. (Alan/Anshuman). - In Patch #3, fix the timeout value selection for the final gt-pm idle-wait that was incorrectly using a 'ns' #define as a milisec timeout. v3: - In Patch #3, when deregister_context fails, instead of calling intel_gt_pm_put(that might sleep), call __intel_wakeref_put (without ASYNC flag) (Rodrigo/Anshuman). - In wait_for_suspend add an rcu_barrier before we proceed to wait for idle. (Anshuman) v2: - Patch #2 Restructure code in guc_lrc_desc_unpin so it's more readible to differentiate (1)direct guc-id cleanup ..vs (2) sending the H2G ctx-destroy action .. vs (3) the unrolling steps if the H2G fails. - Patch #2 Add a check to close the race sooner by checking for intel_guc_is_ready from destroyed_worker_func. - Patch #2 When guc_submission_send_busy_loop gets a failure from intel_guc_send_busy_loop, we need to undo i.e. decrement the outstanding_submission_g2h. - Patch #3 In wait_for_suspend, fix checking of return from intel_gt_pm_wait_timeout_for_idle to now use -ETIMEDOUT and add documentation for intel_wakeref_wait_for_idle. (Rodrigo). Alan Previn (2): drm/i915/guc: Flush context destruction worker at suspend drm/i915/guc: Close deregister-context race against CT-loss drivers/gpu/drm/i915/gem/i915_gem_pm.c| 10 +++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 78 +-- .../gpu/drm/i915/gt/uc/intel_guc_submission.h | 2 + drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 + 4 files changed, 87 insertions(+), 5 deletions(-) base-commit: b4182ec1538e8cebf630083ec4296bed0061d594 -- 2.39.0
Re: [PATCH 2/3] dt-bindings: lcdif: Decouple imx8mq from imx6sx
On Mon, Dec 11, 2023 at 05:41:37PM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > On i.MX6SX, the LCDIF has an associated power domain. > > i.MX8MQ does not have an LCDIF power domain, so allow passing only > "fsl,imx8mq-lcdif" as compatible string to fix the following > dt-schema warning: > > imx8mq-evk.dtb: lcd-controller@3032: 'power-domains' is a required > property > from schema $id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# > > Signed-off-by: Fabio Estevam Acked-by: Conor Dooley Cheers, Conor. signature.asc Description: PGP signature
Re: [PATCH v3 0/7] dma-buf: heaps: Add secure heap
Is there a chance to pick a better name than "secure" here? "Secure" is super overloaded, it's not clear at all what it means from just the name. Something like "restricted" would be an improvement.
[PATCH] drm/ci: uprev mesa version: fix kdl commit fetch
build-kdl.sh was doing a `clone --depth 1` of the default branch,
then checking out a commit that might not be the latest of that
branch, resulting in container build error.
https://gitlab.freedesktop.org/mesa/mesa/-/commit/5efa4d56 fixes
kdl commit fetch issue. Uprev mesa in drm-ci to fix this.
This commit also updates the kernel tag and adds .never-post-merge-rules
due to the mesa uprev.
Tested-by: Abhinav Kumar
Signed-off-by: Vignesh Raman
---
drivers/gpu/drm/ci/gitlab-ci.yml | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ci/gitlab-ci.yml b/drivers/gpu/drm/ci/gitlab-ci.yml
index aeb9bab1b069..b215d7b3d6ca 100644
--- a/drivers/gpu/drm/ci/gitlab-ci.yml
+++ b/drivers/gpu/drm/ci/gitlab-ci.yml
@@ -1,6 +1,6 @@
variables:
DRM_CI_PROJECT_PATH: mesa/mesa
- DRM_CI_COMMIT_SHA:
edfbf74df1d4d6ce54ffe24566108be0e1a98c3d
+ DRM_CI_COMMIT_SHA:
9d162de9a05155e1c4041857a5848842749164cf
UPSTREAM_REPO: git://anongit.freedesktop.org/drm/drm
TARGET_BRANCH: drm-next
@@ -25,7 +25,9 @@ variables:
# per-job artifact storage on MinIO
JOB_ARTIFACTS_BASE: ${PIPELINE_ARTIFACTS_BASE}/${CI_JOB_ID}
# default kernel for rootfs before injecting the current kernel tree
- KERNEL_IMAGE_BASE:
https://${S3_HOST}/mesa-lava/gfx-ci/linux/v6.4.12-for-mesa-ci-f6b4ad45f48d
+ KERNEL_REPO: "gfx-ci/linux"
+ KERNEL_TAG: "v6.6.4-for-mesa-ci-e4f4c500f7fb"
+ KERNEL_IMAGE_BASE: https://${S3_HOST}/mesa-lava/${KERNEL_REPO}/${KERNEL_TAG}
LAVA_TAGS: subset-1-gfx
LAVA_JOB_PRIORITY: 30
@@ -133,6 +135,11 @@ stages:
- if: '$GITLAB_USER_LOGIN == "marge-bot" &&
$CI_PIPELINE_SOURCE == "merge_request_event"'
when: on_success
+.never-post-merge-rules:
+ rules:
+- if: *is-post-merge
+ when: never
+
# Rule to filter for only scheduled pipelines.
.scheduled_pipeline-rules:
rules:
@@ -150,6 +157,7 @@ stages:
.build-rules:
rules:
- !reference [.no_scheduled_pipelines-rules, rules]
+- !reference [.never-post-merge-rules, rules]
# Run automatically once all dependency jobs have passed
- when: on_success
@@ -157,6 +165,7 @@ stages:
.container+build-rules:
rules:
- !reference [.no_scheduled_pipelines-rules, rules]
+- !reference [.never-post-merge-rules, rules]
- when: manual
.ci-deqp-artifacts:
@@ -175,6 +184,7 @@ stages:
.container-rules:
rules:
- !reference [.no_scheduled_pipelines-rules, rules]
+- !reference [.never-post-merge-rules, rules]
# Run pipeline by default in the main project if any CI pipeline
# configuration files were changed, to ensure docker images are up to date
- if: *is-post-merge
--
2.40.1
Re: [PATCH v7 3/6] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios (v7)
Hi Vivek,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url:
https://github.com/intel-lab-lkp/linux/commits/Vivek-Kasireddy/udmabuf-Use-vmf_insert_pfn-and-VM_PFNMAP-for-handling-mmap/20231212-160312
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git
mm-everything
patch link:
https://lore.kernel.org/r/20231212073803.3233055-4-vivek.kasireddy%40intel.com
patch subject: [PATCH v7 3/6] mm/gup: Introduce memfd_pin_folios() for pinning
memfd folios (v7)
config: i386-allnoconfig
(https://download.01.org/0day-ci/archive/20231212/202312122339.viqujwiw-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231212/202312122339.viqujwiw-...@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/202312122339.viqujwiw-...@intel.com/
All errors (new ones prefixed by >>):
>> mm/gup.c:3543:11: error: incompatible pointer types assigning to 'struct
>> folio *' from 'struct page *' [-Werror,-Wincompatible-pointer-types]
folio = memfd_alloc_folio(memfd, start_idx);
^ ~~~
1 error generated.
vim +3543 mm/gup.c
3417
3418 /**
3419 * memfd_pin_folios() - pin folios associated with a memfd
3420 * @memfd: the memfd whose folios are to be pinned
3421 * @start: starting memfd offset
3422 * @nr_pages: number of pages from start to pin
3423 * @folios: array that receives pointers to the folios pinned.
3424 * Should be at-least nr_pages long.
3425 * @offsets:array that receives offsets of pages in their folios.
3426 * Should be at-least nr_pages long.
3427 *
3428 * Attempt to pin folios associated with a memfd; given that a memfd is
3429 * either backed by shmem or hugetlb, the folios can either be found in
3430 * the page cache or need to be allocated if necessary. Once the folios
3431 * are located, they are all pinned via FOLL_PIN and the @offsets array
3432 * is populated with offsets of the pages in their respective folios.
3433 * Therefore, for each page the caller requested, there will be a
3434 * corresponding entry in both @folios and @offsets. And, eventually,
3435 * these pinned folios need to be released either using
unpin_user_pages()
3436 * or unpin_user_page().
3437 *
3438 * It must be noted that the folios may be pinned for an indefinite
amount
3439 * of time. And, in most cases, the duration of time they may stay
pinned
3440 * would be controlled by the userspace. This behavior is effectively
the
3441 * same as using FOLL_LONGTERM with other GUP APIs.
3442 *
3443 * Returns number of folios pinned. This would be equal to the number of
3444 * pages requested. If no folios were pinned, it returns -errno.
3445 */
3446 long memfd_pin_folios(struct file *memfd, unsigned long start,
3447unsigned long nr_pages, struct folio **folios,
3448pgoff_t *offsets)
3449 {
3450 unsigned long end = start + (nr_pages << PAGE_SHIFT) - 1;
3451 unsigned int max_pgs, pgoff, pgshift = PAGE_SHIFT;
3452 pgoff_t start_idx, end_idx, next_idx;
3453 unsigned int flags, nr_folios, i, j;
3454 struct folio *folio = NULL;
3455 struct folio_batch fbatch;
3456 struct page **pages;
3457 struct hstate *h;
3458 long ret;
3459
3460 if (!nr_pages)
3461 return -EINVAL;
3462
3463 if (!memfd)
3464 return -EINVAL;
3465
3466 if (!shmem_file(memfd) && !is_file_hugepages(memfd))
3467 return -EINVAL;
3468
3469 pages = kmalloc_array(nr_pages, sizeof(*pages), GFP_KERNEL);
3470 if (!pages)
3471 return -ENOMEM;
3472
3473 if (is_file_hugepages(memfd)) {
3474 h = hstate_file(memfd);
3475 pgshift = huge_page_shift(h);
3476 }
3477
3478 flags = memalloc_pin_save();
3479 do {
3480 i = 0;
3481 start_idx = start >> pgshift;
3482 end_idx = end >> pgshift;
3483 if (is_file_hugepages(memfd)) {
3484 start_idx <<= huge_page_order(h);
3485 end_idx <<= huge_page_order(h);
3486 }
3487
3488
Re: [PATCH v8 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
On Tue, Dec 12, 2023 at 07:22:18AM -0800, Guenter Roeck wrote: > On 12/12/23 03:41, Uwe Kleine-König wrote: > > Several affected maintainers already acked, so I guess it's fine to take > > this via the pwm tree. An Ack from the remaining maintainers would be > > very welcome, an alternative would be to split the patch. > > Missing Acks so far: > > - Jean Delvare / Guenter Roeck for drivers/hwmon/pwm-fan.c > > - Javier Martinez Canillas for drivers/gpu/drm/solomon/ssd130x.c > > - Liam Girdwood / Mark Brown for drivers/regulator/pwm-regulator.c > > - Helge Deller for drivers/video/fbdev/ssd1307fb.c > Personally I find the change unnecessary and pointless, which is why I > didn't ack it. Even if function names were deemed important enough, keeping > pwm_apply_state() for the time being and just adding pwm_apply_might_sleep() > as duplicate would have done it, all the changes could have gone in long > ago, and per-subsystem cleanup could have been orthogonal. > I refrained from commenting because it might be considered bike shedding, > but I don't want to ack something I deem unnecessary and pointless without > comment. But then don't want to keep arguing either, so I haven't been reading this series because I couldn't tell why I was copied on it, it's only chance that made me open Guenter's mail here... Acked-by: Mark Brown signature.asc Description: PGP signature
Re: [PATCH v8 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
On 12/12/23 03:41, Uwe Kleine-König wrote: On Tue, Dec 12, 2023 at 08:34:00AM +, Sean Young wrote: In order to introduce a pwm api which can be used from atomic context, we will need two functions for applying pwm changes: int pwm_apply_might_sleep(struct pwm *, struct pwm_state *); int pwm_apply_atomic(struct pwm *, struct pwm_state *); This commit just deals with renaming pwm_apply_state(), a following commit will introduce the pwm_apply_atomic() function. Acked-by: Dmitry Torokhov # for input Acked-by: Hans de Goede Acked-by: Jani Nikula Acked-by: Lee Jones Signed-off-by: Sean Young Acked-by: Uwe Kleine-König Several affected maintainers already acked, so I guess it's fine to take this via the pwm tree. An Ack from the remaining maintainers would be very welcome, an alternative would be to split the patch. Missing Acks so far: - Jean Delvare / Guenter Roeck for drivers/hwmon/pwm-fan.c - Javier Martinez Canillas for drivers/gpu/drm/solomon/ssd130x.c - Liam Girdwood / Mark Brown for drivers/regulator/pwm-regulator.c - Helge Deller for drivers/video/fbdev/ssd1307fb.c Best regards Uwe Personally I find the change unnecessary and pointless, which is why I didn't ack it. Even if function names were deemed important enough, keeping pwm_apply_state() for the time being and just adding pwm_apply_might_sleep() as duplicate would have done it, all the changes could have gone in long ago, and per-subsystem cleanup could have been orthogonal. I refrained from commenting because it might be considered bike shedding, but I don't want to ack something I deem unnecessary and pointless without comment. But then don't want to keep arguing either, so Acked-by: Guenter Roeck Guenter
Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider
On Tue, Dec 12, 2023 at 06:58:17AM -0800, Mina Almasry wrote: > Jason, we set the LSB on page_pool_iov pointers before casting it to > struct page pointers. The resulting pointers are not useable as page > pointers at all. I understand that, the second ask is about maintainability of the mm by using correct types. > > Perhaps you can simply avoid this by arranging for this driver to also > > exclusively use some special type to indicate the dual nature of the > > pointer and leave the other drivers as using the struct page version. > > This is certainly possible, but it requires us to rename all the page > pointers in the page_pool to the new type, and requires the driver > adding devmem TCP support to rename all the page* pointer instances to > the new type. It's possible but it introduces lots of code churn. Is > the LSB + cast not a reasonable compromise here? I feel like the trick > of setting the least significant bit on a pointer to indicate it's > something else has a fair amount of precedent in the kernel. Linus himself has complained about exactly this before, and written a cleanup: https://lore.kernel.org/linux-mm/20221108194139.57604-1-torva...@linux-foundation.org/ If you mangle a pointer *so it is no longer a pointer* then give it a proper opaque type so the compiler can check everything statically and require that the necessary converters are called in all cases. You call it churn, I call it future maintainability. :( No objection to using the LSB, just properly type a LSB mangled pointer so everyone knows what is going on and don't call it MM's struct page *. I would say this is important here because it is a large driver facing API surface. Jason
Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider
On Tue, Dec 12, 2023 at 6:39 AM Jason Gunthorpe wrote:
>
> On Tue, Dec 12, 2023 at 06:26:51AM -0800, Mina Almasry wrote:
> > On Tue, Dec 12, 2023 at 4:25 AM Jason Gunthorpe wrote:
> > >
> > > On Thu, Dec 07, 2023 at 04:52:39PM -0800, Mina Almasry wrote:
> > >
> > > > +static inline struct page_pool_iov *page_to_page_pool_iov(struct page
> > > > *page)
> > > > +{
> > > > + if (page_is_page_pool_iov(page))
> > > > + return (struct page_pool_iov *)((unsigned long)page &
> > > > ~PP_IOV);
> > > > +
> > > > + DEBUG_NET_WARN_ON_ONCE(true);
> > > > + return NULL;
> > > > +}
> > >
> > > We already asked not to do this, please do not allocate weird things
> > > can call them 'struct page' when they are not. It undermines the
> > > maintainability of the mm to have things mis-typed like
> > > this. Introduce a new type for your thing so the compiler can check it
> > > properly.
> > >
> >
> > There is a new type introduced, it's the page_pool_iov. We set the LSB
> > on page_pool_iov* and cast it to page* only to avoid the churn of
> > renaming page* to page_pool_iov* in the page_pool and all the net
> > drivers using it. Is that not a reasonable compromise in your opinion?
> > Since the LSB is set on the resulting page pointers, they are not
> > actually usuable as pages, and are never passed to mm APIs per your
> > requirement.
>
> There were two asks, the one you did was to never pass this non-struct
> page memory to the mm, which is great.
>
> The other was to not mistype things, and don't type something as
> struct page when it is, in fact, not.
>
> I fear what you've done is make it so only one driver calls these
> special functions and left the other drivers passing the struct page
> directly to the mm and sort of obfuscating why it is OK based on this
> netdev knowledge of not enabling/using the static branch in the other
> cases.
>
Jason, we set the LSB on page_pool_iov pointers before casting it to
struct page pointers. The resulting pointers are not useable as page
pointers at all.
In order to use the resulting pointers, the driver _must_ use the
special functions that first clear the LSB. It is impossible for the
driver to 'accidentally' use the resulting page pointers with the LSB
set - the kernel would just crash trying to dereference such a
pointer.
The way it works currently is that drivers that support devmem TCP
will declare that support to the net stack, and use the special
functions that clear the LSB and cast the struct back to
page_pool_iov. The drivers that don't support devmem TCP will not
declare support and will get pages allocated from the mm stack from
the page_pool and use them as pages normally.
> Perhaps you can simply avoid this by arranging for this driver to also
> exclusively use some special type to indicate the dual nature of the
> pointer and leave the other drivers as using the struct page version.
>
This is certainly possible, but it requires us to rename all the page
pointers in the page_pool to the new type, and requires the driver
adding devmem TCP support to rename all the page* pointer instances to
the new type. It's possible but it introduces lots of code churn. Is
the LSB + cast not a reasonable compromise here? I feel like the trick
of setting the least significant bit on a pointer to indicate it's
something else has a fair amount of precedent in the kernel.
--
Thanks,
Mina
Re: Radeon regression in 6.6 kernel
On Mon, Dec 11, 2023 at 7:28 PM Phillip Susi wrote: > > Phillip Susi writes: > > > And it works, but 6.7-rc5 does not, even though it includes that patch. > > Here's the syslog from the attempt. I'll start bisecting again. > > I checked out the patch that got merged upstream and it also fails. I > looked at the two commits, and I see what happened. Your original patch > MOVED the call to amdgpu_ttm_set_buffer_funcs_status(). The one that > got merged into Linus' tree instead DUPLICATES the call. Oops? > Yeah, I messed up the patch somehow when I applied it. Fix up already queued up to add the missing chunk. Alex
[PATCH] drm/edid: prefer forward declarations over includes in drm_edid.h
There's no need to include either linux/hdmi.h or drm/drm_mode.h. They
can be removed by using forward declarations.
While at it, group the forward declarations together, and remove the
unnecessary ones.
Signed-off-by: Jani Nikula
---
include/drm/drm_edid.h | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 518d1b8106c7..37c2702ee9ef 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -24,11 +24,14 @@
#define __DRM_EDID_H__
#include
-#include
-#include
+enum hdmi_quantization_range;
+struct drm_connector;
struct drm_device;
+struct drm_display_mode;
struct drm_edid;
+struct hdmi_avi_infoframe;
+struct hdmi_vendor_infoframe;
struct i2c_adapter;
#define EDID_LENGTH 128
@@ -319,11 +322,6 @@ struct cea_sad {
u8 byte2; /* meaning depends on format */
};
-struct drm_encoder;
-struct drm_connector;
-struct drm_connector_state;
-struct drm_display_mode;
-
int drm_edid_to_sad(const struct edid *edid, struct cea_sad **sads);
int drm_edid_to_speaker_allocation(const struct edid *edid, u8 **sadb);
int drm_av_sync_delay(struct drm_connector *connector,
--
2.39.2
Re: [PATCH] drm/amd: include drm/drm_edid.h only where needed
Applied with some minor fixups for our -next tree. Thanks! Alex On Tue, Dec 12, 2023 at 9:10 AM Jani Nikula wrote: > > Including drm_edid.h from amdgpu_mode.h causes the rebuild of literally > hundreds of files when drm_edid.h is modified, while there are only a > handful of files that actually need to include drm_edid.h. > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c| 1 + > drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 1 + > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 1 + > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 1 + > 6 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > index 32fe05c810c6..3802ccdf6f55 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > @@ -32,7 +32,6 @@ > > #include > #include > -#include > #include > #include > #include > @@ -51,6 +50,7 @@ struct amdgpu_device; > struct amdgpu_encoder; > struct amdgpu_router; > struct amdgpu_hpd; > +struct edid; > > #define to_amdgpu_crtc(x) container_of(x, struct amdgpu_crtc, base) > #define to_amdgpu_connector(x) container_of(x, struct amdgpu_connector, base) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > index db6fc0cb18eb..453a4b786cfc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0+ > > #include > +#include > #include > #include > > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > index 3ee219aa2891..7672abe6c140 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > @@ -28,6 +28,7 @@ > > #include > > +#include > #include > #include "amdgpu.h" > #include "amdgpu_connectors.h" > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index bb666cb7522e..587ee632a3b8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -21,6 +21,7 @@ > * > */ > > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index 7af277f61cca..f22ec27365bd 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -21,6 +21,7 @@ > * > */ > > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index b599efda3b19..6f8128130b62 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > #include > #include "dm_services.h" > #include "amdgpu.h" > -- > 2.39.2 >
Re: [net-next v1 02/16] net: page_pool: create hooks for custom page providers
On Tue, Dec 12, 2023 at 12:07 AM Ilias Apalodimas
wrote:
>
> Hi Mina,
>
> Apologies for not participating in the party earlier.
>
No worries, thanks for looking.
> On Fri, 8 Dec 2023 at 02:52, Mina Almasry wrote:
> >
> > From: Jakub Kicinski
> >
> > The page providers which try to reuse the same pages will
> > need to hold onto the ref, even if page gets released from
> > the pool - as in releasing the page from the pp just transfers
> > the "ownership" reference from pp to the provider, and provider
> > will wait for other references to be gone before feeding this
> > page back into the pool.
> >
> > Signed-off-by: Jakub Kicinski
> > Signed-off-by: Mina Almasry
> >
> > ---
> >
> > This is implemented by Jakub in his RFC:
> > https://lore.kernel.org/netdev/f8270765-a27b-6ccf-33ea-cda097168...@redhat.com/T/
> >
> > I take no credit for the idea or implementation; I only added minor
> > edits to make this workable with device memory TCP, and removed some
> > hacky test code. This is a critical dependency of device memory TCP
> > and thus I'm pulling it into this series to make it revewable and
> > mergable.
> >
> > RFC v3 -> v1
> > - Removed unusued mem_provider. (Yunsheng).
> > - Replaced memory_provider & mp_priv with netdev_rx_queue (Jakub).
> >
> > ---
> > include/net/page_pool/types.h | 12 ++
> > net/core/page_pool.c | 43 +++
> > 2 files changed, 50 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> > index ac286ea8ce2d..0e9fa79a5ef1 100644
> > --- a/include/net/page_pool/types.h
> > +++ b/include/net/page_pool/types.h
> > @@ -51,6 +51,7 @@ struct pp_alloc_cache {
> > * @dev: device, for DMA pre-mapping purposes
> > * @netdev:netdev this pool will serve (leave as NULL if none or
> > multiple)
> > * @napi: NAPI which is the sole consumer of pages, otherwise NULL
> > + * @queue: struct netdev_rx_queue this page_pool is being created for.
> > * @dma_dir: DMA mapping direction
> > * @max_len: max DMA sync memory size for PP_FLAG_DMA_SYNC_DEV
> > * @offset:DMA sync address offset for PP_FLAG_DMA_SYNC_DEV
> > @@ -63,6 +64,7 @@ struct page_pool_params {
> > int nid;
> > struct device *dev;
> > struct napi_struct *napi;
> > + struct netdev_rx_queue *queue;
> > enum dma_data_direction dma_dir;
> > unsigned intmax_len;
> > unsigned intoffset;
> > @@ -125,6 +127,13 @@ struct page_pool_stats {
> > };
> > #endif
> >
> > +struct memory_provider_ops {
> > + int (*init)(struct page_pool *pool);
> > + void (*destroy)(struct page_pool *pool);
> > + struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
> > + bool (*release_page)(struct page_pool *pool, struct page *page);
> > +};
> > +
> > struct page_pool {
> > struct page_pool_params_fast p;
> >
> > @@ -174,6 +183,9 @@ struct page_pool {
> > */
> > struct ptr_ring ring;
> >
> > + void *mp_priv;
> > + const struct memory_provider_ops *mp_ops;
> > +
> > #ifdef CONFIG_PAGE_POOL_STATS
> > /* recycle stats are per-cpu to avoid locking */
> > struct page_pool_recycle_stats __percpu *recycle_stats;
> > diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> > index ca1b3b65c9b5..f5c84d2a4510 100644
> > --- a/net/core/page_pool.c
> > +++ b/net/core/page_pool.c
> > @@ -25,6 +25,8 @@
> >
> > #include "page_pool_priv.h"
> >
> > +static DEFINE_STATIC_KEY_FALSE(page_pool_mem_providers);
>
> We could add the existing page pool mechanisms as another 'provider',
> but I assume this is coded like this for performance reasons (IOW skip
> the expensive ptr call for the default case?)
>
Correct, it's done like this for performance reasons.
> > +
> > #define DEFER_TIME (msecs_to_jiffies(1000))
> > #define DEFER_WARN_INTERVAL (60 * HZ)
> >
> > @@ -174,6 +176,7 @@ static int page_pool_init(struct page_pool *pool,
> > const struct page_pool_params *params)
> > {
> > unsigned int ring_qsize = 1024; /* Default */
> > + int err;
> >
> > memcpy(>p, >fast, sizeof(pool->p));
> > memcpy(>slow, >slow, sizeof(pool->slow));
> > @@ -234,10 +237,25 @@ static int page_pool_init(struct page_pool *pool,
> > /* Driver calling page_pool_create() also call page_pool_destroy()
> > */
> > refcount_set(>user_cnt, 1);
> >
> > + if (pool->mp_ops) {
> > + err = pool->mp_ops->init(pool);
> > + if (err) {
> > + pr_warn("%s() mem-provider init failed %d\n",
> > + __func__, err);
> > + goto free_ptr_ring;
> > + }
> > +
> > + static_branch_inc(_pool_mem_providers);
> > + }
> > +
> > if (pool->p.flags &
Re: [PATCH] fbdev/clcd: Delete the old CLCD driver
On 12/12/23 15:18, Linus Walleij wrote:
We have managed to ascertain that all users of the old FBDEV
code that are out of tree are now gone.
The new DRM driver can be found in drivers/gpu/drm/pl111/.
The remaining out of tree user was the ARM FVP emulation
platform, running Android. Thanks to changes in Android
versions 13 and 14, Android can now use the DRM driver when
being emulated under FVP. Some final patches are being put
in place to make it fully featured.
This is essentially a revert of the partial revert in
commit 112c35237c72 ("Partially revert "video: fbdev: amba-clcd: Retire elder CLCD
driver"")
Signed-off-by: Linus Walleij
applied.
Thanks!
Helge
---
MAINTAINERS | 5 -
drivers/video/fbdev/Kconfig | 18 -
drivers/video/fbdev/Makefile| 1 -
drivers/video/fbdev/amba-clcd.c | 984
include/linux/amba/clcd-regs.h | 87
include/linux/amba/clcd.h | 290
6 files changed, 1385 deletions(-)
Re: [PATCH] drm/radeon: include drm/drm_edid.h only where needed
Applied. Thanks! Alex On Tue, Dec 12, 2023 at 9:10 AM Jani Nikula wrote: > > Including drm_edid.h from radeon_mode.h causes the rebuild of more than > a hundred files when drm_edid.h is modified, while there are only a > handful of files that actually need to include drm_edid.h. > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/radeon/atombios_encoders.c | 1 + > drivers/gpu/drm/radeon/dce3_1_afmt.c | 1 + > drivers/gpu/drm/radeon/dce6_afmt.c | 1 + > drivers/gpu/drm/radeon/evergreen.c | 1 + > drivers/gpu/drm/radeon/evergreen_hdmi.c| 1 + > drivers/gpu/drm/radeon/radeon_atombios.c | 1 + > drivers/gpu/drm/radeon/radeon_audio.c | 1 + > drivers/gpu/drm/radeon/radeon_audio.h | 4 +++- > drivers/gpu/drm/radeon/radeon_combios.c| 1 + > drivers/gpu/drm/radeon/radeon_encoders.c | 1 + > drivers/gpu/drm/radeon/radeon_mode.h | 2 +- > 11 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c > b/drivers/gpu/drm/radeon/atombios_encoders.c > index 4aca09cab4b8..6e537c5bd295 100644 > --- a/drivers/gpu/drm/radeon/atombios_encoders.c > +++ b/drivers/gpu/drm/radeon/atombios_encoders.c > @@ -29,6 +29,7 @@ > #include > > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/radeon/dce3_1_afmt.c > b/drivers/gpu/drm/radeon/dce3_1_afmt.c > index e8fe239b9d79..324e9b765098 100644 > --- a/drivers/gpu/drm/radeon/dce3_1_afmt.c > +++ b/drivers/gpu/drm/radeon/dce3_1_afmt.c > @@ -21,6 +21,7 @@ > * OTHER DEALINGS IN THE SOFTWARE. > */ > #include > +#include > > #include "radeon.h" > #include "radeon_asic.h" > diff --git a/drivers/gpu/drm/radeon/dce6_afmt.c > b/drivers/gpu/drm/radeon/dce6_afmt.c > index 4a1d5447eac1..4c06f47453fd 100644 > --- a/drivers/gpu/drm/radeon/dce6_afmt.c > +++ b/drivers/gpu/drm/radeon/dce6_afmt.c > @@ -21,6 +21,7 @@ > * > */ > #include > +#include > > #include "dce6_afmt.h" > #include "radeon.h" > diff --git a/drivers/gpu/drm/radeon/evergreen.c > b/drivers/gpu/drm/radeon/evergreen.c > index f0ae087be914..a424b86008b8 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -26,6 +26,7 @@ > #include > #include > > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c > b/drivers/gpu/drm/radeon/evergreen_hdmi.c > index 5f3078f8ab95..681119c91d94 100644 > --- a/drivers/gpu/drm/radeon/evergreen_hdmi.c > +++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c > @@ -26,6 +26,7 @@ > */ > #include > > +#include > #include > #include "evergreen_hdmi.h" > #include "radeon.h" > diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c > b/drivers/gpu/drm/radeon/radeon_atombios.c > index 85c4bb186203..3596ea4a8b60 100644 > --- a/drivers/gpu/drm/radeon/radeon_atombios.c > +++ b/drivers/gpu/drm/radeon/radeon_atombios.c > @@ -27,6 +27,7 @@ > #include > > #include > +#include > #include > > #include "radeon.h" > diff --git a/drivers/gpu/drm/radeon/radeon_audio.c > b/drivers/gpu/drm/radeon/radeon_audio.c > index 279bf130a18c..053058c5c1fa 100644 > --- a/drivers/gpu/drm/radeon/radeon_audio.c > +++ b/drivers/gpu/drm/radeon/radeon_audio.c > @@ -26,6 +26,7 @@ > #include > > #include > +#include > #include > #include "dce6_afmt.h" > #include "evergreen_hdmi.h" > diff --git a/drivers/gpu/drm/radeon/radeon_audio.h > b/drivers/gpu/drm/radeon/radeon_audio.h > index 05e67867469b..dacaaa007051 100644 > --- a/drivers/gpu/drm/radeon/radeon_audio.h > +++ b/drivers/gpu/drm/radeon/radeon_audio.h > @@ -27,7 +27,9 @@ > > #include > > -#define RREG32_ENDPOINT(block, reg)\ > +struct cea_sad; > + > +#define RREG32_ENDPOINT(block, reg)\ > radeon_audio_endpoint_rreg(rdev, (block), (reg)) > #define WREG32_ENDPOINT(block, reg, v) \ > radeon_audio_endpoint_wreg(rdev, (block), (reg), (v)) > diff --git a/drivers/gpu/drm/radeon/radeon_combios.c > b/drivers/gpu/drm/radeon/radeon_combios.c > index 2620efc7c675..6952b1273b0f 100644 > --- a/drivers/gpu/drm/radeon/radeon_combios.c > +++ b/drivers/gpu/drm/radeon/radeon_combios.c > @@ -28,6 +28,7 @@ > #include > > #include > +#include > #include > > #include "radeon.h" > diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c > b/drivers/gpu/drm/radeon/radeon_encoders.c > index 9cb6401fe97e..3de3dce9e89d 100644 > --- a/drivers/gpu/drm/radeon/radeon_encoders.c > +++ b/drivers/gpu/drm/radeon/radeon_encoders.c > @@ -26,6 +26,7 @@ > > #include > > +#include > #include > #include > > diff --git a/drivers/gpu/drm/radeon/radeon_mode.h > b/drivers/gpu/drm/radeon/radeon_mode.h > index 1decdcec0264..59c4db13d90a 100644 > --- a/drivers/gpu/drm/radeon/radeon_mode.h > +++ b/drivers/gpu/drm/radeon/radeon_mode.h > @@ -32,13 +32,13 @@ > > #include > #include > -#include > #include > #include > #include > #include > #include > > +struct edid; > struct
Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider
On Tue, Dec 12, 2023 at 06:26:51AM -0800, Mina Almasry wrote:
> On Tue, Dec 12, 2023 at 4:25 AM Jason Gunthorpe wrote:
> >
> > On Thu, Dec 07, 2023 at 04:52:39PM -0800, Mina Almasry wrote:
> >
> > > +static inline struct page_pool_iov *page_to_page_pool_iov(struct page
> > > *page)
> > > +{
> > > + if (page_is_page_pool_iov(page))
> > > + return (struct page_pool_iov *)((unsigned long)page &
> > > ~PP_IOV);
> > > +
> > > + DEBUG_NET_WARN_ON_ONCE(true);
> > > + return NULL;
> > > +}
> >
> > We already asked not to do this, please do not allocate weird things
> > can call them 'struct page' when they are not. It undermines the
> > maintainability of the mm to have things mis-typed like
> > this. Introduce a new type for your thing so the compiler can check it
> > properly.
> >
>
> There is a new type introduced, it's the page_pool_iov. We set the LSB
> on page_pool_iov* and cast it to page* only to avoid the churn of
> renaming page* to page_pool_iov* in the page_pool and all the net
> drivers using it. Is that not a reasonable compromise in your opinion?
> Since the LSB is set on the resulting page pointers, they are not
> actually usuable as pages, and are never passed to mm APIs per your
> requirement.
There were two asks, the one you did was to never pass this non-struct
page memory to the mm, which is great.
The other was to not mistype things, and don't type something as
struct page when it is, in fact, not.
I fear what you've done is make it so only one driver calls these
special functions and left the other drivers passing the struct page
directly to the mm and sort of obfuscating why it is OK based on this
netdev knowledge of not enabling/using the static branch in the other
cases.
Perhaps you can simply avoid this by arranging for this driver to also
exclusively use some special type to indicate the dual nature of the
pointer and leave the other drivers as using the struct page version.
Jason
[PATCH] ASoC: hdmi-codec: drop drm/drm_edid.h include
hdmi-codec.h does not appear to need drm/drm_edid.h for anything. Remove it. Signed-off-by: Jani Nikula --- I'm pretty sure I haven't compiled everything that might implicitly depend on the include. However, the right thing to do is to include drm_edid.h where needed, not from somewhat random intermediate headers. I hope this uncovers anything I missed. --- include/sound/hdmi-codec.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h index 9b162ac1e08e..5e1a9eafd10f 100644 --- a/include/sound/hdmi-codec.h +++ b/include/sound/hdmi-codec.h @@ -12,7 +12,6 @@ #include #include -#include #include #include #include -- 2.39.2
Re: [net-next v1 09/16] page_pool: device memory support
On Tue, Dec 12, 2023 at 3:17 AM Yunsheng Lin wrote:
>
> On 2023/12/12 2:14, Mina Almasry wrote:
> > On Mon, Dec 11, 2023 at 3:51 AM Yunsheng Lin wrote:
> >>
> >> On 2023/12/11 12:04, Mina Almasry wrote:
> >>> On Sun, Dec 10, 2023 at 6:26 PM Mina Almasry
> >>> wrote:
>
> On Sun, Dec 10, 2023 at 6:04 PM Yunsheng Lin
> wrote:
> >
> > On 2023/12/9 0:05, Mina Almasry wrote:
> >> On Fri, Dec 8, 2023 at 1:30 AM Yunsheng Lin
> >> wrote:
> >>>
> >>>
> >>> As mentioned before, it seems we need to have the above checking every
> >>> time we need to do some per-page handling in page_pool core, is there
> >>> a plan in your mind how to remove those kind of checking in the
> >>> future?
> >>>
> >>
> >> I see 2 ways to remove the checking, both infeasible:
> >>
> >> 1. Allocate a wrapper struct that pulls out all the fields the page
> >> pool needs:
> >>
> >> struct netmem {
> >> /* common fields */
> >> refcount_t refcount;
> >> bool is_pfmemalloc;
> >> int nid;
> >> ...
> >> union {
> >> struct dmabuf_genpool_chunk_owner *owner;
> >> struct page * page;
> >> };
> >> };
> >>
> >> The page pool can then not care if the underlying memory is iov or
> >> page. However this introduces significant memory bloat as this struct
> >> needs to be allocated for each page or ppiov, which I imagine is not
> >> acceptable for the upside of removing a few static_branch'd if
> >> statements with no performance cost.
> >>
> >> 2. Create a unified struct for page and dmabuf memory, which the mm
> >> folks have repeatedly nacked, and I imagine will repeatedly nack in
> >> the future.
> >>
> >> So I imagine the special handling of ppiov in some form is critical
> >> and the checking may not be removable.
> >
> > If the above is true, perhaps devmem is not really supposed to be
> > intergated
> > into page_pool.
> >
> > Adding a checking for every per-page handling in page_pool core is just
> > too
> > hacky to be really considerred a longterm solution.
> >
>
> The only other option is to implement another page_pool for ppiov and
> have the driver create page_pool or ppiov_pool depending on the state
> of the netdev_rx_queue (or some helper in the net stack to do that for
> the driver). This introduces some code duplication. The ppiov_pool &
> page_pool would look similar in implementation.
> >>
> >> I think there is a design pattern already to deal with this kind of
> >> problem,
> >> refactoring common code used by both page_pool and ppiov into a library to
> >> aovid code duplication if most of them have similar implementation.
> >>
> >
> > Code can be refactored if it's identical, not if it is similar. I
>
> Similarity indicates an opportunity to the refactor out the common
> code, like the page_frag case below:
> https://patchwork.kernel.org/project/netdevbpf/cover/20231205113444.63015-1-linyunsh...@huawei.com/
>
> But untill we do a proof of concept implemention, it is hard to tell if
> it is feasiable or not.
>
> > suspect the page_pools will be only similar, and if you're not willing
> > to take devmem handling into the page pool then refactoring page_pool
> > code into helpers that do devmem handling may also not be an option.
> >
>
> But this was all discussed in detail in RFC v2 and the last response I
> heard from Jesper was in favor if this approach, if I understand
> correctly:
>
> https://lore.kernel.org/netdev/7aedc5d5-0daf-63be-21bc-3b724cc1c...@redhat.com/
>
> Would love to have the maintainer weigh in here.
>
> >>>
> >>> I should note we may be able to remove some of the checking, but maybe
> >>> not all.
> >>>
> >>> - Checks that disable page fragging for ppiov can be removed once
> >>> ppiov has frag support (in this series or follow up).
> >>>
> >>> - If we use page->pp_frag_count (or page->pp_ref_count) for
> >>> refcounting ppiov, we can remove the if checking in the refcounting.
> >>>
> >
> > I'm not sure this is actually possible in the short term. The
> > page_pool uses both page->_refcount and page->pp_frag_count for
> > refcounting, and I will not be able to remove the special handling
> > around page->_refcount as i'm not allowed to call page_ref_*() APIs on
> > a non-struct page.
>
> the page_ref_*() API may be avoided using the below patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/20231113130041.58124-7-linyunsh...@huawei.com/
>
Even after the patch above, you're still calling page_ref_count() in
the page_pool to check for recycling, so after that patch you're still
using page->_refcount.
> But I am not sure how to do that for tx part if devmem for tx is not
> intergating into page_pool, that is why I suggest
Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider
On Tue, Dec 12, 2023 at 4:25 AM Jason Gunthorpe wrote:
>
> On Thu, Dec 07, 2023 at 04:52:39PM -0800, Mina Almasry wrote:
>
> > +static inline struct page_pool_iov *page_to_page_pool_iov(struct page
> > *page)
> > +{
> > + if (page_is_page_pool_iov(page))
> > + return (struct page_pool_iov *)((unsigned long)page &
> > ~PP_IOV);
> > +
> > + DEBUG_NET_WARN_ON_ONCE(true);
> > + return NULL;
> > +}
>
> We already asked not to do this, please do not allocate weird things
> can call them 'struct page' when they are not. It undermines the
> maintainability of the mm to have things mis-typed like
> this. Introduce a new type for your thing so the compiler can check it
> properly.
>
There is a new type introduced, it's the page_pool_iov. We set the LSB
on page_pool_iov* and cast it to page* only to avoid the churn of
renaming page* to page_pool_iov* in the page_pool and all the net
drivers using it. Is that not a reasonable compromise in your opinion?
Since the LSB is set on the resulting page pointers, they are not
actually usuable as pages, and are never passed to mm APIs per your
requirement.
--
Thanks,
Mina
[PATCH] drm/hisilicon: include drm/drm_edid.h only where needed
Reduce the need for rebuilds when drm_edid.h is modified by including it
only where needed.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 1 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index f957552c6c50..207aa3f660b0 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -18,7 +18,6 @@
#include
#include
-#include
#include
struct hibmc_connector {
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 8c6d2ea2a472..94e2c573a7af 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -14,6 +14,7 @@
#include
#include
+#include
#include
#include
#include
--
2.39.2
[PATCH] drm/tegra: include drm/drm_edid.h only where needed
Reduce the need for rebuilds when drm_edid.h is modified by including it
only where needed.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/tegra/drm.h| 2 +-
drivers/gpu/drm/tegra/output.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index ccb5d74fa227..682011166a8f 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -13,7 +13,6 @@
#include
#include
-#include
#include
#include
#include
@@ -26,6 +25,7 @@
/* XXX move to include/uapi/drm/drm_fourcc.h? */
#define DRM_FORMAT_MOD_NVIDIA_SECTOR_LAYOUT BIT_ULL(22)
+struct edid;
struct reset_control;
struct tegra_drm {
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index dc2dcb5ca1c8..88af956ad0e7 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
#include
--
2.39.2
