Re: [PATCH v12 1/5] drm: improve drm_buddy_alloc function
On 14/02/22 2:42 pm, Christian König wrote: > > > Am 14.02.22 um 09:36 schrieb Matthew Auld: >> On Mon, 14 Feb 2022 at 06:32, Christian König >> wrote: >>> Am 13.02.22 um 09:52 schrieb Arunpravin: - Make drm_buddy_alloc a single function to handle range allocation and non-range allocation demands - Implemented a new function alloc_range() which allocates the requested power-of-two block comply with range limitations - Moved order computation and memory alignment logic from i915 driver to drm buddy v2: merged below changes to keep the build unbroken - drm_buddy_alloc_range() becomes obsolete and may be removed - enable ttm range allocation (fpfn / lpfn) support in i915 driver - apply enhanced drm_buddy_alloc() function to i915 driver v3(Matthew Auld): - Fix alignment issues and remove unnecessary list_empty check - add more validation checks for input arguments - make alloc_range() block allocations as bottom-up - optimize order computation logic - replace uint64_t with u64, which is preferred in the kernel v4(Matthew Auld): - keep drm_buddy_alloc_range() function implementation for generic actual range allocations - keep alloc_range() implementation for end bias allocations v5(Matthew Auld): - modify drm_buddy_alloc() passing argument place->lpfn to lpfn as place->lpfn will currently always be zero for i915 v6(Matthew Auld): - fixup potential uaf - If we are unlucky and can't allocate enough memory when splitting blocks, where we temporarily end up with the given block and its buddy on the respective free list, then we need to ensure we delete both blocks, and no just the buddy, before potentially freeing them - fix warnings reported by kernel test robot v7(Matthew Auld): - revert fixup potential uaf - keep __alloc_range() add node to the list logic same as drm_buddy_alloc_blocks() by having a temporary list variable - at drm_buddy_alloc_blocks() keep i915 range_overflows macro and add a new check for end variable v8: - fix warnings reported by kernel test robot v9(Matthew Auld): - remove DRM_BUDDY_RANGE_ALLOCATION flag - remove unnecessary function description Signed-off-by: Arunpravin Reviewed-by: Matthew Auld >>> As long as nobody objects I'm going to push patches 1-3 to drm-misc-next >>> in the next hour or so: >> As part of this could you also push >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F99842%2F&data=04%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Cc50a2b13b2a0425e596f08d9ef9a2d60%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637804268194961068%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ENxu%2BSquLubBYLkNYV1SIUau1u7aZMdjz22izvv3FvM%3D&reserved=0 >> ? > > Sure, but Arun said in our internal chat that I should wait with that > anyway since he wanted to sort out one more issue. > > Christian. > working on 2 issues, 1. I think we need to keep DRM_BUDDY_RANGE_ALLOCATION flag, some corner case didnt allow amdgpu driver load 2. rebasing the existing amdgpu_vram_mgr.c and resolving all conflicts as there are many changes merged in with the below patch - drm/amdgpu: remove VRAM accounting v2 >> >>> Then going to take a deeper look into patches 4 and 5 to get them reviewed. >>> >>> Thanks, >>> Christian. >>> --- drivers/gpu/drm/drm_buddy.c | 292 +- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 63 ++-- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 2 + include/drm/drm_buddy.h | 11 +- 4 files changed, 250 insertions(+), 118 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index d60878bc9c20..e0c0d786a572 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -282,23 +282,97 @@ void drm_buddy_free_list(struct drm_buddy *mm, struct list_head *objects) } EXPORT_SYMBOL(drm_buddy_free_list); -/** - * drm_buddy_alloc_blocks - allocate power-of-two blocks - * - * @mm: DRM buddy manager to allocate from - * @order: size of the allocation - * - * The order value here translates to: - * - * 0 = 2^0 * mm->chunk_size - * 1 = 2^1 * mm->chunk_size - * 2 = 2^2 * mm->chunk_size - * - * Returns: - * allocated ptr to the &drm_buddy_block on success - */ -struct drm_buddy_block * -drm_buddy_alloc_blocks(struct drm_buddy *mm, unsigned int order) +static inline bool overlap
Re: [PATCH] drm: fb-helper: Avoid nesting spinlock_t into raw_spinlock_t
On 2022-02-15 20:59:24 [+0100], Jiri Kosina wrote: > Thanks for confirmation, seems like this problem is indeed cured by your > series. Oki. > I still believe though that we shouldn't let 5.17 released with this > warning being emitted into dmesg, so I'd suggest going with my patch for > mainline, and revert it in your series on top of it. No. That warning is only visible with CONFIG_PROVE_RAW_LOCK_NESTING with the following paragraph in its help: | NOTE: There are known nesting problems. So if you enable this | option expect lockdep splats until these problems have been fully | addressed which is work in progress. This config switch allows to | identify and analyze these problems. It will be removed and the | check permanently enabled once the main issues have been fixed. This warning in this call chain should affect every console driver which acquires a lock. > Thanks, Sebastian
Re: [Freedreno] [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On 2/15/2022 9:14 PM, Bjorn Andersson wrote:
On Tue 15 Feb 20:38 CST 2022, Abhinav Kumar wrote:
On 2/15/2022 6:14 PM, Bjorn Andersson wrote:
On Tue 15 Feb 11:42 CST 2022, Abhinav Kumar wrote:
On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
From: Rob Clark
Add SC8180x to the hardware catalog, for initial support for the
platform. Due to limitations in the DP driver only one of the four DP
interfaces is left enabled.
The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
is flagged appropriately to ensure widebus is disabled - for now.
Signed-off-by: Rob Clark
[bjorn: Reworked intf and irq definitions]
Signed-off-by: Bjorn Andersson
---
Changes since v1:
- Dropped widebus flag
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
drivers/gpu/drm/msm/msm_drv.c | 1 +
4 files changed, 132 insertions(+)
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 aa75991903a6..7ac0fe32df49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -90,6 +90,17 @@
BIT(MDP_INTF3_INTR) | \
BIT(MDP_INTF4_INTR))
+#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+ BIT(MDP_SSPP_TOP0_INTR2) | \
+ BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+ BIT(MDP_INTF0_INTR) | \
+ BIT(MDP_INTF1_INTR) | \
+ BIT(MDP_INTF2_INTR) | \
+ BIT(MDP_INTF3_INTR) | \
+ BIT(MDP_INTF4_INTR) | \
+ BIT(MDP_INTF5_INTR) | \
+ BIT(MDP_AD4_0_INTR) | \
+ BIT(MDP_AD4_1_INTR))
#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH 2048
@@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
.max_vdeci_exp = MAX_VERT_DECIMATION,
};
+static const struct dpu_caps sc8180x_dpu_caps = {
+ .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+ .max_mixer_blendstages = 0xb,
+ .qseed_type = DPU_SSPP_SCALER_QSEED3,
+ .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
+ .ubwc_version = DPU_HW_UBWC_VER_30,
+ .has_src_split = true,
+ .has_dim_layer = true,
+ .has_idle_pc = true,
+ .has_3d_merge = true,
+ .max_linewidth = 4096,
+ .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+ .max_hdeci_exp = MAX_HORZ_DECIMATION,
+ .max_vdeci_exp = MAX_VERT_DECIMATION,
+};
+
static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
@@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
},
};
+static const struct dpu_mdp_cfg sc8180x_mdp[] = {
+ {
+ .name = "top_0", .id = MDP_TOP,
+ .base = 0x0, .len = 0x45C,
+ .features = 0,
+ .highest_bank_bit = 0x3,
+ .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
+ .reg_off = 0x2AC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
+ .reg_off = 0x2B4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
+ .reg_off = 0x2BC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
+ .reg_off = 0x2C4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
+ .reg_off = 0x2AC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
+ .reg_off = 0x2B4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .reg_off = 0x2BC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .reg_off = 0x2C4, .bit_off = 8},
+ },
+};
+
static const struct dpu_mdp_cfg sm8250_mdp[] = {
{
.name = "top_0", .id = MDP_TOP,
@@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
+static const struct dpu_intf_cfg sc8180x_intf[] = {
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
+ INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 28, 29),
+ /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until
thi
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On Tue 15 Feb 20:38 CST 2022, Abhinav Kumar wrote:
>
>
> On 2/15/2022 6:14 PM, Bjorn Andersson wrote:
> > On Tue 15 Feb 11:42 CST 2022, Abhinav Kumar wrote:
> >
> > >
> > >
> > > On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
> > > > On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
> > > >
> > > > >
> > > > >
> > > > > On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
> > > > > > From: Rob Clark
> > > > > >
> > > > > > Add SC8180x to the hardware catalog, for initial support for the
> > > > > > platform. Due to limitations in the DP driver only one of the four
> > > > > > DP
> > > > > > interfaces is left enabled.
> > > > > >
> > > > > > The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag
> > > > > > and
> > > > > > the Windows-on-Snapdragon bootloader leaves the widebus bit set, so
> > > > > > this
> > > > > > is flagged appropriately to ensure widebus is disabled - for now.
> > > > > >
> > > > > > Signed-off-by: Rob Clark
> > > > > > [bjorn: Reworked intf and irq definitions]
> > > > > > Signed-off-by: Bjorn Andersson
> > > > > > ---
> > > > > >
> > > > > > Changes since v1:
> > > > > > - Dropped widebus flag
> > > > > >
> > > > > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129
> > > > > > ++
> > > > > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> > > > > > drivers/gpu/drm/msm/msm_drv.c | 1 +
> > > > > > 4 files changed, 132 insertions(+)
> > > > > >
> > > > > > 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 aa75991903a6..7ac0fe32df49 100644
> > > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > > > > @@ -90,6 +90,17 @@
> > > > > > BIT(MDP_INTF3_INTR) | \
> > > > > > BIT(MDP_INTF4_INTR))
> > > > > > +#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
> > > > > > + BIT(MDP_SSPP_TOP0_INTR2) | \
> > > > > > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> > > > > > + BIT(MDP_INTF0_INTR) | \
> > > > > > + BIT(MDP_INTF1_INTR) | \
> > > > > > + BIT(MDP_INTF2_INTR) | \
> > > > > > + BIT(MDP_INTF3_INTR) | \
> > > > > > + BIT(MDP_INTF4_INTR) | \
> > > > > > + BIT(MDP_INTF5_INTR) | \
> > > > > > + BIT(MDP_AD4_0_INTR) | \
> > > > > > + BIT(MDP_AD4_1_INTR))
> > > > > > #define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
> > > > > > #define DEFAULT_DPU_LINE_WIDTH 2048
> > > > > > @@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps =
> > > > > > {
> > > > > > .max_vdeci_exp = MAX_VERT_DECIMATION,
> > > > > > };
> > > > > > +static const struct dpu_caps sc8180x_dpu_caps = {
> > > > > > + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > > > > > + .max_mixer_blendstages = 0xb,
> > > > > > + .qseed_type = DPU_SSPP_SCALER_QSEED3,
> > > > > > + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
> > > > > > + .ubwc_version = DPU_HW_UBWC_VER_30,
> > > > > > + .has_src_split = true,
> > > > > > + .has_dim_layer = true,
> > > > > > + .has_idle_pc = true,
> > > > > > + .has_3d_merge = true,
> > > > > > + .max_linewidth = 4096,
> > > > > > + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> > > > > > + .max_hdeci_exp = MAX_HORZ_DECIMATION,
> > > > > > + .max_vdeci_exp = MAX_VERT_DECIMATION,
> > > > > > +};
> > > > > > +
> > > > > > static const struct dpu_caps sm8250_dpu_caps = {
> > > > > > .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > > > > > .max_mixer_blendstages = 0xb,
> > > > > > @@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] =
> > > > > > {
> > > > > > },
> > > > > > };
> > > > > > +static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> > > > > > + {
> > > > > > + .name = "top_0", .id = MDP_TOP,
> > > > > > + .base = 0x0, .len = 0x45C,
> > > > > > + .features = 0,
> > > > > > + .highest_bank_bit = 0x3,
> > > > > > + .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
> > > > > > + .reg_off = 0x2AC, .bit_off = 0},
> > > > > > + .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
> > > > > > + .reg_off = 0x2B4, .bit_off = 0},
> > > > > > + .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
> > > > > > + .reg_off = 0x2BC, .bit_off = 0},
> > > > > > + .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
> > > > > > + .reg_off = 0x2C4, .bit_off = 0},
> > > > > > + .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> > > > > > + .reg_off = 0x2AC, .bit_off = 8},
> > > > > > + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> > > > > > + .reg_off = 0x2B4, .bit_off = 8},
> > >
Re: Report in unix_stream_read_generic()
On Wed, Feb 16, 2022 at 01:17:03PM +0900, Byungchul Park wrote: > [7.013330] === > [7.013331] DEPT: Circular dependency has been detected. > [7.013332] 5.17.0-rc1-00014-gcf3441bb2012 #2 Tainted: GW > [7.01] --- > [7.013334] summary > [7.013334] --- > [7.013335] *** DEADLOCK *** > [7.013335] > [7.013335] context A > [7.013336] [S] (unknown)(&(&ei->socket.wq.wait)->dmap:0) > [7.013337] [W] __mutex_lock_common(&u->iolock:0) > [7.013338] [E] event(&(&ei->socket.wq.wait)->dmap:0) > [7.013340] > [7.013340] context B > [7.013341] [S] __raw_spin_lock(&u->lock:0) > [7.013342] [W] wait(&(&ei->socket.wq.wait)->dmap:0) > [7.013343] [E] spin_unlock(&u->lock:0) This seems unlikely to be real. We're surely not actually waiting while holding a spinlock; existing debug checks would catch it. > [7.013407] --- > [7.013407] context B's detail > [7.013408] --- > [7.013408] context B > [7.013409] [S] __raw_spin_lock(&u->lock:0) > [7.013410] [W] wait(&(&ei->socket.wq.wait)->dmap:0) > [7.013411] [E] spin_unlock(&u->lock:0) > [7.013412] > [7.013412] [S] __raw_spin_lock(&u->lock:0): > [7.013413] [] unix_stream_read_generic+0x6bf/0xb60 > [7.013416] stacktrace: > [7.013416] _raw_spin_lock+0x6e/0x90 > [7.013418] unix_stream_read_generic+0x6bf/0xb60 It would be helpful if you'd run this through scripts/decode_stacktrace.sh so we could see line numbers instead of hex offsets (which arene't much use without the binary kernel). > [7.013420] unix_stream_recvmsg+0x40/0x50 > [7.013422] sock_read_iter+0x85/0xd0 > [7.013424] new_sync_read+0x162/0x180 > [7.013426] vfs_read+0xf3/0x190 > [7.013428] ksys_read+0xa6/0xc0 > [7.013429] do_syscall_64+0x3a/0x90 > [7.013431] entry_SYSCALL_64_after_hwframe+0x44/0xae > [7.013433] > [7.013434] [W] wait(&(&ei->socket.wq.wait)->dmap:0): > [7.013434] [] prepare_to_wait+0x47/0xd0 ... this may be the source of confusion. Just because we prepare to wait doesn't mean we end up actually waiting. For example, look at unix_wait_for_peer(): prepare_to_wait_exclusive(&u->peer_wait, &wait, TASK_INTERRUPTIBLE); sched = !sock_flag(other, SOCK_DEAD) && !(other->sk_shutdown & RCV_SHUTDOWN) && unix_recvq_full(other); unix_state_unlock(other); if (sched) timeo = schedule_timeout(timeo); finish_wait(&u->peer_wait, &wait); We *prepare* to wait, *then* drop the lock, then actually schedule.
[PATCH] drm/i915/guc/slpc: Use wrapper for reading RP_STATE_CAP
This will ensure correct values for Gen12+ platforms.
Cc: Matt Roper
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 13b27b8ff74e..cf075e726699 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -6,6 +6,7 @@
#include "i915_drv.h"
#include "intel_guc_slpc.h"
#include "gt/intel_gt.h"
+#include "gt/intel_rps.h"
static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc)
{
@@ -574,10 +575,10 @@ static int slpc_use_fused_rp0(struct intel_guc_slpc *slpc)
static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
{
+ struct intel_rps *rps = &slpc_to_gt(slpc)->rps;
u32 rp_state_cap;
- rp_state_cap = intel_uncore_read(slpc_to_gt(slpc)->uncore,
-GEN6_RP_STATE_CAP);
+ rp_state_cap = intel_rps_read_state_cap(rps);
slpc->rp0_freq = REG_FIELD_GET(RP0_CAP_MASK, rp_state_cap) *
GT_FREQUENCY_MULTIPLIER;
--
2.34.0
Re: [PATCH] drm/msm/dpu: drop unused access macros
On 2/15/2022 6:53 AM, Dmitry Baryshkov wrote:
The access macros BLK_foo are not used by the code, drop them.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 16
1 file changed, 16 deletions(-)
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 cac0298aeb52..975ff3a4ae3d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -778,22 +778,6 @@ struct dpu_mdss_cfg {
unsigned long mdss_irqs;
};
-/*
- * Access Macros
- */
-#define BLK_MDP(s) ((s)->mdp)
-#define BLK_CTL(s) ((s)->ctl)
-#define BLK_VIG(s) ((s)->vig)
-#define BLK_RGB(s) ((s)->rgb)
-#define BLK_DMA(s) ((s)->dma)
-#define BLK_CURSOR(s) ((s)->cursor)
-#define BLK_MIXER(s) ((s)->mixer)
-#define BLK_PINGPONG(s) ((s)->pingpong)
-#define BLK_INTF(s) ((s)->intf)
-#define BLK_AD(s) ((s)->ad)
-#define BLK_DSPP(s) ((s)->dspp)
-#define BLK_MERGE3d(s) ((s)->merge_3d)
-
/**
* dpu_hw_catalog_init - dpu hardware catalog init API retrieves
* hardcoded target specific catalog information in config structure
Re: [PATCH v2 1/2] drm/msm/dpu: Add INTF_5 interrupts
On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
SC8180x has the eDP controller wired up to INTF_5, so add the interrupt
register block for this interface to the list.
Signed-off-by: Bjorn Andersson
Reviewed-by: Abhinav Kumar
---
Changes since v1:
- None
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 6 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index a77a5eaa78ad..dd2161e7bdb6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -23,6 +23,7 @@
#define MDP_INTF_2_OFF0x6B000
#define MDP_INTF_3_OFF0x6B800
#define MDP_INTF_4_OFF0x6C000
+#define MDP_INTF_5_OFF 0x6C800
#define MDP_AD4_0_OFF 0x7C000
#define MDP_AD4_1_OFF 0x7D000
#define MDP_AD4_INTR_EN_OFF 0x41c
@@ -93,6 +94,11 @@ static const struct dpu_intr_reg dpu_intr_set[] = {
MDP_INTF_4_OFF+INTF_INTR_EN,
MDP_INTF_4_OFF+INTF_INTR_STATUS
},
+ {
+ MDP_INTF_5_OFF+INTF_INTR_CLEAR,
+ MDP_INTF_5_OFF+INTF_INTR_EN,
+ MDP_INTF_5_OFF+INTF_INTR_STATUS
+ },
{
MDP_AD4_0_OFF + MDP_AD4_INTR_CLEAR_OFF,
MDP_AD4_0_OFF + MDP_AD4_INTR_EN_OFF,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
index 1ab75cccd145..37379966d8ec 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
@@ -22,6 +22,7 @@ enum dpu_hw_intr_reg {
MDP_INTF2_INTR,
MDP_INTF3_INTR,
MDP_INTF4_INTR,
+ MDP_INTF5_INTR,
MDP_AD4_0_INTR,
MDP_AD4_1_INTR,
MDP_INTF0_7xxx_INTR,
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On 2/15/2022 6:14 PM, Bjorn Andersson wrote:
On Tue 15 Feb 11:42 CST 2022, Abhinav Kumar wrote:
On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
From: Rob Clark
Add SC8180x to the hardware catalog, for initial support for the
platform. Due to limitations in the DP driver only one of the four DP
interfaces is left enabled.
The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
is flagged appropriately to ensure widebus is disabled - for now.
Signed-off-by: Rob Clark
[bjorn: Reworked intf and irq definitions]
Signed-off-by: Bjorn Andersson
---
Changes since v1:
- Dropped widebus flag
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
drivers/gpu/drm/msm/msm_drv.c | 1 +
4 files changed, 132 insertions(+)
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 aa75991903a6..7ac0fe32df49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -90,6 +90,17 @@
BIT(MDP_INTF3_INTR) | \
BIT(MDP_INTF4_INTR))
+#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+ BIT(MDP_SSPP_TOP0_INTR2) | \
+ BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+ BIT(MDP_INTF0_INTR) | \
+ BIT(MDP_INTF1_INTR) | \
+ BIT(MDP_INTF2_INTR) | \
+ BIT(MDP_INTF3_INTR) | \
+ BIT(MDP_INTF4_INTR) | \
+ BIT(MDP_INTF5_INTR) | \
+ BIT(MDP_AD4_0_INTR) | \
+ BIT(MDP_AD4_1_INTR))
#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH 2048
@@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
.max_vdeci_exp = MAX_VERT_DECIMATION,
};
+static const struct dpu_caps sc8180x_dpu_caps = {
+ .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+ .max_mixer_blendstages = 0xb,
+ .qseed_type = DPU_SSPP_SCALER_QSEED3,
+ .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
+ .ubwc_version = DPU_HW_UBWC_VER_30,
+ .has_src_split = true,
+ .has_dim_layer = true,
+ .has_idle_pc = true,
+ .has_3d_merge = true,
+ .max_linewidth = 4096,
+ .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+ .max_hdeci_exp = MAX_HORZ_DECIMATION,
+ .max_vdeci_exp = MAX_VERT_DECIMATION,
+};
+
static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
@@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
},
};
+static const struct dpu_mdp_cfg sc8180x_mdp[] = {
+ {
+ .name = "top_0", .id = MDP_TOP,
+ .base = 0x0, .len = 0x45C,
+ .features = 0,
+ .highest_bank_bit = 0x3,
+ .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
+ .reg_off = 0x2AC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
+ .reg_off = 0x2B4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
+ .reg_off = 0x2BC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
+ .reg_off = 0x2C4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
+ .reg_off = 0x2AC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
+ .reg_off = 0x2B4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .reg_off = 0x2BC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .reg_off = 0x2C4, .bit_off = 8},
+ },
+};
+
static const struct dpu_mdp_cfg sm8250_mdp[] = {
{
.name = "top_0", .id = MDP_TOP,
@@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
+static const struct dpu_intf_cfg sc8180x_intf[] = {
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
+ INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 28, 29),
+ /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until
this is supported */
+ INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On Wednesday, 16 February 2022 1:03:57 PM AEDT Jason Gunthorpe wrote: > On Wed, Feb 16, 2022 at 12:23:44PM +1100, Alistair Popple wrote: > > > Device private and device coherent pages are not marked with pte_devmap and > > they > > are backed by a struct page. The only way of inserting them is via > > migrate_vma. > > The refcount is decremented in zap_pte_range() on munmap() with special > > handling > > for device private pages. Looking at it again though I wonder if there is > > any > > special treatment required in zap_pte_range() for device coherent pages > > given > > they count as present pages. > > This is what I guessed, but we shouldn't be able to just drop > pte_devmap on these pages without any other work?? Granted it does > very little already.. Yes, I agree we need to check this more closely. For device private pages not having pte_devmap is fine, because they are non-present swap entries so they always get special handling in the swap entry paths but the same isn't true for coherent device pages. > I thought at least gup_fast needed to be touched or did this get > handled by scanning the page list after the fact? Right, for gup I think the only special handling required is to prevent pinning. I had assumed that check_and_migrate_movable_pages() would still get called for gup_fast but unless I've missed something I don't think it does. That means gup_fast could still pin movable and coherent pages. Technically that is ok for coherent pages, but it's undesirable. - Alistair > Jason >
Re: [PATCH v4 2/2] drm/msm/dp: rewrite dss_module_power to use bulk clock functions
Quoting Dmitry Baryshkov (2022-01-31 13:05:13)
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 094b39bfed8c..f16072f33cdb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -10,7 +10,6 @@
> #include
> #include
>
> -#include "dp_clk_util.h"
> #include "msm_drv.h"
>
> #define DP_LABEL "MDSS DP DISPLAY"
> @@ -106,6 +105,22 @@ struct dp_regulator_cfg {
> struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
> };
>
> +enum dss_clk_type {
> + DSS_CLK_AHB, /* no set rate. rate controlled through rpm */
> + DSS_CLK_PCLK,
> +};
> +
> +struct dss_clk {
> + enum dss_clk_type type;
> + unsigned long rate;
> +};
> +
> +struct dss_module_power {
> + unsigned int num_clk;
> + struct clk_bulk_data *clocks;
> + struct dss_clk *clk_config;
> +};
> +
> /**
> * struct dp_parser - DP parser's data exposed to clients
> *
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c
> b/drivers/gpu/drm/msm/dp/dp_power.c
> index b48b45e92bfa..318e1f8629cb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -105,59 +105,40 @@ static int dp_power_clk_init(struct dp_power_private
> *power)
> ctrl = &power->parser->mp[DP_CTRL_PM];
> stream = &power->parser->mp[DP_STREAM_PM];
>
> - rc = msm_dss_get_clk(dev, core->clk_config, core->num_clk);
> + rc = devm_clk_bulk_get(dev, core->num_clk, core->clocks);
Could we go further and devm_clk_bulk_get_all() and then either have
some new clk API that goes through the bulk list and finds the one named
something and hands over a pointer to it, think "clk_get() on top of
clk_bulk_data", or just get the clk again that you want to set a rate on
and have two pointers but otherwise it's a don't care. Then we wouldn't
need to do much at all here to store the rate settable clk and find it
in a loop.
> if (rc) {
> DRM_ERROR("failed to get %s clk. err=%d\n",
> dp_parser_pm_name(DP_CORE_PM), rc);
> return rc;
> }
>
> - rc = msm_dss_get_clk(dev, ctrl->clk_config, ctrl->num_clk);
> + rc = devm_clk_bulk_get(dev, ctrl->num_clk, ctrl->clocks);
> if (rc) {
> DRM_ERROR("failed to get %s clk. err=%d\n",
> dp_parser_pm_name(DP_CTRL_PM), rc);
> - msm_dss_put_clk(core->clk_config, core->num_clk);
> return -ENODEV;
> }
>
> - rc = msm_dss_get_clk(dev, stream->clk_config, stream->num_clk);
> + rc = devm_clk_bulk_get(dev, stream->num_clk, stream->clocks);
> if (rc) {
> DRM_ERROR("failed to get %s clk. err=%d\n",
> dp_parser_pm_name(DP_CTRL_PM), rc);
> - msm_dss_put_clk(core->clk_config, core->num_clk);
> return -ENODEV;
> }
>
> return 0;
> }
>
> -static int dp_power_clk_deinit(struct dp_power_private *power)
> -{
> - struct dss_module_power *core, *ctrl, *stream;
> -
> - core = &power->parser->mp[DP_CORE_PM];
> - ctrl = &power->parser->mp[DP_CTRL_PM];
> - stream = &power->parser->mp[DP_STREAM_PM];
> -
> - if (!core || !ctrl || !stream) {
> - DRM_ERROR("invalid power_data\n");
> - return -EINVAL;
> - }
> -
> - msm_dss_put_clk(ctrl->clk_config, ctrl->num_clk);
> - msm_dss_put_clk(core->clk_config, core->num_clk);
> - msm_dss_put_clk(stream->clk_config, stream->num_clk);
> - return 0;
> -}
> -
> static int dp_power_clk_set_link_rate(struct dp_power_private *power,
> - struct dss_clk *clk_arry, int num_clk, int enable)
> + struct dss_module_power *mp, int enable)
> {
> + struct dss_clk *clk_arry = mp->clk_config;
> + int num_clk = mp->num_clk;
> u32 rate;
> int i, rc = 0;
>
> for (i = 0; i < num_clk; i++) {
> - if (clk_arry[i].clk) {
> + if (mp->clocks[i].clk) {
> if (clk_arry[i].type == DSS_CLK_PCLK) {
> if (enable)
> rate = clk_arry[i].rate;
> @@ -168,9 +149,49 @@ static int dp_power_clk_set_link_rate(struct
> dp_power_private *power,
> if (rc)
> break;
> }
> + } else {
> + DRM_ERROR("%pS->%s: '%s' is not available\n",
> + __builtin_return_address(0), __func__,
> + mp->clocks[i].id);
> + rc = -EPERM;
> + break;
> + }
> + }
> + return rc;
> +}
> +
> +static int dp_clk_set_rate(struct dss_module_power *mp)
> +{
> + struct dss_clk *clk_arry = mp->clk_config;
> + int num_clk = mp->num_
Re: [PATCH 6/6] drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq()
On 2/1/2022 7:10 AM, Dmitry Baryshkov wrote:
Pass IRQ number directly rather than passing an index in the dpu_encoder's irq
table.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +--
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 +--
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 9 --
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 12
5 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 83b6715820fa..4c9e7c4fa14b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -260,38 +260,35 @@ static int dpu_encoder_helper_wait_event_timeout(int32_t
drm_id,
u32 irq_idx, struct dpu_encoder_wait_info *info);
int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx,
+ int irq,
void (*func)(void *arg, int irq_idx),
struct dpu_encoder_wait_info *wait_info)
{
- int irq;
u32 irq_status;
int ret;
- if (!wait_info || intr_idx >= INTR_IDX_MAX) {
+ if (!wait_info) {
DPU_ERROR("invalid params\n");
return -EINVAL;
}
- irq = phys_enc->irq[intr_idx];
-
/* note: do master / slave checking outside */
/* return EWOULDBLOCK since we know the wait isn't necessary */
if (phys_enc->enable_state == DPU_ENC_DISABLED) {
- DRM_ERROR("encoder is disabled id=%u, intr=%d, irq=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_ERROR("encoder is disabled id=%u, callback=%ps, irq=%d\n",
+ DRMID(phys_enc->parent), func,
irq);
return -EWOULDBLOCK;
}
if (irq < 0) {
- DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d\n",
- DRMID(phys_enc->parent), intr_idx);
+ DRM_DEBUG_KMS("skip irq wait id=%u, callback=%ps\n",
+ DRMID(phys_enc->parent), func);
return 0;
}
- DRM_DEBUG_KMS("id=%u, intr=%d, irq=%d, pp=%d, pending_cnt=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_DEBUG_KMS("id=%u, callback=%ps, irq=%d, pp=%d, pending_cnt=%d\n",
+ DRMID(phys_enc->parent), func,
irq, phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
@@ -305,8 +302,8 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
if (irq_status) {
unsigned long flags;
- DRM_DEBUG_KMS("irq not triggered id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_DEBUG_KMS("irq not triggered id=%u, callback=%ps,
irq=%d, pp=%d, atomic_cnt=%d\n",
+ DRMID(phys_enc->parent), func,
irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
@@ -316,8 +313,8 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys
*phys_enc,
ret = 0;
} else {
ret = -ETIMEDOUT;
- DRM_DEBUG_KMS("irq timeout id=%u, intr=%d, irq=%d, pp=%d,
atomic_cnt=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_DEBUG_KMS("irq timeout id=%u, callback=%ps, irq=%d,
pp=%d, atomic_cnt=%d\n",
+ DRMID(phys_enc->parent), func,
irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
@@ -325,7 +322,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys
*phys_enc,
} else {
ret = 0;
trace_dpu_enc_irq_wait_success(DRMID(phys_enc->parent),
- intr_idx, irq,
+ func, irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
}
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 803fd6f25da1..9843acdc33bd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,13 +341,13 @@ void dpu_encoder_helper_report_irq_timeout(struct
dpu_encoder_phys *phys_enc,
* dpu_encoder_helper_wait_for_irq - utility to wait on
Re: [PATCH v4 1/2] drm/msm/dpu: simplify clocks handling
Quoting Dmitry Baryshkov (2022-01-31 13:05:12)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 60fe06018581..4d184122d63e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -405,10 +394,11 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>
> trace_dpu_core_perf_update_clk(kms->dev, stop_req, clk_rate);
>
> - ret = _dpu_core_perf_set_core_clk_rate(kms, clk_rate);
> + if (clk_rate > kms->perf.max_core_clk_rate)
> + clk_rate = kms->perf.max_core_clk_rate;
Using
clk_rate = min(clk_rate, kms->perf.max_core_clk_rate)
would be type safer. And min_t() would be explicit if the types don't
match, but we should try to make them be compatible.
> + ret = dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate);
> if (ret) {
> - DPU_ERROR("failed to set %s clock rate %llu\n",
> - kms->perf.core_clk->clk_name,
> clk_rate);
> + DPU_ERROR("failed to set core clock rate %llu\n",
> clk_rate);
> return ret;
> }
>
> @@ -529,13 +519,13 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
> int dpu_core_perf_init(struct dpu_core_perf *perf,
> struct drm_device *dev,
> struct dpu_mdss_cfg *catalog,
> - struct dss_clk *core_clk)
> + struct clk *core_clk)
> {
> perf->dev = dev;
> perf->catalog = catalog;
> perf->core_clk = core_clk;
>
> - perf->max_core_clk_rate = core_clk->max_rate;
> + perf->max_core_clk_rate = clk_get_rate(core_clk);
> if (!perf->max_core_clk_rate) {
> DPU_DEBUG("optional max core clk rate, use default\n");
> perf->max_core_clk_rate = DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 2d385b4b7f5e..5f562413bb63 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -21,7 +21,6 @@
> #include "dpu_hw_lm.h"
> #include "dpu_hw_interrupts.h"
> #include "dpu_hw_top.h"
> -#include "dpu_io_util.h"
> #include "dpu_rm.h"
> #include "dpu_core_perf.h"
>
> @@ -113,7 +112,8 @@ struct dpu_kms {
> struct platform_device *pdev;
> bool rpm_enabled;
>
> - struct dss_module_power mp;
> + struct clk_bulk_data *clocks;
> + int num_clocks;
size_t?
>
> /* reference count bandwidth requests, so we know when we can
> * release bandwidth. Each atomic update increments, and frame-
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 131c1f1a869c..8c038416e119 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -29,7 +29,8 @@ struct dpu_irq_controller {
> struct dpu_mdss {
> struct msm_mdss base;
> void __iomem *mmio;
> - struct dss_module_power mp;
> + struct clk_bulk_data *clocks;
> + int num_clocks;
size_t?
> struct dpu_irq_controller irq_controller;
> };
>
Re: [PATCH] drm/msm: populate intf_audio_select() base on hardware capability
Quoting Kuogee Hsieh (2022-02-11 15:23:42) > intf_audio_select() callback function use to configure > HDMI_DP_CORE_SELECT to decide audio output routes to HDMI or DP > interface. HDMI is obsoleted at newer chipset. To keep supporting > legacy hdmi application, intf_audio_select call back function have > to be populated base on hardware chip capability where legacy > chipsets have has_audio_select flag set to true. > > Signed-off-by: Kuogee Hsieh > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3] drm/msm/dpu: Only create debugfs for PRIMARY minor
Quoting Dmitry Baryshkov (2022-02-11 16:38:11) > From: Bjorn Andersson > > dpu_kms_debugfs_init() is invoked for each minor being registered. Most > of the files created are unrelated to the minor, so there's no reason to > present them per minor. > The exception to this is the DisplayPort code, which ends up invoking > dp_debug_get() for each minor, each time associate the allocated object > with dp->debug. > > As such dp_debug will create debugfs files in both the PRIMARY and the > RENDER minor's debugfs directory, but only the last reference will be > remembered. > > The only use of this reference today is in the cleanup path in > dp_display_deinit_sub_modules() and the dp_debug_private object does > outlive the debugfs entries in either case, so there doesn't seem to be > any adverse effects of this, but per the code the current behavior is > unexpected, so change it to only create debugfs files for the PRIMARY > minor. > > Signed-off-by: Bjorn Andersson > [DB: slightly change description and in-patch comment] > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH 6/6] drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq()
Quoting Dmitry Baryshkov (2022-02-01 07:10:56) > Pass IRQ number directly rather than passing an index in the dpu_encoder's > irq table. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH 5/6] drm/msm/dpu: remove struct dpu_encoder_irq
Quoting Dmitry Baryshkov (2022-02-01 07:10:55)
> 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 ff2218155b44..803fd6f25da1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -231,7 +216,7 @@ struct dpu_encoder_phys {
> atomic_t pending_ctlstart_cnt;
> atomic_t pending_kickoff_cnt;
> wait_queue_head_t pending_kickoff_wq;
> - struct dpu_encoder_irq irq[INTR_IDX_MAX];
> + int irq[INTR_IDX_MAX];
> };
>
> static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
> @@ -729,20 +727,8 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
> phys_enc->split_role = p->split_role;
> phys_enc->intf_mode = INTF_MODE_VIDEO;
> phys_enc->enc_spinlock = p->enc_spinlock;
> - for (i = 0; i < INTR_IDX_MAX; i++) {
> - irq = &phys_enc->irq[i];
> - irq->irq_idx = -EINVAL;
> - }
> -
> - irq = &phys_enc->irq[INTR_IDX_VSYNC];
> - irq->name = "vsync_irq";
> - irq->intr_idx = INTR_IDX_VSYNC;
> - irq->func = dpu_encoder_phys_vid_vblank_irq;
> -
> - irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
> - irq->name = "underrun";
> - irq->intr_idx = INTR_IDX_UNDERRUN;
> - irq->func = dpu_encoder_phys_vid_underrun_irq;
> + for (i = 0; i < INTR_IDX_MAX; i++)
Can this use ARRAY_SIZE(phys_enc->irq)? Safer that way.
> + phys_enc->irq[i] = -EINVAL;
>
> atomic_set(&phys_enc->vblank_refcount, 0);
> atomic_set(&phys_enc->pending_kickoff_cnt, 0);
Re: [PATCH 4/6] drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq
Quoting Dmitry Baryshkov (2022-02-01 07:10:54) > Get rid of dpu_encoder_helper_register_irq/unregister_irq helpers, call > dpu_core_register/unregister_callback directly, without surrounding them > with helpers. > > Signed-off-by: Dmitry Baryshkov > Reviewed-by: Abhinav Kumar > --- Reviewed-by: Stephen Boyd
Re: [PATCH 2/6] drm/msm/dpu: remove always-true argument of dpu_core_irq_read()
Quoting Dmitry Baryshkov (2022-02-01 07:10:52) > The argument clear of the function dpu_core_irq_read() is always true. > Remove it. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH 1/6] drm/msm/dpu: remove extra wrappers around dpu_core_irq
Quoting Dmitry Baryshkov (2022-02-01 07:10:51) > Remove extra dpu_irq_* wrappers from dpu_kms.c, merge them directly into > dpu_core_irq_* functions. > > Signed-off-by: Dmitry Baryshkov > Reviewed-by: Abhinav Kumar > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 7/8] drm/msm/dpu: pull connector from dpu_encoder_phys to dpu_encoder_virt
Quoting Dmitry Baryshkov (2022-02-15 18:13:09) > On Wed, 16 Feb 2022 at 05:00, Stephen Boyd wrote: > > > > Quoting Dmitry Baryshkov (2022-02-15 06:16:42) > > > All physical encoders used by virtual encoder share the same connector, > > > so pull the connector field from dpu_encoder_phys into dpu_encoder_virt > > > structure. > > > > What is the benefit? Less code? Clearer association? > > Clearer association. Otherwise code suggests that different phys_encs > can have different connectors. Got it. With that info added to the commit text Reviewed-by: Stephen Boyd
Re: [Freedreno] [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On 2/15/2022 6:03 PM, Bjorn Andersson wrote: On Tue 15 Feb 19:34 CST 2022, Abhinav Kumar wrote: On 2/15/2022 4:20 PM, Dmitry Baryshkov wrote: On Tue, 15 Feb 2022 at 23:21, Abhinav Kumar wrote: On 2/15/2022 10:42 AM, Dmitry Baryshkov wrote: On Tue, 15 Feb 2022 at 20:42, Abhinav Kumar wrote: On 2/15/2022 9:28 AM, Bjorn Andersson wrote: On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote: On 2/14/2022 8:33 PM, Bjorn Andersson wrote: From: Rob Clark [..] (thus leading us to cases when someone would forget to add INTF_EDP next to INTF_DP) Also, if we are switching from INTF_DP to INTF_EDP, should we stop using end-to-end numbering (like MSM_DP_CONTROLLER_2 for INTF_5) and add a separate numbering scheme for INTF_EDP? We should change the controller ID to match what it actually is. Now that you pointed this out, this looks even more confusing to me to say that MSM_DP_CONTROLLER_2 is actually a EDP controller because fundamentally and even hardware block wise they are different. So, do we split msm_priv->dp too? It's indexed using MSM_DP_CONTROLLER_n entries. Do we want to teach drm/msm/dp code that there are priv->dp[] and priv->edp arrays? ok so now priv->dp and priv->edp arrays are also in the picture here :) Actually all these questions should have probably come when we were figuring out how best to re-use eDP and DP driver. Either way atleast, its good we are documenting all these questions on this thread so that anyone can refer this to know what all was missed out :) priv->dp is of type msm_dp. When re-using DP driver for eDP and since struct msm_dp is the shared struct between dpu and the msm/dp, I get your point of re-using MSM_DP_CONTROLLER_* as thats being use to index. So MSM_DP_CONTROLLER_* is more of an index into the DP driver and not really a hardware indexing scheme. If we split into two arrays, we need more changes to dpu_encoder too. Too instrusive a change at this point, even though probably correct. I'm sorry, but performing such a split would create a whole bunch of duplication and I don't see the reasons yet. Can you please give me an example of when the DPU _code_ would benefit from being specifically written for EDP vs DP? Things where it doesn't make sense to enable certain features in runtime - but really have different implementation for the two interface types. Like I have mentioned in my previous comment, this would be a big change and I am also not in favor of this big change. But are you seeing more changes required even if we just change INTF_DP to INTF_eDP for the eDP entries? What are the challenges there? What are the benefits? In terms of current code, again like I said before in my previous comments several times I do not have an example. I was keeping the separation in case in future for some features we do need to differentiate eDP and DP. Somehow I also feel this change and below are interlinked that way. https://patchwork.freedesktop.org/patch/473871/ The only reason we need this change is because both eDP and DP use DRM_MODE_ENCODER_TMDS and specifying the intf_type directly will clear the confusion because DRM_MODE_ENCODER_DSI means DSI and DRM_MODE_ENCODER_VIRTUAL means Writeback but DRM_MODE_ENCODER_TMDS can mean DP OR eDP interface. The ambiguity was always for eDP and DP. That led to the discussion about the INTF_* we are specifying in the dpu_hw_catalog only to find the discrepancy. So now by clearing that ambiguity that change makes sense. That discussion trickled into this one. Regards, Bjorn
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On Tue 15 Feb 11:42 CST 2022, Abhinav Kumar wrote:
>
>
> On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
> > On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
> >
> > >
> > >
> > > On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
> > > > From: Rob Clark
> > > >
> > > > Add SC8180x to the hardware catalog, for initial support for the
> > > > platform. Due to limitations in the DP driver only one of the four DP
> > > > interfaces is left enabled.
> > > >
> > > > The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
> > > > the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
> > > > is flagged appropriately to ensure widebus is disabled - for now.
> > > >
> > > > Signed-off-by: Rob Clark
> > > > [bjorn: Reworked intf and irq definitions]
> > > > Signed-off-by: Bjorn Andersson
> > > > ---
> > > >
> > > > Changes since v1:
> > > > - Dropped widebus flag
> > > >
> > > >.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129
> > > > ++
> > > >.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> > > >drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> > > >drivers/gpu/drm/msm/msm_drv.c | 1 +
> > > >4 files changed, 132 insertions(+)
> > > >
> > > > 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 aa75991903a6..7ac0fe32df49 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > > @@ -90,6 +90,17 @@
> > > > BIT(MDP_INTF3_INTR) | \
> > > > BIT(MDP_INTF4_INTR))
> > > > +#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
> > > > + BIT(MDP_SSPP_TOP0_INTR2) | \
> > > > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> > > > + BIT(MDP_INTF0_INTR) | \
> > > > + BIT(MDP_INTF1_INTR) | \
> > > > + BIT(MDP_INTF2_INTR) | \
> > > > + BIT(MDP_INTF3_INTR) | \
> > > > + BIT(MDP_INTF4_INTR) | \
> > > > + BIT(MDP_INTF5_INTR) | \
> > > > + BIT(MDP_AD4_0_INTR) | \
> > > > + BIT(MDP_AD4_1_INTR))
> > > >#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
> > > >#define DEFAULT_DPU_LINE_WIDTH 2048
> > > > @@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
> > > > .max_vdeci_exp = MAX_VERT_DECIMATION,
> > > >};
> > > > +static const struct dpu_caps sc8180x_dpu_caps = {
> > > > + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > > > + .max_mixer_blendstages = 0xb,
> > > > + .qseed_type = DPU_SSPP_SCALER_QSEED3,
> > > > + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
> > > > + .ubwc_version = DPU_HW_UBWC_VER_30,
> > > > + .has_src_split = true,
> > > > + .has_dim_layer = true,
> > > > + .has_idle_pc = true,
> > > > + .has_3d_merge = true,
> > > > + .max_linewidth = 4096,
> > > > + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> > > > + .max_hdeci_exp = MAX_HORZ_DECIMATION,
> > > > + .max_vdeci_exp = MAX_VERT_DECIMATION,
> > > > +};
> > > > +
> > > >static const struct dpu_caps sm8250_dpu_caps = {
> > > > .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > > > .max_mixer_blendstages = 0xb,
> > > > @@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
> > > > },
> > > >};
> > > > +static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> > > > + {
> > > > + .name = "top_0", .id = MDP_TOP,
> > > > + .base = 0x0, .len = 0x45C,
> > > > + .features = 0,
> > > > + .highest_bank_bit = 0x3,
> > > > + .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
> > > > + .reg_off = 0x2AC, .bit_off = 0},
> > > > + .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
> > > > + .reg_off = 0x2B4, .bit_off = 0},
> > > > + .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
> > > > + .reg_off = 0x2BC, .bit_off = 0},
> > > > + .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
> > > > + .reg_off = 0x2C4, .bit_off = 0},
> > > > + .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> > > > + .reg_off = 0x2AC, .bit_off = 8},
> > > > + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> > > > + .reg_off = 0x2B4, .bit_off = 8},
> > > > + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> > > > + .reg_off = 0x2BC, .bit_off = 8},
> > > > + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> > > > + .reg_off = 0x2C4, .bit_off = 8},
> > > > + },
> > > > +};
> > > > +
> > > >static const struct dpu_mdp_cfg sm8250_mdp[] = {
> > > > {
> > > > .name = "top_0", .id = MDP_TOP,
> > > > @@ -861,6 +913,16 @@ stat
Re: [PATCH v2 7/8] drm/msm/dpu: pull connector from dpu_encoder_phys to dpu_encoder_virt
On Wed, 16 Feb 2022 at 05:00, Stephen Boyd wrote: > > Quoting Dmitry Baryshkov (2022-02-15 06:16:42) > > All physical encoders used by virtual encoder share the same connector, > > so pull the connector field from dpu_encoder_phys into dpu_encoder_virt > > structure. > > What is the benefit? Less code? Clearer association? Clearer association. Otherwise code suggests that different phys_encs can have different connectors. -- With best wishes Dmitry
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On Wed, Feb 16, 2022 at 12:23:44PM +1100, Alistair Popple wrote: > Device private and device coherent pages are not marked with pte_devmap and > they > are backed by a struct page. The only way of inserting them is via > migrate_vma. > The refcount is decremented in zap_pte_range() on munmap() with special > handling > for device private pages. Looking at it again though I wonder if there is any > special treatment required in zap_pte_range() for device coherent pages given > they count as present pages. This is what I guessed, but we shouldn't be able to just drop pte_devmap on these pages without any other work?? Granted it does very little already.. I thought at least gup_fast needed to be touched or did this get handled by scanning the page list after the fact? Jason
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On Tue 15 Feb 19:34 CST 2022, Abhinav Kumar wrote: > > > On 2/15/2022 4:20 PM, Dmitry Baryshkov wrote: > > On Tue, 15 Feb 2022 at 23:21, Abhinav Kumar > > wrote: > > > On 2/15/2022 10:42 AM, Dmitry Baryshkov wrote: > > > > On Tue, 15 Feb 2022 at 20:42, Abhinav Kumar > > > > wrote: > > > > > On 2/15/2022 9:28 AM, Bjorn Andersson wrote: > > > > > > On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote: > > > > > > > On 2/14/2022 8:33 PM, Bjorn Andersson wrote: > > > > > > > > From: Rob Clark [..] > > > > (thus leading us to cases when someone would forget to add INTF_EDP > > > > next to INTF_DP) > > > > > > > > Also, if we are switching from INTF_DP to INTF_EDP, should we stop > > > > using end-to-end numbering (like MSM_DP_CONTROLLER_2 for INTF_5) and > > > > add a separate numbering scheme for INTF_EDP? > > > > > > > We should change the controller ID to match what it actually is. > > > > > > Now that you pointed this out, this looks even more confusing to me to > > > say that MSM_DP_CONTROLLER_2 is actually a EDP controller because > > > fundamentally and even hardware block wise they are different. > > > > So, do we split msm_priv->dp too? It's indexed using > > MSM_DP_CONTROLLER_n entries. > > Do we want to teach drm/msm/dp code that there are priv->dp[] and > > priv->edp arrays? > > ok so now priv->dp and priv->edp arrays are also in the picture here :) > > Actually all these questions should have probably come when we were figuring > out how best to re-use eDP and DP driver. > > Either way atleast, its good we are documenting all these questions on this > thread so that anyone can refer this to know what all was missed out :) > > priv->dp is of type msm_dp. When re-using DP driver for eDP and since > struct msm_dp is the shared struct between dpu and the msm/dp, I get your > point of re-using MSM_DP_CONTROLLER_* as thats being use to index. > > So MSM_DP_CONTROLLER_* is more of an index into the DP driver and not really > a hardware indexing scheme. > > If we split into two arrays, we need more changes to dpu_encoder too. > > Too instrusive a change at this point, even though probably correct. > I'm sorry, but performing such a split would create a whole bunch of duplication and I don't see the reasons yet. Can you please give me an example of when the DPU _code_ would benefit from being specifically written for EDP vs DP? Things where it doesn't make sense to enable certain features in runtime - but really have different implementation for the two interface types. > But are you seeing more changes required even if we just change INTF_DP to > INTF_eDP for the eDP entries? What are the challenges there? > What are the benefits? Regards, Bjorn
Re: [PATCH] drm/msm/dpu: drop unused access macros
Quoting Dmitry Baryshkov (2022-02-15 06:53:06) > The access macros BLK_foo are not used by the code, drop them. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On Tue, Feb 15, 2022 at 05:49:07PM -0500, Felix Kuehling wrote:
> > Userspace does
> > 1) mmap(MAP_PRIVATE) to allocate anon memory
> > 2) something to trigger migration to install a ZONE_DEVICE page
> > 3) munmap()
> >
> > Who decrements the refcout on the munmap?
> >
> > When a ZONE_DEVICE page is installed in the PTE is supposed to be
> > marked as pte_devmap and that disables all the normal page refcounting
> > during munmap().
> >
> > fsdax makes this work by working the refcounts backwards, the page is
> > refcounted while it exists in the driver, when the driver decides to
> > remove it then unmap_mapping_range() is called to purge it from all
> > PTEs and then refcount is decrd. munmap/fork/etc don't change the
> > refcount.
>
> Hmm, that just means, whether or not there are PTEs doesn't really
> matter.
Yes, that is the FSDAX model
> It should work the same as it does for DEVICE_PRIVATE pages. I'm not sure
> where DEVICE_PRIVATE page's refcounts are decremented on unmap, TBH. But I
> can't find it in our driver, or in the test_hmm driver for that matter.
It is not the same as DEVICE_PRIVATE because DEVICE_PRIVATE uses swap
entries. The put_page for that case is here:
static unsigned long zap_pte_range(struct mmu_gather *tlb,
struct vm_area_struct *vma, pmd_t *pmd,
unsigned long addr, unsigned long end,
struct zap_details *details)
{
[..]
if (is_device_private_entry(entry) ||
is_device_exclusive_entry(entry)) {
struct page *page = pfn_swap_entry_to_page(entry);
if (unlikely(zap_skip_check_mapping(details, page)))
continue;
pte_clear_not_present_full(mm, addr, pte, tlb->fullmm);
rss[mm_counter(page)]--;
if (is_device_private_entry(entry))
page_remove_rmap(page, false);
put_page(page);
However the devmap case will return NULL from vm_normal_page() and won't
do the put_page() embedded inside the __tlb_remove_page() in the
pte_present() block in the same function.
After reflecting for awhile, I think Christoph's idea is quite
good. Just make it so you don't set pte_devmap() on your pages and
then lets avoid pte_devmap for all refcount correct ZONE_DEVICE pages.
Jason
Re: [PATCH v2 7/8] drm/msm/dpu: pull connector from dpu_encoder_phys to dpu_encoder_virt
Quoting Dmitry Baryshkov (2022-02-15 06:16:42) > All physical encoders used by virtual encoder share the same connector, > so pull the connector field from dpu_encoder_phys into dpu_encoder_virt > structure. What is the benefit? Less code? Clearer association?
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
Jason Gunthorpe writes: > On Tue, Feb 15, 2022 at 04:35:56PM -0500, Felix Kuehling wrote: >> >> On 2022-02-15 14:41, Jason Gunthorpe wrote: >> > On Tue, Feb 15, 2022 at 07:32:09PM +0100, Christoph Hellwig wrote: >> > > On Tue, Feb 15, 2022 at 10:45:24AM -0400, Jason Gunthorpe wrote: >> > > > > Do you know if DEVICE_GENERIC pages would end up as PageAnon()? My >> > > > > assumption was that they would be part of a special mapping. >> > > > We need to stop using the special PTEs and VMAs for things that have a >> > > > struct page. This is a mistake DAX created that must be undone. >> > > Yes, we'll get to it. Maybe we can do it for the non-DAX devmap >> > > ptes first given that DAX is more complicated. >> > Probably, I think we can check the page->pgmap type to tell the >> > difference. >> > >> > I'm not sure how the DEVICE_GENERIC can work without this, as DAX was >> > made safe by using the unmap_mapping_range(), which won't work >> > here. Is there some other trick being used to keep track of references >> > inside the AMD driver? >> >> Not sure I'm following all the discussion about VMAs and DAX. So I may be >> answering the wrong question: We treat each ZONE_DEVICE page as a reference >> to the BO (buffer object) that backs the page. We increment the BO refcount >> for each page we migrate into it. In the dev_pagemap_ops.page_free callback >> we drop that reference. Once all pages backed by a BO are freed, the BO >> refcount reaches 0 [*] and we can free the BO allocation. > > Userspace does > 1) mmap(MAP_PRIVATE) to allocate anon memory > 2) something to trigger migration to install a ZONE_DEVICE page > 3) munmap() > > Who decrements the refcout on the munmap? > > When a ZONE_DEVICE page is installed in the PTE is supposed to be > marked as pte_devmap and that disables all the normal page refcounting > during munmap(). Device private and device coherent pages are not marked with pte_devmap and they are backed by a struct page. The only way of inserting them is via migrate_vma. The refcount is decremented in zap_pte_range() on munmap() with special handling for device private pages. Looking at it again though I wonder if there is any special treatment required in zap_pte_range() for device coherent pages given they count as present pages. > fsdax makes this work by working the refcounts backwards, the page is > refcounted while it exists in the driver, when the driver decides to > remove it then unmap_mapping_range() is called to purge it from all > PTEs and then refcount is decrd. munmap/fork/etc don't change the > refcount. The equivalent here is for drivers to use migrate_vma to migrate the pages back from device memory to CPU memory. In this case the refcounting is (mostly) handled by migration code which decrements the refcount on the original source device page during the migration. - Alistair > Jason
Re: [PATCH v2 6/8] drm/msm/dpu: switch dpu_encoder to use atomic_mode_set
Quoting Dmitry Baryshkov (2022-02-15 06:16:41) > Make dpu_encoder use atomic_mode_set to receive connector and CRTC > states as arguments rather than finding connector and CRTC by manually > looping through the respective lists. > > Reviewed-by: Abhinav Kumar > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 5/8] drm/msm/dpu: encoder: drop unused mode_fixup callback
Quoting Dmitry Baryshkov (2022-02-15 06:16:40) > Both cmd and vid backends provide useless mode_fixup() callback. Drop > it. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 4/8] drm/msm/dpu: drop bus_scaling_client field
Quoting Dmitry Baryshkov (2022-02-15 06:16:39) > We do not use MSM bus client, so drop bus_scaling_client field from > dpu_encoder_virt. > > Reviewed-by: Abhinav Kumar > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 3/8] drm/msm/dpu: remove msm_dp cached in dpu_encoder_virt
Quoting Dmitry Baryshkov (2022-02-15 06:16:38)
> Stop caching msm_dp instance in dpu_encoder_virt since it's not used
> now.
>
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable
> and disable")
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH 5/6] drm/msm/dpu: remove struct dpu_encoder_irq
On 2/1/2022 7:10 AM, Dmitry Baryshkov wrote:
Remove additional indirection: specify IRQ callbacks and IRQ indices
directly rather than through the pointer in the irq structure. For each
IRQ we have a constant IRQ callback. This change simplifies code review
as the reader no longer needs to remember which function is called.
Signed-off-by: Dmitry Baryshkov
The change itself is fine as it will help to reduce yet another struct
struct dpu_encoder_irq.
One suggestion I can think of is, this also gets rid of the "name"
member which can be useful for debugging.
Do you think we can add a helper dpu_encoder_irq_idx_to_name(int idx)
Which will just translate this enum to the relevant string and add that
to the prints which were previously using the "name" member?
156 enum dpu_intr_idx {
157 INTR_IDX_VSYNC,
158 INTR_IDX_PINGPONG,
159 INTR_IDX_UNDERRUN,
160 INTR_IDX_CTL_START,
161 INTR_IDX_RDPTR,
162 INTR_IDX_MAX,
163 };
Let me know if you think this will be useful.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 28 +++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 21 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 73 +++
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 42 ---
4 files changed, 58 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dbcbf96cf8eb..83b6715820fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -261,9 +261,10 @@ static int dpu_encoder_helper_wait_event_timeout(int32_t
drm_id,
int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
enum dpu_intr_idx intr_idx,
+ void (*func)(void *arg, int irq_idx),
struct dpu_encoder_wait_info *wait_info)
{
- struct dpu_encoder_irq *irq;
+ int irq;
u32 irq_status;
int ret;
@@ -271,7 +272,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
DPU_ERROR("invalid params\n");
return -EINVAL;
}
- irq = &phys_enc->irq[intr_idx];
+ irq = phys_enc->irq[intr_idx];
/* note: do master / slave checking outside */
@@ -279,53 +280,52 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
if (phys_enc->enable_state == DPU_ENC_DISABLED) {
DRM_ERROR("encoder is disabled id=%u, intr=%d, irq=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx);
+ irq);
return -EWOULDBLOCK;
}
- if (irq->irq_idx < 0) {
- DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d, irq=%s\n",
- DRMID(phys_enc->parent), intr_idx,
- irq->name);
+ if (irq < 0) {
+ DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d\n",
+ DRMID(phys_enc->parent), intr_idx);
return 0;
}
DRM_DEBUG_KMS("id=%u, intr=%d, irq=%d, pp=%d, pending_cnt=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx, phys_enc->hw_pp->idx - PINGPONG_0,
+ irq, phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
ret = dpu_encoder_helper_wait_event_timeout(
DRMID(phys_enc->parent),
- irq->irq_idx,
+ irq,
wait_info);
if (ret <= 0) {
- irq_status = dpu_core_irq_read(phys_enc->dpu_kms, irq->irq_idx);
+ irq_status = dpu_core_irq_read(phys_enc->dpu_kms, irq);
if (irq_status) {
unsigned long flags;
DRM_DEBUG_KMS("irq not triggered id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx,
+ irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
local_irq_save(flags);
- irq->func(phys_enc, irq->irq_idx);
+ func(phys_enc, irq);
local_irq_restore(flags);
ret = 0;
} else {
ret = -ETIMEDOUT;
DRM_DEBUG_KMS("irq timeout id=%u, intr=%d, irq=%d, pp=%d,
atomic_cnt=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx,
+ irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(w
Re: [PATCH v2 2/8] drm/msm: move struct msm_display_info to dpu driver
Quoting Dmitry Baryshkov (2022-02-15 06:16:37) > The msm_display_info structure is not used by the rest of msm driver, so > move it into the dpu1 (dpu_encoder.h to be precise). > > Reviewed-by: Abhinav Kumar > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 1/8] drm/msm/dpu: fix dp audio condition
Quoting Dmitry Baryshkov (2022-02-15 06:16:36)
> DP audio enablement code which is comparing intf_type,
> DRM_MODE_ENCODER_TMDS (= 2) with DRM_MODE_CONNECTOR_DisplayPort (= 10).
> Which would never succeed. Fix it to check for DRM_MODE_ENCODER_TMDS.
>
> Fixes: d13e36d7d222 ("drm/msm/dp: add audio support for Display Port on MSM")
> Signed-off-by: Dmitry Baryshkov
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On 2/15/2022 4:20 PM, Dmitry Baryshkov wrote:
On Tue, 15 Feb 2022 at 23:21, Abhinav Kumar wrote:
On 2/15/2022 10:42 AM, Dmitry Baryshkov wrote:
On Tue, 15 Feb 2022 at 20:42, Abhinav Kumar wrote:
On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
From: Rob Clark
Add SC8180x to the hardware catalog, for initial support for the
platform. Due to limitations in the DP driver only one of the four DP
interfaces is left enabled.
The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
is flagged appropriately to ensure widebus is disabled - for now.
Signed-off-by: Rob Clark
[bjorn: Reworked intf and irq definitions]
Signed-off-by: Bjorn Andersson
---
Changes since v1:
- Dropped widebus flag
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
drivers/gpu/drm/msm/msm_drv.c | 1 +
4 files changed, 132 insertions(+)
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 aa75991903a6..7ac0fe32df49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -90,6 +90,17 @@
BIT(MDP_INTF3_INTR) | \
BIT(MDP_INTF4_INTR))
+#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+ BIT(MDP_SSPP_TOP0_INTR2) | \
+ BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+ BIT(MDP_INTF0_INTR) | \
+ BIT(MDP_INTF1_INTR) | \
+ BIT(MDP_INTF2_INTR) | \
+ BIT(MDP_INTF3_INTR) | \
+ BIT(MDP_INTF4_INTR) | \
+ BIT(MDP_INTF5_INTR) | \
+ BIT(MDP_AD4_0_INTR) | \
+ BIT(MDP_AD4_1_INTR))
#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH 2048
@@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
.max_vdeci_exp = MAX_VERT_DECIMATION,
};
+static const struct dpu_caps sc8180x_dpu_caps = {
+ .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+ .max_mixer_blendstages = 0xb,
+ .qseed_type = DPU_SSPP_SCALER_QSEED3,
+ .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
+ .ubwc_version = DPU_HW_UBWC_VER_30,
+ .has_src_split = true,
+ .has_dim_layer = true,
+ .has_idle_pc = true,
+ .has_3d_merge = true,
+ .max_linewidth = 4096,
+ .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+ .max_hdeci_exp = MAX_HORZ_DECIMATION,
+ .max_vdeci_exp = MAX_VERT_DECIMATION,
+};
+
static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
@@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
},
};
+static const struct dpu_mdp_cfg sc8180x_mdp[] = {
+ {
+ .name = "top_0", .id = MDP_TOP,
+ .base = 0x0, .len = 0x45C,
+ .features = 0,
+ .highest_bank_bit = 0x3,
+ .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
+ .reg_off = 0x2AC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
+ .reg_off = 0x2B4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
+ .reg_off = 0x2BC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
+ .reg_off = 0x2C4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
+ .reg_off = 0x2AC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
+ .reg_off = 0x2B4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .reg_off = 0x2BC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .reg_off = 0x2C4, .bit_off = 8},
+ },
+};
+
static const struct dpu_mdp_cfg sm8250_mdp[] = {
{
.name = "top_0", .id = MDP_TOP,
@@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
+static const struct dpu_intf_cfg sc8180x_intf[] = {
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
+ INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 28, 29),
+ /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until this
is supported */
+ INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 30, 31),
+ INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7180_MASK, MD
Re: [PATCH] drm/virtio: Fix capset-id query size
On Tue, Feb 15, 2022 at 5:15 PM Rob Clark wrote:
> From: Rob Clark
>
> The UABI was already defined for pointer to 64b value, and all the
> userspace users of this ioctl that I could find are already using a
> uint64_t (but zeroing it out to work around kernel only copying 32b).
> Unfortunately this ioctl doesn't have a length field, so out of paranoia
> I restricted the change to copy 64b to the single 64b param that can be
> queried.
>
> Fixes: 78aa20fa4381 ("drm/virtio: implement context init: advertise
> feature to userspace")
> Signed-off-by: Rob Clark
>
Reviewed-by: Gurchetan Singh
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 0f2f3f54dbf9..0158d27d5645 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -269,7 +269,8 @@ static int virtio_gpu_getparam_ioctl(struct drm_device
> *dev, void *data,
> {
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct drm_virtgpu_getparam *param = data;
> - int value;
> + int value, ret, sz = sizeof(int);
> + uint64_t value64;
>
> switch (param->param) {
> case VIRTGPU_PARAM_3D_FEATURES:
> @@ -291,13 +292,20 @@ static int virtio_gpu_getparam_ioctl(struct
> drm_device *dev, void *data,
> value = vgdev->has_context_init ? 1 : 0;
> break;
> case VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs:
> - value = vgdev->capset_id_mask;
> + value64 = vgdev->capset_id_mask;
> + sz = sizeof(value64);
> break;
> default:
> return -EINVAL;
> }
> - if (copy_to_user(u64_to_user_ptr(param->value), &value,
> sizeof(int)))
> - return -EFAULT;
> +
> + if (sz == sizeof(int)) {
> + if (copy_to_user(u64_to_user_ptr(param->value), &value,
> sz))
> + return -EFAULT;
> + } else {
> + if (copy_to_user(u64_to_user_ptr(param->value), &value64,
> sz))
> + return -EFAULT;
> + }
>
> return 0;
> }
> --
> 2.34.1
>
>
Re: [PATCH 1/1] drm/amdkfd: Replace zero-length array with flexible-array member
On 2022-02-15 7:38 p.m., Felix Kuehling
wrote:
Reference:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
CC: Changcheng Deng
Signed-off-by: Felix Kuehling
Reviewed-by: Philip Yang
---
include/uapi/linux/kfd_ioctl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 6e4268f5e482..baec5a41de3e 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -673,7 +673,7 @@ struct kfd_ioctl_svm_args {
__u32 op;
__u32 nattr;
/* Variable length array of attributes */
- struct kfd_ioctl_svm_attribute attrs[0];
+ struct kfd_ioctl_svm_attribute attrs[];
};
/**
[PATCH] drm/virtio: Fix capset-id query size
From: Rob Clark
The UABI was already defined for pointer to 64b value, and all the
userspace users of this ioctl that I could find are already using a
uint64_t (but zeroing it out to work around kernel only copying 32b).
Unfortunately this ioctl doesn't have a length field, so out of paranoia
I restricted the change to copy 64b to the single 64b param that can be
queried.
Fixes: 78aa20fa4381 ("drm/virtio: implement context init: advertise feature to
userspace")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 0f2f3f54dbf9..0158d27d5645 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -269,7 +269,8 @@ static int virtio_gpu_getparam_ioctl(struct drm_device
*dev, void *data,
{
struct virtio_gpu_device *vgdev = dev->dev_private;
struct drm_virtgpu_getparam *param = data;
- int value;
+ int value, ret, sz = sizeof(int);
+ uint64_t value64;
switch (param->param) {
case VIRTGPU_PARAM_3D_FEATURES:
@@ -291,13 +292,20 @@ static int virtio_gpu_getparam_ioctl(struct drm_device
*dev, void *data,
value = vgdev->has_context_init ? 1 : 0;
break;
case VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs:
- value = vgdev->capset_id_mask;
+ value64 = vgdev->capset_id_mask;
+ sz = sizeof(value64);
break;
default:
return -EINVAL;
}
- if (copy_to_user(u64_to_user_ptr(param->value), &value, sizeof(int)))
- return -EFAULT;
+
+ if (sz == sizeof(int)) {
+ if (copy_to_user(u64_to_user_ptr(param->value), &value, sz))
+ return -EFAULT;
+ } else {
+ if (copy_to_user(u64_to_user_ptr(param->value), &value64, sz))
+ return -EFAULT;
+ }
return 0;
}
--
2.34.1
Re: [PATCH v2] drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX
On Tue, Feb 15, 2022 at 3:46 PM Doug Anderson wrote: > On Tue, Feb 15, 2022 at 2:52 PM Brian Norris wrote: > > It still makes me wonder what the point > > of the /dev/drm_dp_aux interface is though, because it seems like > > you're pretty much destined to not have reliable operation through > > that means. > > I can't say I have tons of history for those files. I seem to recall > maybe someone using them to have userspace tweak the embedded > backlight on some external DP connected panels? I think we also might > use it in Chrome OS to update the firmware of panels (dunno if > internal or external) in some cases too? I suspect that it works OK > for certain situations but it's really not going to work in all > cases... Yes, I believe I'm only submitting patches like this because fwupd apparently likes to indiscriminately whack at dpaux devices: https://github.com/fwupd/fwupd/tree/main/plugins/synaptics-mst#kernel-dp-aux-interface That seems like a bad idea. (We've already disabled that plugin on these systems, but it seems wise not to leave the stumbling block here for the next time.) > I suppose this just further proves the point that this is really not a > great interface to rely on. It's fine for debugging during hardware > bringup and I guess in limited situations it might be OK, but it's > really not something we want userspace tweaking with anyway, right? In > general I expect it's up to the kernel to be controlling peripherals > on the DP AUX bus. The kernel should have a backlight driver and > should do the AUX transfers needed. Having userspace in there mucking > with things is just a bad idea. I mean, userspace also doesn't know > when a panel has been power cycled and potentially lost any changes > that they might have written, right? > > I sorta suspect that most of the uses of these files are there because > there wasn't a kernel driver and someone thought that doing it in > userspace was the way to go? *shrug* beats me. Brian
[PATCH 1/1] drm/amdkfd: Replace zero-length array with flexible-array member
Reference:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
CC: Changcheng Deng
Signed-off-by: Felix Kuehling
---
include/uapi/linux/kfd_ioctl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 6e4268f5e482..baec5a41de3e 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -673,7 +673,7 @@ struct kfd_ioctl_svm_args {
__u32 op;
__u32 nattr;
/* Variable length array of attributes */
- struct kfd_ioctl_svm_attribute attrs[0];
+ struct kfd_ioctl_svm_attribute attrs[];
};
/**
--
2.32.0
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On Tue, 15 Feb 2022 at 23:21, Abhinav Kumar wrote:
> On 2/15/2022 10:42 AM, Dmitry Baryshkov wrote:
> > On Tue, 15 Feb 2022 at 20:42, Abhinav Kumar
> > wrote:
> >> On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
> >>> On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
> >>>
>
>
> On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
> > From: Rob Clark
> >
> > Add SC8180x to the hardware catalog, for initial support for the
> > platform. Due to limitations in the DP driver only one of the four DP
> > interfaces is left enabled.
> >
> > The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
> > the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
> > is flagged appropriately to ensure widebus is disabled - for now.
> >
> > Signed-off-by: Rob Clark
> > [bjorn: Reworked intf and irq definitions]
> > Signed-off-by: Bjorn Andersson
> > ---
> >
> > Changes since v1:
> > - Dropped widebus flag
> >
> > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129
> > ++
> > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> > drivers/gpu/drm/msm/msm_drv.c | 1 +
> > 4 files changed, 132 insertions(+)
> >
> > 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 aa75991903a6..7ac0fe32df49 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -90,6 +90,17 @@
> > BIT(MDP_INTF3_INTR) | \
> > BIT(MDP_INTF4_INTR))
> > +#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
> > + BIT(MDP_SSPP_TOP0_INTR2) | \
> > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> > + BIT(MDP_INTF0_INTR) | \
> > + BIT(MDP_INTF1_INTR) | \
> > + BIT(MDP_INTF2_INTR) | \
> > + BIT(MDP_INTF3_INTR) | \
> > + BIT(MDP_INTF4_INTR) | \
> > + BIT(MDP_INTF5_INTR) | \
> > + BIT(MDP_AD4_0_INTR) | \
> > + BIT(MDP_AD4_1_INTR))
> > #define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
> > #define DEFAULT_DPU_LINE_WIDTH 2048
> > @@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
> > .max_vdeci_exp = MAX_VERT_DECIMATION,
> > };
> > +static const struct dpu_caps sc8180x_dpu_caps = {
> > + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > + .max_mixer_blendstages = 0xb,
> > + .qseed_type = DPU_SSPP_SCALER_QSEED3,
> > + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
> > + .ubwc_version = DPU_HW_UBWC_VER_30,
> > + .has_src_split = true,
> > + .has_dim_layer = true,
> > + .has_idle_pc = true,
> > + .has_3d_merge = true,
> > + .max_linewidth = 4096,
> > + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> > + .max_hdeci_exp = MAX_HORZ_DECIMATION,
> > + .max_vdeci_exp = MAX_VERT_DECIMATION,
> > +};
> > +
> > static const struct dpu_caps sm8250_dpu_caps = {
> > .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > .max_mixer_blendstages = 0xb,
> > @@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
> > },
> > };
> > +static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> > + {
> > + .name = "top_0", .id = MDP_TOP,
> > + .base = 0x0, .len = 0x45C,
> > + .features = 0,
> > + .highest_bank_bit = 0x3,
> > + .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
> > + .reg_off = 0x2AC, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
> > + .reg_off = 0x2B4, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
> > + .reg_off = 0x2BC, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
> > + .reg_off = 0x2C4, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> > + .reg_off = 0x2AC, .bit_off = 8},
> > + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> > + .reg_off = 0x2B4, .bit_off = 8},
> > + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> > + .reg_off = 0x2BC, .bit_off = 8},
> > + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> > + .reg_off = 0x2C4, .bit_off = 8},
> > + },
> > +};
> > +
> > static const struct dpu_mdp_cfg sm8250_mdp[] = {
> > {
> > .name = "top_0", .id = MDP_TOP,
> > @@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
> > INTF_BLK("intf_5", INTF_5, 0x39000,
Re: [PATCH] drm/amdkfd: Replace zero-length array with flexible-array member
On 2022-02-15 04:11, cgel@gmail.com wrote:
From: Changcheng Deng
There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use "flexible array members" for these cases. The older
style of one-element or zero-length arrays should no longer be used.
Reference:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
Reported-by: Zeal Robot
Signed-off-by: Changcheng Deng
Makes sense. I'm applying this patch to amd-staging-drm-next.
I also found a similar issue in include/uapi/linux/kfd_ioctl.h. I'll
send another patch to fix that as well.
Regards,
Felix
---
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index e54a52785690..7d39191d13f6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1084,7 +1084,7 @@ struct kfd_criu_svm_range_priv_data {
uint64_t start_addr;
uint64_t size;
/* Variable length array of attributes */
- struct kfd_ioctl_svm_attribute attrs[0];
+ struct kfd_ioctl_svm_attribute attrs[];
};
struct kfd_criu_queue_priv_data {
Re: [PATCH 3/6] drm/msm/dpu: allow just single IRQ callback
On 2/1/2022 7:10 AM, Dmitry Baryshkov wrote:
DPU interrupts code allows multiple callbacks per interrut. In reality
none of the interrupts is shared between blocks (and will probably never
be). Drop support for registering multiple callbacks per interrupt to
simplify interrupt handling code.
Signed-off-by: Dmitry Baryshkov
I think this has some kbot issues, will ack once those are addressed.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 16 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 6 +-
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 140 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 12 --
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 12 +-
9 files changed, 75 insertions(+), 142 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 6dce5d89f817..b5b6e7031fb9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -44,10 +44,8 @@ u32 dpu_core_irq_read(
* interrupt
* @dpu_kms: DPU handle
* @irq_idx: irq index
- * @irq_cb:IRQ callback structure, containing callback function
- * and argument. Passing NULL for irq_cb will unregister
- * the callback for the given irq_idx
- * This must exist until un-registration.
+ * @irq_cb:IRQ callback funcion.
+ * @irq_arg: IRQ callback argument.
* @return: 0 for success registering callback, otherwise failure
*
* This function supports registration of multiple callbacks for each
interrupt.
@@ -55,25 +53,21 @@ u32 dpu_core_irq_read(
int dpu_core_irq_register_callback(
struct dpu_kms *dpu_kms,
int irq_idx,
- struct dpu_irq_callback *irq_cb);
+ void (*irq_cb)(void *arg, int irq_idx),
+ void *irq_arg);
/**
* dpu_core_irq_unregister_callback - For unregistering callback function on
IRQ
* interrupt
* @dpu_kms: DPU handle
* @irq_idx: irq index
- * @irq_cb:IRQ callback structure, containing callback function
- * and argument. Passing NULL for irq_cb will unregister
- * the callback for the given irq_idx
- * This must match with registration.
* @return: 0 for success registering callback, otherwise failure
*
* This function supports registration of multiple callbacks for each
interrupt.
*/
int dpu_core_irq_unregister_callback(
struct dpu_kms *dpu_kms,
- int irq_idx,
- struct dpu_irq_callback *irq_cb);
+ int irq_idx);
/**
* dpu_debugfs_core_irq_init - register core irq debugfs
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5576b8a3e6ee..17ca149e7dcd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -311,7 +311,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys
*phys_enc,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
local_irq_save(flags);
- irq->cb.func(phys_enc, irq->irq_idx);
+ irq->func(phys_enc, irq->irq_idx);
local_irq_restore(flags);
ret = 0;
} else {
@@ -352,7 +352,7 @@ int dpu_encoder_helper_register_irq(struct dpu_encoder_phys
*phys_enc,
}
ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx,
- &irq->cb);
+ irq->func, phys_enc);
if (ret) {
DPU_ERROR_PHYS(phys_enc,
"failed to register IRQ callback for %s\n",
@@ -383,8 +383,7 @@ int dpu_encoder_helper_unregister_irq(struct
dpu_encoder_phys *phys_enc,
return 0;
}
- ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx,
- &irq->cb);
+ ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx);
if (ret) {
DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d",
DRMID(phys_enc->parent), intr_idx,
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 e7270eb6b84b..80d87871fd94 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/d
[PATCH 1/2] drm/bridge: analogix_dp: Support PSR-exit to disable transition
Most eDP panel functions only work correctly when the panel is not in
self-refresh. In particular, analogix_dp_bridge_disable() tends to hit
AUX channel errors if the panel is in self-refresh.
Given the above, it appears that so far, this driver assumes that we are
never in self-refresh when it comes time to fully disable the bridge.
Prior to commit 846c7dfc1193 ("drm/atomic: Try to preserve the crtc
enabled state in drm_atomic_remove_fb, v2."), this tended to be true,
because we would automatically disable the pipe when framebuffers were
removed, and so we'd typically disable the bridge shortly after the last
display activity.
However, that is not guaranteed: an idle (self-refresh) display pipe may
be disabled, e.g., when switching CRTCs. We need to exit PSR first.
Stable notes: this is definitely a bugfix, and the bug has likely
existed in some form for quite a while. It may predate the "PSR helpers"
refactor, but the code looked very different before that, and it's
probably not worth rewriting the fix.
Cc:
Fixes: 6c836d965bad ("drm/rockchip: Use the helpers for PSR")
Signed-off-by: Brian Norris
---
.../drm/bridge/analogix/analogix_dp_core.c| 42 +--
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index b7d2e4449cfa..6ee0f62a7161 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1268,6 +1268,25 @@ static int analogix_dp_bridge_attach(struct drm_bridge
*bridge,
return 0;
}
+static
+struct drm_crtc *analogix_dp_get_old_crtc(struct analogix_dp_device *dp,
+ struct drm_atomic_state *state)
+{
+ struct drm_encoder *encoder = dp->encoder;
+ struct drm_connector *connector;
+ struct drm_connector_state *conn_state;
+
+ connector = drm_atomic_get_old_connector_for_encoder(state, encoder);
+ if (!connector)
+ return NULL;
+
+ conn_state = drm_atomic_get_old_connector_state(state, connector);
+ if (!conn_state)
+ return NULL;
+
+ return conn_state->crtc;
+}
+
static
struct drm_crtc *analogix_dp_get_new_crtc(struct analogix_dp_device *dp,
struct drm_atomic_state *state)
@@ -1448,14 +1467,16 @@ analogix_dp_bridge_atomic_disable(struct drm_bridge
*bridge,
{
struct drm_atomic_state *old_state = old_bridge_state->base.state;
struct analogix_dp_device *dp = bridge->driver_private;
- struct drm_crtc *crtc;
+ struct drm_crtc *old_crtc, *new_crtc;
+ struct drm_crtc_state *old_crtc_state = NULL;
struct drm_crtc_state *new_crtc_state = NULL;
+ int ret;
- crtc = analogix_dp_get_new_crtc(dp, old_state);
- if (!crtc)
+ new_crtc = analogix_dp_get_new_crtc(dp, old_state);
+ if (!new_crtc)
goto out;
- new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc);
+ new_crtc_state = drm_atomic_get_new_crtc_state(old_state, new_crtc);
if (!new_crtc_state)
goto out;
@@ -1464,6 +1485,19 @@ analogix_dp_bridge_atomic_disable(struct drm_bridge
*bridge,
return;
out:
+ old_crtc = analogix_dp_get_old_crtc(dp, old_state);
+ if (old_crtc) {
+ old_crtc_state = drm_atomic_get_old_crtc_state(old_state,
+ old_crtc);
+
+ /* When moving from PSR to fully disabled, exit PSR first. */
+ if (old_crtc_state && old_crtc_state->self_refresh_active) {
+ ret = analogix_dp_disable_psr(dp);
+ if (ret)
+ DRM_ERROR("Failed to disable psr (%d)\n", ret);
+ }
+ }
+
analogix_dp_bridge_disable(bridge);
}
--
2.35.1.265.g69c8d7142f-goog
[PATCH 2/2] drm/atomic: Force bridge self-refresh-exit on CRTC switch
It's possible to change which CRTC is in use for a given
connector/encoder/bridge while we're in self-refresh without fully
disabling the connector/encoder/bridge along the way. This can confuse
the bridge encoder/bridge, because
(a) it needs to track the SR state (trying to perform "active"
operations while the panel is still in SR can be Bad(TM)); and
(b) it tracks the SR state via the CRTC state (and after the switch, the
previous SR state is lost).
Thus, we need to either somehow carry the self-refresh state over to the
new CRTC, or else force an encoder/bridge self-refresh transition during
such a switch.
I choose the latter, so we disable the encoder (and exit PSR) before
attaching it to the new CRTC (where we can continue to assume a clean
(non-self-refresh) state).
This fixes PSR issues seen on Rockchip RK3399 systems with
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c.
Cc:
Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh mode in
drivers")
Signed-off-by: Brian Norris
---
drivers/gpu/drm/drm_atomic_helper.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 9603193d2fa1..74161d007894 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1011,9 +1011,19 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
return drm_atomic_crtc_effectively_active(old_state);
/*
-* We need to run through the crtc_funcs->disable() function if the CRTC
-* is currently on, if it's transitioning to self refresh mode, or if
-* it's in self refresh mode and needs to be fully disabled.
+* We need to disable bridge(s) and CRTC if we're transitioning out of
+* self-refresh and changing CRTCs at the same time, because the
+* bridge tracks self-refresh status via CRTC state.
+*/
+ if (old_state->self_refresh_active && new_state->enable &&
+ old_state->crtc != new_state->crtc)
+ return true;
+
+ /*
+* We also need to run through the crtc_funcs->disable() function if
+* the CRTC is currently on, if it's transitioning to self refresh
+* mode, or if it's in self refresh mode and needs to be fully
+* disabled.
*/
return old_state->active ||
(old_state->self_refresh_active && !new_state->active) ||
--
2.35.1.265.g69c8d7142f-goog
[PATCH 0/2] drm/bridge: analogix_dp: Self-refresh state machine fixes
Hi, I've been investigating several eDP issues on a Rockchip RK3399 system and have two proposed bugfixes. RK3399 has two CRTCs, either of which can be used for eDP output. For both fixes, we have bugs due to the relationship between the generalized "self refresh helpers" and the analogix-dp bridge driver which controls much of the PSR mechanics. These bugs are most visible when switching between CRTCs. I'm not a DRM expert, but I've been poking at a lot of Rockchip display drivers recently. I'd love some skeptical eyes, so feel free to ask questions if I haven't explained issues well, or the proposals look fishy. Regards, Brian Brian Norris (2): drm/bridge: analogix_dp: Support PSR-exit to disable transition drm/atomic: Force bridge self-refresh-exit on CRTC switch .../drm/bridge/analogix/analogix_dp_core.c| 42 +-- drivers/gpu/drm/drm_atomic_helper.c | 16 +-- 2 files changed, 51 insertions(+), 7 deletions(-) -- 2.35.1.265.g69c8d7142f-goog
Re: [PATCH v2] drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX
Hi, On Tue, Feb 15, 2022 at 2:52 PM Brian Norris wrote: > > On Tue, Feb 15, 2022 at 1:31 PM Doug Anderson wrote: > > > > Hi, > > Hi! > > > On Fri, Oct 1, 2021 at 2:50 PM Brian Norris > > wrote: > > > > > > If the display is not enable()d, then we aren't holding a runtime PM > > > reference here. Thus, it's easy to accidentally cause a hang, if user > > > space is poking around at /dev/drm_dp_aux0 at the "wrong" time. > > > > > > Let's get the panel and PM state right before trying to talk AUX. > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > > index b7d2e4449cfa..6fc46ac93ef8 100644 > > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > > @@ -1632,8 +1632,27 @@ static ssize_t analogix_dpaux_transfer(struct > > > drm_dp_aux *aux, > ... > > > + pm_runtime_get_sync(dp->dev); > > > + ret = analogix_dp_transfer(dp, msg); > > > + pm_runtime_put(dp->dev); > > > > I've spent an unfortunate amount of time digging around the DP AUX bus > > recently, so I can at least say that I have some experience and some > > opinions here. > > Thanks! Experience is welcome, and opinions too sometimes ;) > > > IMO: > > > > 1. Don't power the panel on. If the panel isn't powered on then the DP > > AUX transfer will timeout. Tough nuggies. Think of yourself more like > > an i2c controller and of this as an i2c transfer implementation. The > > i2c controller isn't in charge of powering up the i2c devices on the > > bus. If userspace does an "i2c detect" on an i2c bus and some of the > > devices aren't powered then they won't be found. If you try to > > read/write from a powered off device that won't work either. > > I guess this, paired with the driver examples below (ti-sn65dsi86.c, > especially, which specifically throws errors if the panel isn't on), > makes some sense. It's approximately (but more verbosely) what Andrzej > was recommending too, I guess. It still makes me wonder what the point > of the /dev/drm_dp_aux interface is though, because it seems like > you're pretty much destined to not have reliable operation through > that means. I can't say I have tons of history for those files. I seem to recall maybe someone using them to have userspace tweak the embedded backlight on some external DP connected panels? I think we also might use it in Chrome OS to update the firmware of panels (dunno if internal or external) in some cases too? I suspect that it works OK for certain situations but it's really not going to work in all cases... > Also note: I found that the AUX bus is really not working properly at > all (even with this patch) in some cases due to self-refresh. Not only > do we need the panel enabled, but we need to not be in self-refresh > mode. Self-refresh is not currently exposed to user space, so user > space has no way of knowing the panel is currently active, aside from > racily inducing artificial display activity. I suppose this just further proves the point that this is really not a great interface to rely on. It's fine for debugging during hardware bringup and I guess in limited situations it might be OK, but it's really not something we want userspace tweaking with anyway, right? In general I expect it's up to the kernel to be controlling peripherals on the DP AUX bus. The kernel should have a backlight driver and should do the AUX transfers needed. Having userspace in there mucking with things is just a bad idea. I mean, userspace also doesn't know when a panel has been power cycled and potentially lost any changes that they might have written, right? I sorta suspect that most of the uses of these files are there because there wasn't a kernel driver and someone thought that doing it in userspace was the way to go? > But if we're OK with "just throw errors" or "just let stuff time out", > then I guess that's not a big deal. My purpose is to avoid hanging the > system, not to make /dev/drm_dp_aux useful. > > > 2. In theory if the DP driver can read HPD (I haven't looked through > > the analogix code to see how it handles it) then you can fail an AUX > > transfer right away if HPD isn't asserted instead of timing out. If > > this is hard, it's probably fine to just time out though. > > This driver does handle HPD, but it also has overrides because > apparently it doesn't work on some systems. I might see if we can > leverage it, or I might just follow the bridge-enabled state (similar > to ti-sn65dsi86.c's 'comms_enabled'). The "comms_enabled" is a bit ugly and is mostly there because we couldn't enable the bridge chip at the right time for some (probably unused) configuration, so I wouldn't necessarily say that it's the best model to follow. That being said, happy to review something if this model looks like the best way to go. > > 3. Do the "pm_runtime" calls, but enable "autosuspend" with something > > ~1 second au
Re: [PATCH v2 0/3] drm/panel-edp: Debugfs for panel-edp
Hi,
On Fri, Feb 4, 2022 at 4:14 PM Douglas Anderson wrote:
>
> The main goal of this series is the final patch in the series: to
> allow test code to reliably find out if we ended up hitting the
> "fallback" case of the generic edp-panel driver where we don't
> recognize a panel and choose to use super conservative timing.
>
> Version 1 of the patch actually landed but was quickly reverted since
> it was pointed out that it should have been done in debugfs, not
> sysfs.
>
> As discussed on IRC [1], we want this support to be under the
> "connector" directory of debugfs but there was no existing way to do
> that. Thus patch #2 in the series was born to try to plumb this
> through. It was asserted that it would be OK to rely on a fairly
> modern display pipeline for this plumbing and perhaps fail to populate
> the debugfs file if we're using older/deprecated pipelines.
>
> Patch #1 in the series was born because the bridge chip I was using
> was still using an older/deprecated pipeline. While this doesn't get
> us fully to a modern pipeline for ti-sn65dsi86 (it still doesn't move
> to "NO_CONNECTOR") it hopefully moves us in the right direction.
>
> This was tested on sc7180-trogdor devices with _both_ the ti-sn65dsi86
> and the parade-ps8640 bridge chips (since some devices have one, some
> the other). The parade-ps8640 already uses supports "NO_CONNECTOR",
> luckily.
>
> [1]
> https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2022-02-02
>
> Changes in v2:
> - ("ti-sn65dsi86: Use drm_bridge_connector") new for v2.
> - ("drm: Plumb debugfs_init through to panels") new for v2.
> - Now using debugfs, not sysfs
>
> Douglas Anderson (3):
> drm/bridge: ti-sn65dsi86: Use drm_bridge_connector
> drm: Plumb debugfs_init through to panels
> drm/panel-edp: Allow querying the detected panel via debugfs
>
> drivers/gpu/drm/bridge/panel.c | 12 +
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 72 +-
> drivers/gpu/drm/drm_bridge_connector.c | 15 ++
> drivers/gpu/drm/drm_debugfs.c | 3 ++
> drivers/gpu/drm/panel/panel-edp.c | 37 +++--
> include/drm/drm_bridge.h | 7 +++
> include/drm/drm_connector.h| 7 +++
> include/drm/drm_panel.h| 8 +++
> 8 files changed, 98 insertions(+), 63 deletions(-)
Landed these three patches to drm-misc-next w/ accumulated tags:
$ git log --oneline -3
6ed19359d6bd drm/panel-edp: Allow querying the detected panel via debugfs
2509969a9862 drm: Plumb debugfs_init through to panels
e283820cbf80 drm/bridge: ti-sn65dsi86: Use drm_bridge_connector
If it turns out that we want to add more reporting when debugfs calls
return errors then I'm happy to submit follow-on patches. Discussion
about that can be found in:
https://lore.kernel.org/r/CAD=FV=Ut3N9syXbN7i939mNsx3h7-u9cU9j6=xfkz9vrh0v...@mail.gmail.com
Unless something changes, though, my current plan is not to do
follow-up patches and leave this as-is without any extra error
reporting.
-Doug
Re: [Intel-gfx] [PATCH v2 08/18] drm/i915/guc: Convert engine record to iosys_map
On Tue, Feb 08, 2022 at 02:45:14AM -0800, Lucas De Marchi wrote:
> Use iosys_map to read fields from the dma_blob so access to IO and
> system memory is abstracted away.
>
> Cc: Matt Roper
> Cc: Thomas Hellström
> Cc: Daniel Vetter
> Cc: John Harrison
> Cc: Matthew Brost
> Cc: Daniele Ceraolo Spurio
Reviewed-by: Matt Atwood
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 14 ++
> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h | 3 ++-
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 17 ++---
> 3 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> index 6a34ab38b45f..383c5994d4ef 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> @@ -695,18 +695,16 @@ void intel_guc_ads_reset(struct intel_guc *guc)
>
> u32 intel_guc_engine_usage_offset(struct intel_guc *guc)
> {
> - struct __guc_ads_blob *blob = guc->ads_blob;
> - u32 base = intel_guc_ggtt_offset(guc, guc->ads_vma);
> - u32 offset = base + ptr_offset(blob, engine_usage);
> -
> - return offset;
> + return intel_guc_ggtt_offset(guc, guc->ads_vma) +
> + offsetof(struct __guc_ads_blob, engine_usage);
> }
>
> -struct guc_engine_usage_record *intel_guc_engine_usage(struct
> intel_engine_cs *engine)
> +struct iosys_map intel_guc_engine_usage_record_map(struct intel_engine_cs
> *engine)
> {
> struct intel_guc *guc = &engine->gt->uc.guc;
> - struct __guc_ads_blob *blob = guc->ads_blob;
> u8 guc_class = engine_class_to_guc_class(engine->class);
> + size_t offset = offsetof(struct __guc_ads_blob,
> +
> engine_usage.engines[guc_class][ilog2(engine->logical_mask)]);
>
> - return
> &blob->engine_usage.engines[guc_class][ilog2(engine->logical_mask)];
> + return IOSYS_MAP_INIT_OFFSET(&guc->ads_map, offset);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h
> index e74c110facff..1c64f4d6ea21 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h
> @@ -7,6 +7,7 @@
> #define _INTEL_GUC_ADS_H_
>
> #include
> +#include
>
> struct intel_guc;
> struct drm_printer;
> @@ -18,7 +19,7 @@ void intel_guc_ads_init_late(struct intel_guc *guc);
> void intel_guc_ads_reset(struct intel_guc *guc);
> void intel_guc_ads_print_policy_info(struct intel_guc *guc,
>struct drm_printer *p);
> -struct guc_engine_usage_record *intel_guc_engine_usage(struct
> intel_engine_cs *engine);
> +struct iosys_map intel_guc_engine_usage_record_map(struct intel_engine_cs
> *engine);
> u32 intel_guc_engine_usage_offset(struct intel_guc *guc);
>
> #endif
> 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 b3a429a92c0d..ab3cea352fb3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1139,6 +1139,9 @@ __extend_last_switch(struct intel_guc *guc, u64
> *prev_start, u32 new_start)
> *prev_start = ((u64)gt_stamp_hi << 32) | new_start;
> }
>
> +#define record_read(map_, field_) \
> + iosys_map_rd_field(map_, 0, struct guc_engine_usage_record, field_)
> +
> /*
> * GuC updates shared memory and KMD reads it. Since this is not
> synchronized,
> * we run into a race where the value read is inconsistent. Sometimes the
> @@ -1153,17 +1156,17 @@ __extend_last_switch(struct intel_guc *guc, u64
> *prev_start, u32 new_start)
> static void __get_engine_usage_record(struct intel_engine_cs *engine,
> u32 *last_in, u32 *id, u32 *total)
> {
> - struct guc_engine_usage_record *rec = intel_guc_engine_usage(engine);
> + struct iosys_map rec_map = intel_guc_engine_usage_record_map(engine);
> int i = 0;
>
> do {
> - *last_in = READ_ONCE(rec->last_switch_in_stamp);
> - *id = READ_ONCE(rec->current_context_index);
> - *total = READ_ONCE(rec->total_runtime);
> + *last_in = record_read(&rec_map, last_switch_in_stamp);
> + *id = record_read(&rec_map, current_context_index);
> + *total = record_read(&rec_map, total_runtime);
>
> - if (READ_ONCE(rec->last_switch_in_stamp) == *last_in &&
> - READ_ONCE(rec->current_context_index) == *id &&
> - READ_ONCE(rec->total_runtime) == *total)
> + if (record_read(&rec_map, last_switch_in_stamp) == *last_in &&
> + record_read(&rec_map, current_context_index) == *id &&
> + record_read(&rec_map, total_runtime) == *total)
> break;
> } while (++i < 6);
> }
> --
> 2.35.1
>
Re: [PATCH 2/6] drm/msm/dpu: remove always-true argument of dpu_core_irq_read()
On 2/1/2022 7:10 AM, Dmitry Baryshkov wrote:
The argument clear of the function dpu_core_irq_read() is always true.
Remove it.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 4 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 ++--
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 7023ccb79814..6dce5d89f817 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -33,13 +33,11 @@ irqreturn_t dpu_core_irq(struct msm_kms *kms);
* dpu_core_irq_read - IRQ helper function for reading IRQ status
* @dpu_kms: DPU handle
* @irq_idx: irq index
- * @clear: True to clear the irq after read
* @return: non-zero if irq detected; otherwise no irq detected
*/
u32 dpu_core_irq_read(
struct dpu_kms *dpu_kms,
- int irq_idx,
- bool clear);
+ int irq_idx);
/**
* dpu_core_irq_register_callback - For registering callback function on IRQ
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1e648db439f9..5576b8a3e6ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -301,8 +301,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys
*phys_enc,
wait_info);
if (ret <= 0) {
- irq_status = dpu_core_irq_read(phys_enc->dpu_kms,
- irq->irq_idx, true);
+ irq_status = dpu_core_irq_read(phys_enc->dpu_kms, irq->irq_idx);
if (irq_status) {
unsigned long flags;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 71882d3fe705..85404c9ab4e1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -357,7 +357,7 @@ static void dpu_disable_all_irqs(struct dpu_kms *dpu_kms)
wmb();
}
-u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx, bool clear)
+u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx)
{
struct dpu_hw_intr *intr = dpu_kms->hw_intr;
int reg_idx;
@@ -384,7 +384,7 @@ u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx,
bool clear)
intr_status = DPU_REG_READ(&intr->hw,
dpu_intr_set[reg_idx].status_off) &
DPU_IRQ_MASK(irq_idx);
- if (intr_status && clear)
+ if (intr_status)
DPU_REG_WRITE(&intr->hw, dpu_intr_set[reg_idx].clr_off,
intr_status);
Re: [PATCH v2 2/3] drm: Plumb debugfs_init through to panels
Hi,
On Tue, Feb 15, 2022 at 2:20 PM Andrzej Hajda wrote:
>
> On 15.02.2022 23:09, Javier Martinez Canillas wrote:
> > Hello Doug,
> >
> > On 2/5/22 01:13, Douglas Anderson wrote:
> >
> > [snip]
> >
> >> +static void panel_bridge_debugfs_init(struct drm_bridge *bridge,
> >> + struct dentry *root)
> >> +{
> >> +struct panel_bridge *panel_bridge =
> >> drm_bridge_to_panel_bridge(bridge);
> >> +struct drm_panel *panel = panel_bridge->panel;
> >> +
> >> +root = debugfs_create_dir("panel", root);
> > This could return a ERR_PTR(-errno) if the function doesn't succeed.
> >
> > I noticed that most kernel code doesn't check the return value though...
> >
> >> +if (panel->funcs->debugfs_init)
> > Probably if (!(IS_ERR(root) && panel->funcs->debugfs_init) ?
> >
> >> +panel->funcs->debugfs_init(panel, root);
> >> +}
> > [snip]
> >
> >> @@ -436,6 +436,9 @@ void drm_debugfs_connector_add(struct drm_connector
> >> *connector)
> >> /* vrr range */
> >> debugfs_create_file("vrr_range", S_IRUGO, root, connector,
> >> &vrr_range_fops);
> > Same here, wonder if the return value should be checked.
My plan (confirmed with Javier over IRC) is to land my patches and we
can address as needed with follow-up patches.
I actually wrote said follow-up patches and they were ready to go, but
when I was trying to come up with the right "Fixes" tag I found commit
b792e64021ec ("drm: no need to check return value of debugfs_create
functions"). So what's being requested is nearly the opposite of what
Greg did there.
I thought about perhaps only checking for directories but even that
type of check was removed by Greg's patch. Further checking shows that
start_creating() actually has:
if (IS_ERR(parent))
return parent;
...so I guess that explains why it's fine to skip the check even for parents?
Sure enough I confirmed that if I pass `ERR_PTR(-EINVAL)` as the root
for `panel->funcs->debugfs_init()` that nothing bad seems to happen...
> I've seen sometimes that file/dir was already created with the same
> name, reporting error in such case will be helpful.
It sure looks like start_creating() already handles that type of
reporting... Sure enough, I tried to create the "force" file twice,
adding no error checking myself, and I see:
debugfs: File 'force' in directory 'eDP-1' already present!
debugfs: File 'force' in directory 'DP-1' already present!
So tl;dr is that I'm going to land the patches and now am _not_
planning on doing followup patches. However, if I'm confused about any
of the above then please let me know and I'll dig more / can send
follow-up patches.
-Doug
Re: [PATCH v2] drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX
On Tue, Feb 15, 2022 at 1:31 PM Doug Anderson wrote: > > Hi, Hi! > On Fri, Oct 1, 2021 at 2:50 PM Brian Norris wrote: > > > > If the display is not enable()d, then we aren't holding a runtime PM > > reference here. Thus, it's easy to accidentally cause a hang, if user > > space is poking around at /dev/drm_dp_aux0 at the "wrong" time. > > > > Let's get the panel and PM state right before trying to talk AUX. > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index b7d2e4449cfa..6fc46ac93ef8 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -1632,8 +1632,27 @@ static ssize_t analogix_dpaux_transfer(struct > > drm_dp_aux *aux, ... > > + pm_runtime_get_sync(dp->dev); > > + ret = analogix_dp_transfer(dp, msg); > > + pm_runtime_put(dp->dev); > > I've spent an unfortunate amount of time digging around the DP AUX bus > recently, so I can at least say that I have some experience and some > opinions here. Thanks! Experience is welcome, and opinions too sometimes ;) > IMO: > > 1. Don't power the panel on. If the panel isn't powered on then the DP > AUX transfer will timeout. Tough nuggies. Think of yourself more like > an i2c controller and of this as an i2c transfer implementation. The > i2c controller isn't in charge of powering up the i2c devices on the > bus. If userspace does an "i2c detect" on an i2c bus and some of the > devices aren't powered then they won't be found. If you try to > read/write from a powered off device that won't work either. I guess this, paired with the driver examples below (ti-sn65dsi86.c, especially, which specifically throws errors if the panel isn't on), makes some sense. It's approximately (but more verbosely) what Andrzej was recommending too, I guess. It still makes me wonder what the point of the /dev/drm_dp_aux interface is though, because it seems like you're pretty much destined to not have reliable operation through that means. Also note: I found that the AUX bus is really not working properly at all (even with this patch) in some cases due to self-refresh. Not only do we need the panel enabled, but we need to not be in self-refresh mode. Self-refresh is not currently exposed to user space, so user space has no way of knowing the panel is currently active, aside from racily inducing artificial display activity. But if we're OK with "just throw errors" or "just let stuff time out", then I guess that's not a big deal. My purpose is to avoid hanging the system, not to make /dev/drm_dp_aux useful. > 2. In theory if the DP driver can read HPD (I haven't looked through > the analogix code to see how it handles it) then you can fail an AUX > transfer right away if HPD isn't asserted instead of timing out. If > this is hard, it's probably fine to just time out though. This driver does handle HPD, but it also has overrides because apparently it doesn't work on some systems. I might see if we can leverage it, or I might just follow the bridge-enabled state (similar to ti-sn65dsi86.c's 'comms_enabled'). > 3. Do the "pm_runtime" calls, but enable "autosuspend" with something > ~1 second autosuspend delay. When using the AUX bus to read an EDID > the underlying code will call your function 16 times in quick > succession. If you're powering up and down constantly that'll be a bit > of a waste. Does this part really matter? For properly active cases, the bridge remains enabled, and it holds a runtime PM reference. For "maybe active" (your "tough nuggies" situation above), you're probably right that it's inefficient, but does it matter, when it's going to be a slow timed-out operation anyway? The AUX failure will be much slower than the PM transition. I guess I can do this anyway, but frankly, I'll just be copy/pasting stuff from other drivers, because the runtime PM documentation still confuses me, and moreso once you involve autosuspend. > For a reference, you could look at > `drivers/gpu/drm/bridge/ti-sn65dsi86.c`. Also > `drivers/gpu/drm/bridge/parade-ps8640.c` Thanks for these. They look like reasonable patterns to follow. Brian
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On 2022-02-15 16:47, Jason Gunthorpe wrote: On Tue, Feb 15, 2022 at 04:35:56PM -0500, Felix Kuehling wrote: On 2022-02-15 14:41, Jason Gunthorpe wrote: On Tue, Feb 15, 2022 at 07:32:09PM +0100, Christoph Hellwig wrote: On Tue, Feb 15, 2022 at 10:45:24AM -0400, Jason Gunthorpe wrote: Do you know if DEVICE_GENERIC pages would end up as PageAnon()? My assumption was that they would be part of a special mapping. We need to stop using the special PTEs and VMAs for things that have a struct page. This is a mistake DAX created that must be undone. Yes, we'll get to it. Maybe we can do it for the non-DAX devmap ptes first given that DAX is more complicated. Probably, I think we can check the page->pgmap type to tell the difference. I'm not sure how the DEVICE_GENERIC can work without this, as DAX was made safe by using the unmap_mapping_range(), which won't work here. Is there some other trick being used to keep track of references inside the AMD driver? Not sure I'm following all the discussion about VMAs and DAX. So I may be answering the wrong question: We treat each ZONE_DEVICE page as a reference to the BO (buffer object) that backs the page. We increment the BO refcount for each page we migrate into it. In the dev_pagemap_ops.page_free callback we drop that reference. Once all pages backed by a BO are freed, the BO refcount reaches 0 [*] and we can free the BO allocation. Userspace does 1) mmap(MAP_PRIVATE) to allocate anon memory 2) something to trigger migration to install a ZONE_DEVICE page 3) munmap() Who decrements the refcout on the munmap? When a ZONE_DEVICE page is installed in the PTE is supposed to be marked as pte_devmap and that disables all the normal page refcounting during munmap(). fsdax makes this work by working the refcounts backwards, the page is refcounted while it exists in the driver, when the driver decides to remove it then unmap_mapping_range() is called to purge it from all PTEs and then refcount is decrd. munmap/fork/etc don't change the refcount. Hmm, that just means, whether or not there are PTEs doesn't really matter. It should work the same as it does for DEVICE_PRIVATE pages. I'm not sure where DEVICE_PRIVATE page's refcounts are decremented on unmap, TBH. But I can't find it in our driver, or in the test_hmm driver for that matter. Regards, Felix Jason
[PATCH 1/2] drm/mm: Add an iterator to optimally walk over holes for an allocation (v3)
This iterator relies on drm_mm_first_hole() and drm_mm_next_hole()
functions to identify suitable holes for an allocation of a given
size by efficiently traversing the rbtree associated with the given
allocator.
It replaces the for loop in drm_mm_insert_node_in_range() and can
also be used by drm drivers to quickly identify holes of a certain
size within a given range.
v2: (Tvrtko)
- Prepend a double underscore for the newly exported first/next_hole
- s/each_best_hole/each_suitable_hole/g
- Mask out DRM_MM_INSERT_ONCE from the mode before calling
first/next_hole and elsewhere.
v3: (Tvrtko)
- Reduce the number of hunks by retaining the "mode" variable name
Cc: Christian König
Reviewed-by: Tvrtko Ursulin
Suggested-by: Tvrtko Ursulin
Signed-off-by: Vivek Kasireddy
---
drivers/gpu/drm/drm_mm.c | 32 +++-
include/drm/drm_mm.h | 36
2 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 8257f9d4f619..8efea548ae9f 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -352,10 +352,10 @@ static struct drm_mm_node *find_hole_addr(struct drm_mm
*mm, u64 addr, u64 size)
return node;
}
-static struct drm_mm_node *
-first_hole(struct drm_mm *mm,
- u64 start, u64 end, u64 size,
- enum drm_mm_insert_mode mode)
+struct drm_mm_node *
+__drm_mm_first_hole(struct drm_mm *mm,
+ u64 start, u64 end, u64 size,
+ enum drm_mm_insert_mode mode)
{
switch (mode) {
default:
@@ -374,6 +374,7 @@ first_hole(struct drm_mm *mm,
hole_stack);
}
}
+EXPORT_SYMBOL(__drm_mm_first_hole);
/**
* DECLARE_NEXT_HOLE_ADDR - macro to declare next hole functions
@@ -410,11 +411,11 @@ static struct drm_mm_node *name(struct drm_mm_node
*entry, u64 size) \
DECLARE_NEXT_HOLE_ADDR(next_hole_high_addr, rb_left, rb_right)
DECLARE_NEXT_HOLE_ADDR(next_hole_low_addr, rb_right, rb_left)
-static struct drm_mm_node *
-next_hole(struct drm_mm *mm,
- struct drm_mm_node *node,
- u64 size,
- enum drm_mm_insert_mode mode)
+struct drm_mm_node *
+__drm_mm_next_hole(struct drm_mm *mm,
+ struct drm_mm_node *node,
+ u64 size,
+ enum drm_mm_insert_mode mode)
{
switch (mode) {
default:
@@ -432,6 +433,7 @@ next_hole(struct drm_mm *mm,
return &node->hole_stack == &mm->hole_stack ? NULL : node;
}
}
+EXPORT_SYMBOL(__drm_mm_next_hole);
/**
* drm_mm_reserve_node - insert an pre-initialized node
@@ -516,11 +518,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
u64 size, u64 alignment,
unsigned long color,
u64 range_start, u64 range_end,
- enum drm_mm_insert_mode mode)
+ enum drm_mm_insert_mode caller_mode)
{
struct drm_mm_node *hole;
u64 remainder_mask;
- bool once;
+ enum drm_mm_insert_mode mode = caller_mode & ~DRM_MM_INSERT_ONCE;
DRM_MM_BUG_ON(range_start > range_end);
@@ -533,13 +535,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
if (alignment <= 1)
alignment = 0;
- once = mode & DRM_MM_INSERT_ONCE;
- mode &= ~DRM_MM_INSERT_ONCE;
-
remainder_mask = is_power_of_2(alignment) ? alignment - 1 : 0;
- for (hole = first_hole(mm, range_start, range_end, size, mode);
-hole;
-hole = once ? NULL : next_hole(mm, hole, size, mode)) {
+ drm_mm_for_each_suitable_hole(hole, mm, range_start, range_end,
+ size, mode) {
u64 hole_start = __drm_mm_hole_node_start(hole);
u64 hole_end = hole_start + hole->hole_size;
u64 adj_start, adj_end;
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index ac33ba1b18bc..777f659f9692 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -400,6 +400,42 @@ static inline u64 drm_mm_hole_node_end(const struct
drm_mm_node *hole_node)
1 : 0; \
pos = list_next_entry(pos, hole_stack))
+struct drm_mm_node *
+__drm_mm_first_hole(struct drm_mm *mm,
+ u64 start, u64 end, u64 size,
+ enum drm_mm_insert_mode mode);
+
+struct drm_mm_node *
+__drm_mm_next_hole(struct drm_mm *mm,
+ struct drm_mm_node *node,
+ u64 size,
+ enum drm_mm_insert_mode mode);
+
+/**
+ * drm_mm_for_each_suitable_hole - iterator to optimally walk over all
+ * holes that can fit an allocation of the given @size.
+ * @pos: &drm_mm_node used internally to track progress
+ * @mm: &drm_mm allocator to walk
+ * @range_start: start of the allowed range for the allocat
Re: [PATCH] drm/panfrost: Dynamically allocate pm_domains
> I'd do the oneliner changing it to 5 and be done with it. That being > said, we have plenty of examples of doing this both ways, so whatever > makes people happy. Excellent, that's the patch I wrote originally :-) Dropping this patch, unless Angelo (or someone else) strongly objects.
Re: [PATCH v2 2/3] drm: Plumb debugfs_init through to panels
On 15.02.2022 23:09, Javier Martinez Canillas wrote:
Hello Doug,
On 2/5/22 01:13, Douglas Anderson wrote:
[snip]
+static void panel_bridge_debugfs_init(struct drm_bridge *bridge,
+ struct dentry *root)
+{
+ struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
+ struct drm_panel *panel = panel_bridge->panel;
+
+ root = debugfs_create_dir("panel", root);
This could return a ERR_PTR(-errno) if the function doesn't succeed.
I noticed that most kernel code doesn't check the return value though...
+ if (panel->funcs->debugfs_init)
Probably if (!(IS_ERR(root) && panel->funcs->debugfs_init) ?
+ panel->funcs->debugfs_init(panel, root);
+}
[snip]
@@ -436,6 +436,9 @@ void drm_debugfs_connector_add(struct drm_connector
*connector)
/* vrr range */
debugfs_create_file("vrr_range", S_IRUGO, root, connector,
&vrr_range_fops);
Same here, wonder if the return value should be checked.
I've seen sometimes that file/dir was already created with the same
name, reporting error in such case will be helpful.
Regards
Andrzej
I leave it to you to decide, but regardless of that the patch looks good to me.
Reviewed-by: Javier Martinez Canillas
Best regards,
Re: [PATCH v2 3/3] drm/panel-edp: Allow querying the detected panel via debugfs
On 2/5/22 01:13, Douglas Anderson wrote: > Recently we added generic "edp-panel"s probed by EDID. To support > panels in this way we look at the panel ID in the EDID and look up the > panel in a table that has power sequence timings. If we find a panel > that's not in the table we will still attempt to use it but we'll use > conservative timings. While it's likely that these conservative > timings will work for most nearly all panels, the performance of > turning the panel off and on suffers. > > We'd like to be able to reliably detect the case that we're using the > hardcoded timings without relying on parsing dmesg. This allows us to > implement tests that ensure that no devices get shipped that are > relying on the conservative timings. > > Let's add a new debugfs entry to panel devices. It will have one of: > * UNKNOWN - We tried to detect a panel but it wasn't in our table. > * HARDCODED - We're not using generic "edp-panel" probed by EDID. > * A panel name - This is the name of the panel from our table. > > Signed-off-by: Douglas Anderson > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v2 2/3] drm: Plumb debugfs_init through to panels
Hello Doug,
On 2/5/22 01:13, Douglas Anderson wrote:
[snip]
> +static void panel_bridge_debugfs_init(struct drm_bridge *bridge,
> + struct dentry *root)
> +{
> + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
> + struct drm_panel *panel = panel_bridge->panel;
> +
> + root = debugfs_create_dir("panel", root);
This could return a ERR_PTR(-errno) if the function doesn't succeed.
I noticed that most kernel code doesn't check the return value though...
> + if (panel->funcs->debugfs_init)
Probably if (!(IS_ERR(root) && panel->funcs->debugfs_init) ?
> + panel->funcs->debugfs_init(panel, root);
> +}
[snip]
> @@ -436,6 +436,9 @@ void drm_debugfs_connector_add(struct drm_connector
> *connector)
> /* vrr range */
> debugfs_create_file("vrr_range", S_IRUGO, root, connector,
> &vrr_range_fops);
Same here, wonder if the return value should be checked.
I leave it to you to decide, but regardless of that the patch looks good to me.
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH] drm/panfrost: Dynamically allocate pm_domains
On Mon, Feb 14, 2022 at 2:31 PM Alyssa Rosenzweig wrote: > > MT8192 requires 5 power domains. Rather than bump MAX_PM_DOMAINS and > waste memory on every supported Panfrost chip, instead dynamically > allocate pm_domain_devs and pm_domain_links. This adds some flexibility; > it seems inevitable a new MediaTek device will require more than 5 > domains. > > On non-MediaTek devices, this saves a small amount of memory. How much? You measured it? It's not that simple. kmalloc has finite allocation sizes (see /proc/slabinfo). So unless panfrost_device shrinks or grows to the next smaller or larger size, the memory used doesn't change. And each devm_kmalloc adds its own overhead as well. I'd do the oneliner changing it to 5 and be done with it. That being said, we have plenty of examples of doing this both ways, so whatever makes people happy. Rob
Re: [PATCH v2 1/3] migrate.c: Remove vma check in migrate_vma_setup()
On 2/6/22 20:26, Alistair Popple wrote:
migrate_vma_setup() checks that a valid vma is passed so that the page
tables can be walked to find the pfns associated with a given address
range. However in some cases the pfns are already known, such as when
migrating device coherent pages during pin_user_pages() meaning a valid
vma isn't required.
Signed-off-by: Alistair Popple
Acked-by: Felix Kuehling
---
Changes for v2:
- Added Felix's Acked-by
mm/migrate.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
Hi Alistair,
Another late-breaking review question, below. :)
diff --git a/mm/migrate.c b/mm/migrate.c
index a9aed12..0d6570d 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2602,24 +2602,24 @@ int migrate_vma_setup(struct migrate_vma *args)
args->start &= PAGE_MASK;
args->end &= PAGE_MASK;
- if (!args->vma || is_vm_hugetlb_page(args->vma) ||
- (args->vma->vm_flags & VM_SPECIAL) || vma_is_dax(args->vma))
- return -EINVAL;
- if (nr_pages <= 0)
- return -EINVAL;
Was the above check left out intentionally? If so, then it needs a
commit description note. And maybe even a separate patch, because it
changes the behavior.
If you do want to change the behavior:
* The kerneldoc comment above this function supports such a change: it
requires returning 0 for the case of zero pages requested. So your
change would bring the comments into alignment with the code.
* I don't think memset deals properly with a zero length input arg, so
it's probably better to return 0, before that point.
thanks,
--
John Hubbard
NVIDIA
- if (args->start < args->vma->vm_start ||
- args->start >= args->vma->vm_end)
- return -EINVAL;
- if (args->end <= args->vma->vm_start || args->end > args->vma->vm_end)
- return -EINVAL;
if (!args->src || !args->dst)
return -EINVAL;
-
- memset(args->src, 0, sizeof(*args->src) * nr_pages);
- args->cpages = 0;
- args->npages = 0;
-
- migrate_vma_collect(args);
+ if (args->vma) {
+ if (is_vm_hugetlb_page(args->vma) ||
+ (args->vma->vm_flags & VM_SPECIAL) ||
vma_is_dax(args->vma))
+ return -EINVAL;
+ if (args->start < args->vma->vm_start ||
+ args->start >= args->vma->vm_end)
+ return -EINVAL;
+ if (args->end <= args->vma->vm_start || args->end >
args->vma->vm_end)
+ return -EINVAL;
+
+ memset(args->src, 0, sizeof(*args->src) * nr_pages);
+ args->cpages = 0;
+ args->npages = 0;
+
+ migrate_vma_collect(args);
+ }
if (args->cpages)
migrate_vma_unmap(args);
@@ -2804,7 +2804,7 @@ void migrate_vma_pages(struct migrate_vma *migrate)
continue;
}
- if (!page) {
+ if (!page && migrate->vma) {
if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE))
continue;
if (!notified) {
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On Tue, Feb 15, 2022 at 04:35:56PM -0500, Felix Kuehling wrote: > > On 2022-02-15 14:41, Jason Gunthorpe wrote: > > On Tue, Feb 15, 2022 at 07:32:09PM +0100, Christoph Hellwig wrote: > > > On Tue, Feb 15, 2022 at 10:45:24AM -0400, Jason Gunthorpe wrote: > > > > > Do you know if DEVICE_GENERIC pages would end up as PageAnon()? My > > > > > assumption was that they would be part of a special mapping. > > > > We need to stop using the special PTEs and VMAs for things that have a > > > > struct page. This is a mistake DAX created that must be undone. > > > Yes, we'll get to it. Maybe we can do it for the non-DAX devmap > > > ptes first given that DAX is more complicated. > > Probably, I think we can check the page->pgmap type to tell the > > difference. > > > > I'm not sure how the DEVICE_GENERIC can work without this, as DAX was > > made safe by using the unmap_mapping_range(), which won't work > > here. Is there some other trick being used to keep track of references > > inside the AMD driver? > > Not sure I'm following all the discussion about VMAs and DAX. So I may be > answering the wrong question: We treat each ZONE_DEVICE page as a reference > to the BO (buffer object) that backs the page. We increment the BO refcount > for each page we migrate into it. In the dev_pagemap_ops.page_free callback > we drop that reference. Once all pages backed by a BO are freed, the BO > refcount reaches 0 [*] and we can free the BO allocation. Userspace does 1) mmap(MAP_PRIVATE) to allocate anon memory 2) something to trigger migration to install a ZONE_DEVICE page 3) munmap() Who decrements the refcout on the munmap? When a ZONE_DEVICE page is installed in the PTE is supposed to be marked as pte_devmap and that disables all the normal page refcounting during munmap(). fsdax makes this work by working the refcounts backwards, the page is refcounted while it exists in the driver, when the driver decides to remove it then unmap_mapping_range() is called to purge it from all PTEs and then refcount is decrd. munmap/fork/etc don't change the refcount. Jason
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On 2022-02-15 14:41, Jason Gunthorpe wrote: On Tue, Feb 15, 2022 at 07:32:09PM +0100, Christoph Hellwig wrote: On Tue, Feb 15, 2022 at 10:45:24AM -0400, Jason Gunthorpe wrote: Do you know if DEVICE_GENERIC pages would end up as PageAnon()? My assumption was that they would be part of a special mapping. We need to stop using the special PTEs and VMAs for things that have a struct page. This is a mistake DAX created that must be undone. Yes, we'll get to it. Maybe we can do it for the non-DAX devmap ptes first given that DAX is more complicated. Probably, I think we can check the page->pgmap type to tell the difference. I'm not sure how the DEVICE_GENERIC can work without this, as DAX was made safe by using the unmap_mapping_range(), which won't work here. Is there some other trick being used to keep track of references inside the AMD driver? Not sure I'm following all the discussion about VMAs and DAX. So I may be answering the wrong question: We treat each ZONE_DEVICE page as a reference to the BO (buffer object) that backs the page. We increment the BO refcount for each page we migrate into it. In the dev_pagemap_ops.page_free callback we drop that reference. Once all pages backed by a BO are freed, the BO refcount reaches 0 [*] and we can free the BO allocation. Regards, Felix [*] That's a somewhat simplified view. There may be other references to the BO, which allows us to reuse the same BO for the same virtual address range. Jason
Re: [PATCH v2] drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX
Hi,
On Fri, Oct 1, 2021 at 2:50 PM Brian Norris wrote:
>
> If the display is not enable()d, then we aren't holding a runtime PM
> reference here. Thus, it's easy to accidentally cause a hang, if user
> space is poking around at /dev/drm_dp_aux0 at the "wrong" time.
>
> Let's get the panel and PM state right before trying to talk AUX.
>
> Fixes: 0d97ad03f422 ("drm/bridge: analogix_dp: Remove duplicated code")
> Cc:
> Cc: Tomeu Vizoso
> Signed-off-by: Brian Norris
> ---
>
> Changes in v2:
> - Fix spelling in Subject
> - DRM_DEV_ERROR() -> drm_err()
> - Propagate errors from un-analogix_dp_prepare_panel()
>
> .../drm/bridge/analogix/analogix_dp_core.c| 21 ++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index b7d2e4449cfa..6fc46ac93ef8 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1632,8 +1632,27 @@ static ssize_t analogix_dpaux_transfer(struct
> drm_dp_aux *aux,
>struct drm_dp_aux_msg *msg)
> {
> struct analogix_dp_device *dp = to_dp(aux);
> + int ret, ret2;
>
> - return analogix_dp_transfer(dp, msg);
> + ret = analogix_dp_prepare_panel(dp, true, false);
> + if (ret) {
> + drm_err(dp->drm_dev, "Failed to prepare panel (%d)\n", ret);
> + return ret;
> + }
> +
> + pm_runtime_get_sync(dp->dev);
> + ret = analogix_dp_transfer(dp, msg);
> + pm_runtime_put(dp->dev);
I've spent an unfortunate amount of time digging around the DP AUX bus
recently, so I can at least say that I have some experience and some
opinions here.
IMO:
1. Don't power the panel on. If the panel isn't powered on then the DP
AUX transfer will timeout. Tough nuggies. Think of yourself more like
an i2c controller and of this as an i2c transfer implementation. The
i2c controller isn't in charge of powering up the i2c devices on the
bus. If userspace does an "i2c detect" on an i2c bus and some of the
devices aren't powered then they won't be found. If you try to
read/write from a powered off device that won't work either.
2. In theory if the DP driver can read HPD (I haven't looked through
the analogix code to see how it handles it) then you can fail an AUX
transfer right away if HPD isn't asserted instead of timing out. If
this is hard, it's probably fine to just time out though.
3. Do the "pm_runtime" calls, but enable "autosuspend" with something
~1 second autosuspend delay. When using the AUX bus to read an EDID
the underlying code will call your function 16 times in quick
succession. If you're powering up and down constantly that'll be a bit
of a waste.
The above will help set us up for when someone goes through and
enables the "new" DP AUX bus and generic eDP panels on for
analogix-edp. See commit aeb33699fc2c ("drm: Introduce the DP AUX
bus"). In that case panels will actually be instantiated DP AUX
Endpoints instead of platform devices. They'll be given the DP AUX bus
and they'll be able to read the EDID over that. In this case, the
panel code turns itself on (it knows how to turn itself on enough to
read the EDID) and then calls the DP AUX transfer code. :-)
For a reference, you could look at
`drivers/gpu/drm/bridge/ti-sn65dsi86.c`. Also
`drivers/gpu/drm/bridge/parade-ps8640.c`
-Doug
[PATCH v5 2/2] drm/msm/dp: enable widebus feature for display port
Widebus feature will transmit two pixel data per pixel clock to interface.
This feature now is required to be enabled to easy migrant to higher
resolution applications in future. However since some legacy chipsets
does not support this feature, this feature is enabled base on chip's
hardware revision.
changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
-- enable widebus feature base on chip hardware revision
Changes in v5:
-- DP_INTF_CONFIG_DATABUS_WIDEN
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 34 +++--
drivers/gpu/drm/msm/dp/dp_catalog.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_ctrl.c| 13 +++
drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 32 +++
drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
drivers/gpu/drm/msm/msm_drv.h | 6 +
10 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2b2dbb7..1e96cede 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2138,8 +2138,10 @@ int dpu_encoder_setup(struct drm_device *dev, struct
drm_encoder *enc,
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
- else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) {
dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
+ dpu_enc->wide_bus_en = msm_dp_wide_bus_enable(dpu_enc->dp);
+ }
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 6ae9b29..524eccc 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -24,6 +24,8 @@
#define DP_INTERRUPT_STATUS_ACK_SHIFT 1
#define DP_INTERRUPT_STATUS_MASK_SHIFT 2
+#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
+
#define DP_INTERRUPT_STATUS1 \
(DP_INTR_AUX_I2C_DONE| \
DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
@@ -483,6 +485,22 @@ int dp_catalog_ctrl_set_pattern(struct dp_catalog
*dp_catalog,
}
/**
+ * dp_catalog_hw_revision() - retrieve DP hw revision
+ *
+ * @dp_catalog: DP catalog structure
+ *
+ * Return: DP controller hw revision
+ *
+ */
+u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)
+{
+ struct dp_catalog_private *catalog = container_of(dp_catalog,
+ struct dp_catalog_private, dp_catalog);
+
+ return dp_read_ahb(catalog, REG_DP_HW_VERSION);
+}
+
+/**
* dp_catalog_ctrl_reset() - reset DP controller
*
* @dp_catalog: DP catalog structure
@@ -739,10 +757,11 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct dp_catalog
*dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, bool
wide_bus_en)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
+ u32 reg;
dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
dp_catalog->total);
@@ -751,7 +770,18 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
dp_catalog->width_blanking);
dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_catalog->dp_active);
- dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, 0);
+
+ reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
+
+ if (wide_bus_en)
+ reg |= DP_INTF_CONFIG_DATABUS_WIDEN;
+ else
+ reg &= ~DP_INTF_CONFIG_DATABUS_WIDEN;
+
+
+ DRM_DEBUG_DP("wide_bus_en=%d reg=%x\n", wide_bus_en, reg);
+
+ dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, reg);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 6965afa..2ba1ea4 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -95,6 +95,7 @@ void dp_catalog_ctrl_config_misc(struct dp_catalog
*dp_catalog, u32 cc, u32 tb);
void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 rate,
u32 stream_rate_khz, bool
[PATCH v5 1/2] drm/msm/dpu: revise timing engine programming to support widebus feature
Widebus feature will transmit two pixel data per pixel clock to interface.
Timing engine provides driving force for this purpose. This patch base
on HPG (Hardware Programming Guide) to revise timing engine register
setting to accommodate both widebus and non widebus application. Also
horizontal width parameters need to be reduced by half since two pixel
data are clocked out per pixel clock when widebus feature enabled.
Widebus can be enabled individually at DP. However at DSI, widebus have
to be enabled along with DSC to achieve pixel clock rate be scaled down
with same ratio as compression ratio when 10 bits per source component.
Therefore this patch add no supports of DSI related widebus and compression.
Changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
Changes in v4:
-- rework timing engine to not interfere with dsi/hdmi
-- cover both widebus and compression
Changes in v5:
-- remove supports of DSI widebus and compression
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 +
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 63 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 +
5 files changed, 76 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1e648db..2b2dbb7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -208,6 +208,8 @@ struct dpu_encoder_virt {
u32 idle_timeout;
+ bool wide_bus_en;
+
struct msm_dp *dp;
};
@@ -217,6 +219,14 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+
+bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc)
+{
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ return dpu_enc->wide_bus_en;
+}
+
static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned
bpc)
{
struct dpu_hw_dither_cfg dither_cfg = { 0 };
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index e241914..0d73550 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc);
*/
int dpu_encoder_get_vsync_count(struct drm_encoder *drm_enc);
+bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc);
+
#endif /* __DPU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index ddd9d89..04ac2dc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params(
timing->v_back_porch += timing->v_front_porch;
timing->v_front_porch = 0;
}
+
+ timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent);
+
+ /*
+* for DP, divide the horizonal parameters by 2 when
+* widebus is enabled
+*/
+ if (phys_enc->hw_intf->cap->type == INTF_DP && timing->wide_bus_en) {
+ timing->width = timing->width >> 1;
+ timing->xres = timing->xres >> 1;
+ timing->h_back_porch = timing->h_back_porch >> 1;
+ timing->h_front_porch = timing->h_front_porch >> 1;
+ timing->hsync_pulse_width = timing->hsync_pulse_width >> 1;
+ }
}
static u32 get_horizontal_total(const struct intf_timing_params *timing)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 116e2b5..303bd03 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -33,6 +33,7 @@
#define INTF_TP_COLOR1 0x05C
#define INTF_CONFIG20x060
#define INTF_DISPLAY_DATA_HCTL 0x064
+#define INTF_ACTIVE_DATA_HCTL 0x068
#define INTF_FRAME_LINE_COUNT_EN0x0A8
#define INTF_FRAME_COUNT0x0AC
#define INTF_LINE_COUNT 0x0B0
@@ -60,6 +61,14 @@
#define INTF_MUX 0x25C
+#define INTF_CFG_ACTIVE_H_EN BIT(29)
+#define INTF_CFG_ACTIVE_V_EN BIT(30)
+
+#define INTF_CFG2_DATABUS_WIDENBIT(0)
+#define INTF_CFG2_DATA_HCTL_EN BIT(4)
+#define INTF_CFG2_DCE_DATA_COMPRESSBIT(12)
+
+
static const struct dpu_intf_cfg *_intf_offset(enum dpu_intf intf,
const struct dpu_mdss_cfg *m,
[PATCH v5 0/2] drm/msm/dp: enable widebus feature base on chip hardware revision
revise widebus timing engine programming and enable widebus feature base on chip hardware revision Kuogee Hsieh (2): drm/msm/dpu: revise timing engine programming to support widebus feature drm/msm/dp: enable widebus feature for display port drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 14 - drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 + .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 63 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 + drivers/gpu/drm/msm/dp/dp_catalog.c| 34 +++- drivers/gpu/drm/msm/dp/dp_catalog.h| 3 +- drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++-- drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c| 32 +++ drivers/gpu/drm/msm/dp/dp_display.h| 2 + drivers/gpu/drm/msm/dp/dp_panel.c | 4 +- drivers/gpu/drm/msm/dp/dp_panel.h | 2 +- drivers/gpu/drm/msm/msm_drv.h | 6 +++ 14 files changed, 166 insertions(+), 26 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 22/23] video: omapfb: dss: Make use of the helper component_compare_dev
On 2/14/22 07:08, Yong Wu wrote:
> Use the common compare helper from component.
>
> Cc: Helge Deller
> Cc: linux-o...@vger.kernel.org
> Cc: linux-fb...@vger.kernel.org
> Signed-off-by: Yong Wu
Applied to the fbdev for-next branch.
Thanks!
Helge
> drivers/video/fbdev/omap2/omapfb/dss/dss.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> index a6b1c1598040..45b9d3cf3860 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> @@ -1193,12 +1193,6 @@ static const struct component_master_ops
> dss_component_ops = {
> .unbind = dss_unbind,
> };
>
> -static int dss_component_compare(struct device *dev, void *data)
> -{
> - struct device *child = data;
> - return dev == child;
> -}
> -
> static int dss_add_child_component(struct device *dev, void *data)
> {
> struct component_match **match = data;
> @@ -1212,7 +1206,7 @@ static int dss_add_child_component(struct device *dev,
> void *data)
> if (strstr(dev_name(dev), "rfbi"))
> return 0;
>
> - component_match_add(dev->parent, match, dss_component_compare, dev);
> + component_match_add(dev->parent, match, component_compare_dev, dev);
>
> return 0;
> }
>
Re: [PATCH 0/3] video: fbdev: atari: Miscellaneous fixes
Hi Geert, On 2/15/22 12:21, Geert Uytterhoeven wrote: > This is a small series of miscellaneous fixes for the Atari frame buffer > device driver. > > The first patch has been sent before, and is untested, as I have no > access to the hardware. > The other patches have been tested on ARAnyM. > > Thanks! > > Geert Uytterhoeven (3): > video: fbdev: atari: Fix TT High video mode > video: fbdev: atari: Convert to standard round_up() helper > video: fbdev: atari: Remove unused atafb_setcolreg() > > drivers/video/fbdev/atafb.c | 23 +-- > 1 file changed, 5 insertions(+), 18 deletions(-) I've applied all 3 patches to the fbdev for-next tree. Thank you! Helge
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On 2/15/2022 10:42 AM, Dmitry Baryshkov wrote:
On Tue, 15 Feb 2022 at 20:42, Abhinav Kumar wrote:
On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
From: Rob Clark
Add SC8180x to the hardware catalog, for initial support for the
platform. Due to limitations in the DP driver only one of the four DP
interfaces is left enabled.
The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
is flagged appropriately to ensure widebus is disabled - for now.
Signed-off-by: Rob Clark
[bjorn: Reworked intf and irq definitions]
Signed-off-by: Bjorn Andersson
---
Changes since v1:
- Dropped widebus flag
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
drivers/gpu/drm/msm/msm_drv.c | 1 +
4 files changed, 132 insertions(+)
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 aa75991903a6..7ac0fe32df49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -90,6 +90,17 @@
BIT(MDP_INTF3_INTR) | \
BIT(MDP_INTF4_INTR))
+#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+ BIT(MDP_SSPP_TOP0_INTR2) | \
+ BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+ BIT(MDP_INTF0_INTR) | \
+ BIT(MDP_INTF1_INTR) | \
+ BIT(MDP_INTF2_INTR) | \
+ BIT(MDP_INTF3_INTR) | \
+ BIT(MDP_INTF4_INTR) | \
+ BIT(MDP_INTF5_INTR) | \
+ BIT(MDP_AD4_0_INTR) | \
+ BIT(MDP_AD4_1_INTR))
#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH 2048
@@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
.max_vdeci_exp = MAX_VERT_DECIMATION,
};
+static const struct dpu_caps sc8180x_dpu_caps = {
+ .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+ .max_mixer_blendstages = 0xb,
+ .qseed_type = DPU_SSPP_SCALER_QSEED3,
+ .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
+ .ubwc_version = DPU_HW_UBWC_VER_30,
+ .has_src_split = true,
+ .has_dim_layer = true,
+ .has_idle_pc = true,
+ .has_3d_merge = true,
+ .max_linewidth = 4096,
+ .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+ .max_hdeci_exp = MAX_HORZ_DECIMATION,
+ .max_vdeci_exp = MAX_VERT_DECIMATION,
+};
+
static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
@@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
},
};
+static const struct dpu_mdp_cfg sc8180x_mdp[] = {
+ {
+ .name = "top_0", .id = MDP_TOP,
+ .base = 0x0, .len = 0x45C,
+ .features = 0,
+ .highest_bank_bit = 0x3,
+ .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
+ .reg_off = 0x2AC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
+ .reg_off = 0x2B4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
+ .reg_off = 0x2BC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
+ .reg_off = 0x2C4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
+ .reg_off = 0x2AC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
+ .reg_off = 0x2B4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .reg_off = 0x2BC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .reg_off = 0x2C4, .bit_off = 8},
+ },
+};
+
static const struct dpu_mdp_cfg sm8250_mdp[] = {
{
.name = "top_0", .id = MDP_TOP,
@@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
+static const struct dpu_intf_cfg sc8180x_intf[] = {
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
+ INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 28, 29),
+ /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until this
is supported */
+ INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 30, 31),
+ INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21),
+ INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, MSM_DP_CONTROLLER_2, 24,
INTF_SC7180_MASK, M
Re: [PATCH] drm: fb-helper: Avoid nesting spinlock_t into raw_spinlock_t
On Tue, 15 Feb 2022, John Ogness wrote: > >> drm_fb_helper_damage() is acquiring spinlock_t (helper->damage_lock), > >> while it can be called from contexts where raw_spinlock_t is held (e.g. > >> console_owner lock obtained on vprintk_emit() codepath). > >> > >> As the critical sections protected by damage_lock are super-tiny, let's > >> fix this by converting it to raw_spinlock_t in order not to violate > >> PREEMPT_RT-imposed lock nesting rules. > >> > >> This fixes the splat below. > >> > >> = > >> [ BUG: Invalid wait context ] > >> 5.17.0-rc4-2-gd567f5db412e #1 Not tainted > > > > rc4. Is this also the case in the RT tree which includes John's printk > > changes? > > In the RT tree, the fbcon's write() callback is only called in > preemptible() contexts. So this is only a mainline issue. > > The series I recently posted to LKML [0] should also address this issue > (if/when it gets accepted). > > John > > [0] > https://lore.kernel.org/lkml/20220207194323.273637-1-john.ogn...@linutronix.de Thanks for confirmation, seems like this problem is indeed cured by your series. I still believe though that we shouldn't let 5.17 released with this warning being emitted into dmesg, so I'd suggest going with my patch for mainline, and revert it in your series on top of it. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On Tue, Feb 15, 2022 at 07:32:09PM +0100, Christoph Hellwig wrote: > On Tue, Feb 15, 2022 at 10:45:24AM -0400, Jason Gunthorpe wrote: > > > Do you know if DEVICE_GENERIC pages would end up as PageAnon()? My > > > assumption was that they would be part of a special mapping. > > > > We need to stop using the special PTEs and VMAs for things that have a > > struct page. This is a mistake DAX created that must be undone. > > Yes, we'll get to it. Maybe we can do it for the non-DAX devmap > ptes first given that DAX is more complicated. Probably, I think we can check the page->pgmap type to tell the difference. I'm not sure how the DEVICE_GENERIC can work without this, as DAX was made safe by using the unmap_mapping_range(), which won't work here. Is there some other trick being used to keep track of references inside the AMD driver? Jason
Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce format modifier for DG2 clear color
On 15.2.2022 20.24, Chery, Nanley G wrote:
-Original Message-
From: Juha-Pekka Heikkila
Sent: Tuesday, February 15, 2022 9:32 AM
To: Chery, Nanley G ; Nanley Chery
; C, Ramalingam
Cc: intel-gfx ; Auld, Matthew
; dri-devel
Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce format
modifier for DG2 clear color
On 15.2.2022 18.44, Chery, Nanley G wrote:
-Original Message-
From: Juha-Pekka Heikkila
Sent: Tuesday, February 15, 2022 8:15 AM
To: Chery, Nanley G ; Nanley Chery
; C, Ramalingam
Cc: intel-gfx ; Auld, Matthew
; dri-devel
Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce
format modifier for DG2 clear color
On 15.2.2022 17.02, Chery, Nanley G wrote:
-Original Message-
From: Juha-Pekka Heikkila
Sent: Tuesday, February 15, 2022 6:56 AM
To: Nanley Chery ; C, Ramalingam
Cc: intel-gfx ; Chery, Nanley G
; Auld, Matthew ;
dri- devel
Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce
format modifier for DG2 clear color
On 12.2.2022 3.19, Nanley Chery wrote:
On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C
wrote:
From: Mika Kahola
DG2 clear color render compression uses Tile4 layout. Therefore,
we need to define a new format modifier for uAPI to support clear
color
rendering.
v2:
Display version is fixed. [Imre]
KDoc is enhanced for cc modifier. [Nanley & Lionel]
Signed-off-by: Mika Kahola
cc: Anshuman Gupta
Signed-off-by: Juha-Pekka Heikkilä
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/display/intel_fb.c| 8
drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 -
include/uapi/drm/drm_fourcc.h | 10 ++
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 4d4d01963f15..3df6ef5ffec5 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -144,6 +144,12 @@ static const struct intel_modifier_desc
intel_modifiers[] = {
.modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS,
.display_ver = { 13, 13 },
.plane_caps = INTEL_PLANE_CAP_TILING_4 |
INTEL_PLANE_CAP_CCS_MC,
+ }, {
+ .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC,
+ .display_ver = { 13, 13 },
+ .plane_caps = INTEL_PLANE_CAP_TILING_4 |
+ INTEL_PLANE_CAP_CCS_RC_CC,
+
+ .ccs.cc_planes = BIT(1),
}, {
.modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS,
.display_ver = { 13, 13 }, @@ -559,6 +565,7 @@
intel_tile_width_bytes(const struct drm_framebuffer *fb, int
color_plane)
else
return 512;
case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
+ case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
case I915_FORMAT_MOD_4_TILED:
/*
@@ -763,6 +770,7 @@ unsigned int intel_surf_alignment(const
struct
drm_framebuffer *fb,
case I915_FORMAT_MOD_Yf_TILED:
return 1 * 1024 * 1024;
case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
+ case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
return 16 * 1024;
default:
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index c38ae0876c15..b4dced1907c5 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -772,6 +772,8 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
return PLANE_CTL_TILED_4 |
PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE |
PLANE_CTL_CLEAR_COLOR_DISABLE;
+ case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
+ return PLANE_CTL_TILED_4 |
+ PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
case I915_FORMAT_MOD_Y_TILED_CCS:
case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
return PLANE_CTL_TILED_Y |
PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
@@ -2358,10 +2360,15 @@ skl_get_initial_plane_config(struct
intel_crtc
*crtc,
break;
case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+
*/
if (HAS_4TILE(dev_priv)) {
- if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
+ u32 rc_mask =
PLANE_CTL_RENDER_DECOMPRESSION_ENABLE |
+
+ PLANE_CTL_CLEAR_COLOR_DISABLE;
+
+ if ((val & rc_mask) == rc_mask)
fb->modifier =
I915_FORMAT_MOD_4_TILED_DG2_RC_CCS;
else if (val &
PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE)
fb->modifier =
I915_FORMA
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Tue, Feb 15, 2022 at 09:19:29PM +0200, Leon Romanovsky wrote: > On Tue, Feb 15, 2022 at 01:21:10PM -0600, Gustavo A. R. Silva wrote: > > On Tue, Feb 15, 2022 at 10:17:40AM -0800, Kees Cook wrote: > > > On Tue, Feb 15, 2022 at 11:47:43AM -0600, Gustavo A. R. Silva wrote: > > > > > > These all look trivially correct to me. Only two didn't have the end of > > > the struct visible in the patch, and checking those showed them to be > > > trailing members as well, so: > > > > > > Reviewed-by: Kees Cook > > > > I'll add this to my -next tree. > > I would like to ask you to send mlx5 patch separately to netdev. We are > working > to delete that file completely and prefer to avoid from unnecessary merge > conflicts. Oh OK. Sure thing; I will do so. Thanks -- Gustavo
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Tue, Feb 15, 2022 at 01:21:10PM -0600, Gustavo A. R. Silva wrote:
> On Tue, Feb 15, 2022 at 10:17:40AM -0800, Kees Cook wrote:
> > On Tue, Feb 15, 2022 at 11:47:43AM -0600, Gustavo A. R. Silva wrote:
> > > There is a regular need in the kernel to provide a way to declare
> > > having a dynamically sized set of trailing elements in a structure.
> > > Kernel code should always use “flexible array members”[1] for these
> > > cases. The older style of one-element or zero-length arrays should
> > > no longer be used[2].
> > >
> > > This code was transformed with the help of Coccinelle:
> > > (next-20220214$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> > > script.cocci --include-headers --dir . > output.patch)
> > >
> > > @@
> > > identifier S, member, array;
> > > type T1, T2;
> > > @@
> > >
> > > struct S {
> > > ...
> > > T1 member;
> > > T2 array[
> > > - 0
> > > ];
> > > };
> >
> > These all look trivially correct to me. Only two didn't have the end of
> > the struct visible in the patch, and checking those showed them to be
> > trailing members as well, so:
> >
> > Reviewed-by: Kees Cook
>
> I'll add this to my -next tree.
I would like to ask you to send mlx5 patch separately to netdev. We are working
to delete that file completely and prefer to avoid from unnecessary merge
conflicts.
Thanks
>
> Thanks!
> --
> Gustavo
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Tue, Feb 15, 2022 at 10:17:40AM -0800, Kees Cook wrote:
> On Tue, Feb 15, 2022 at 11:47:43AM -0600, Gustavo A. R. Silva wrote:
> > There is a regular need in the kernel to provide a way to declare
> > having a dynamically sized set of trailing elements in a structure.
> > Kernel code should always use “flexible array members”[1] for these
> > cases. The older style of one-element or zero-length arrays should
> > no longer be used[2].
> >
> > This code was transformed with the help of Coccinelle:
> > (next-20220214$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> > script.cocci --include-headers --dir . > output.patch)
> >
> > @@
> > identifier S, member, array;
> > type T1, T2;
> > @@
> >
> > struct S {
> > ...
> > T1 member;
> > T2 array[
> > - 0
> > ];
> > };
>
> These all look trivially correct to me. Only two didn't have the end of
> the struct visible in the patch, and checking those showed them to be
> trailing members as well, so:
>
> Reviewed-by: Kees Cook
I'll add this to my -next tree.
Thanks!
--
Gustavo
Re: [PATCH v4 00/10] Overhaul `is_thunderbolt`
On 2/15/2022 01:29, Lukas Wunner wrote: On Mon, Feb 14, 2022 at 06:01:50PM -0600, Mario Limonciello wrote: drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 2 +- drivers/gpu/drm/nouveau/nouveau_vga.c | 4 +- drivers/gpu/drm/radeon/radeon_device.c | 4 +- drivers/gpu/drm/radeon/radeon_kms.c | 2 +- drivers/pci/hotplug/pciehp_hpc.c| 6 +- drivers/pci/pci-acpi.c | 15 - drivers/pci/pci.c | 17 +++-- drivers/pci/probe.c | 52 ++- drivers/pci/quirks.c| 84 + drivers/platform/x86/apple-gmux.c | 2 +- drivers/thunderbolt/nhi.h | 2 - include/linux/pci.h | 25 +--- include/linux/pci_ids.h | 3 + 14 files changed, 173 insertions(+), 47 deletions(-) That's an awful lot of additional LoC for what is primarily a refactoring job with the intent to simplify things. You may recall the first version of this series was just for adding USB4 matches to the existing code paths, and that's when it was noted that is_thunderbolt is a bit overloaded. Honestly this looks like an attempt to fix something that isn't broken. Specifically, the is_thunderbolt bit apparently can't be removed without adding new bits to struct pci_dev. Not sure if that can be called progress. > Thanks, Lukas Within this series there are two new material patches; setting up root ports for both integrated and discrete USB4 controllers to behave well with all the existing drivers that rely upon a hint of how they're connected to configure devices differently. If y'all collectively prefer this direction to not refactor is_thunderbolt and push into quirks, a simpler version of this series would be to leave all the quirks in place, just drop dev->is_thunderbolt, and set dev->external_facing on all 3 cases: * Intel TBT controller * USB4 integrated PCIe tunneling root port/XHCI tunneling root port * USB4 disctete PCIe tunneling root port/XHCI tunneling root port All the other drivers and symbols can stay the same then.
Re: [PATCH 2/6] drm/ttm: add resource iterator v3
On 2022-02-15 12:22, Christian König wrote:
Instead of duplicating that at different places add an iterator over all
the resources in a resource manager.
v2: add lockdep annotation and kerneldoc
v3: fix various bugs pointed out by Felix
Signed-off-by: Christian König
Tested-by: Bas Nieuwenhuizen
Reviewed-by: Daniel Vetter (v3)
---
drivers/gpu/drm/ttm/ttm_bo.c | 42 ++--
drivers/gpu/drm/ttm/ttm_device.c | 26 +++
drivers/gpu/drm/ttm/ttm_resource.c | 51 ++
include/drm/ttm/ttm_resource.h | 23 ++
4 files changed, 102 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb0fa932d495..11e698e3374c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -579,38 +579,30 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource_cursor cursor;
struct ttm_resource *res;
bool locked = false;
- unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(res, &man->lru[i], lru) {
- bool busy;
-
- bo = res->bo;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
- &locked, &busy)) {
- if (busy && !busy_bo && ticket !=
- dma_resv_locking_ctx(bo->base.resv))
- busy_bo = bo;
- continue;
- }
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
- break;
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
+ &locked, &busy)) {
+ if (busy && !busy_bo && ticket !=
+ dma_resv_locking_ctx(res->bo->base.resv))
+ busy_bo = res->bo;
+ continue;
}
- /* If the inner loop terminated early, we have our candidate */
- if (&res->lru != &man->lru[i])
- break;
-
- bo = NULL;
+ if (!ttm_bo_get_unless_zero(res->bo)) {
Just an unrelated nit-pick: If you invert the logic of this if, the loop
behaves more like a normal loop (without a break at the very end, and
one less continue statement):
if (ttm_bo_get_unless_zero(res->bo)) {
bo = res->bo;
break;
}
if (locked)
dma_resv_unlock(res->bo->base.resv);
Either way, the patch is
Reviewed-by: Felix Kuehling
+ if (locked)
+ dma_resv_unlock(res->bo->base.resv);
+ continue;
+ }
+ bo = res->bo;
+ break;
}
if (!bo) {
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index ba35887147ba..a0562ab386f5 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout);
int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
gfp_t gfp_flags)
{
+ struct ttm_resource_cursor cursor;
struct ttm_resource_manager *man;
- struct ttm_buffer_object *bo;
struct ttm_resource *res;
- unsigned i, j;
+ unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
@@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct
ttm_operation_ctx *ctx,
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(res, &man->lru[j], lru) {
- uint32_t num_pages;
-
- bo = res->bo;
- num_pages = PFN_UP(bo->base.size);
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ struct ttm_buffer_object *bo = res->bo;
+ uint32_t num_pages = PFN_UP(bo->base.size);
-ret = ttm_bo_swapout(bo, ctx, gfp_flags);
- /* ttm_bo_swapout has dropped the lru_lock */
- if (!ret)
- return num_pages;
- if (ret != -EBUSY)
-
Re: [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop with simpler while
On Tue, Feb 15, 2022 at 07:14:49PM +0200, Jani Nikula wrote: > On Tue, 15 Feb 2022, Andy Shevchenko > wrote: > > It's hard to parse for-loop which has some magic calculations inside. > > Much cleaner to use while-loop directly. > > I assume you're trying to prove a point following our recent > for-vs-while loop discussion. I really can't think of any other reason > you'd end up looking at this file or this loop. > > With the change, the loop indeed becomes simpler, but it also runs one > iteration further than the original. Whoops. Yeah, sorry for that, the initial condition should be d = depth - 1, of course. > It's a selftest. The loop's been there for five years. What are we > trying to achieve here? So we disagree on loops, fine. Perhaps this is > not the best use of either of our time? Please just let the for loops in > i915 be. Yes, I'm pretty much was sure that no-one will go and apply this anyway (so I haven't paid too much attention), but to prove my point in the certain discussion. And yes, the point is for the new code, I'm not going to change existing suboptimal and too hard to read for-loops, it will consume my time later when I will try to understand the code. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2] drm/i915: fix build issue when using clang
On Mon, Feb 14, 2022 at 11:58:20AM -0800, Tong Zhang wrote:
> drm/i915 adds some extra cflags, namely -Wall, which causes
> instances of -Wformat-security to appear when building with clang, even
> though this warning is turned off kernel-wide in the main Makefile:
>
> > drivers/gpu/drm/i915/gt/intel_gt.c:983:2: error: format string is not a
> > string literal (potentially insecure) [-Werror,-Wformat-security]
> > GEM_TRACE("ERROR\n");
> > ^~~~
> > ./drivers/gpu/drm/i915/i915_gem.h:76:24: note: expanded from macro
> > 'GEM_TRACE'
> > #define GEM_TRACE(...) trace_printk(__VA_ARGS__)
> >^
> > ./include/linux/kernel.h:369:3: note: expanded from macro 'trace_printk'
> > do_trace_printk(fmt, ##__VA_ARGS__);\
> > ^~~
> > ./include/linux/kernel.h:383:30: note: expanded from macro 'do_trace_printk'
> > __trace_bprintk(_THIS_IP_, trace_printk_fmt, ##args); \
> > ^~~~
> >drivers/gpu/drm/i915/gt/intel_gt.c:983:2: note: treat the string as an
> >argument to avoid this
>
> This does not happen with GCC because it does not enable
> -Wformat-security with -Wall. Disable -Wformat-security within the i915
> Makefile so that these warnings do not show up with clang.
>
> Signed-off-by: Tong Zhang
Given this is not enabled for GCC and it is disabled in the main
Makefile:
Reviewed-by: Nathan Chancellor
Additionally, it seems like trace_printk() is designed to be able to
take a string literal without a format argument, so this should be fine.
> ---
>
> v2: revise commit message
>
> drivers/gpu/drm/i915/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1b62b9f65196..c04e05a3d39f 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -13,6 +13,7 @@
> # will most likely get a sudden build breakage... Hopefully we will fix
> # new warnings before CI updates!
> subdir-ccflags-y := -Wall -Wextra
> +subdir-ccflags-y += -Wno-format-security
> subdir-ccflags-y += -Wno-unused-parameter
> subdir-ccflags-y += -Wno-type-limits
> subdir-ccflags-y += -Wno-missing-field-initializers
> --
> 2.25.1
>
>
Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
On 2022-02-15 07:15, David Hildenbrand wrote: On 11.02.22 17:56, Jason Gunthorpe wrote: On Fri, Feb 11, 2022 at 05:49:08PM +0100, David Hildenbrand wrote: On 11.02.22 17:45, Jason Gunthorpe wrote: On Fri, Feb 11, 2022 at 05:15:25PM +0100, David Hildenbrand wrote: ... I'm pretty sure we cannot FOLL_PIN DEVICE_PRIVATE pages Currently the only way to get a DEVICE_PRIVATE page out of the page tables is via hmm_range_fault() and that doesn't manipulate any ref counts. Thanks for clarifying Jason! ... and AFAIU, device exclusive entries are essentially just pointers at ordinary PageAnon() pages. So with DEVICE COHERENT we'll have the first PageAnon() ZONE_DEVICE pages mapped as present in the page tables where GUP could FOLL_PIN them. This is my understanding Though you probably understand what PageAnon means alot better than I do.. I wonder if it really makes sense to talk about that together with ZONE_DEVICE which has alot in common with filesystem originated pages too. For me, PageAnon() means that modifications are visible only to the modifying process. On actual CoW, the underlying page will get replaced -- in the world of DEVICE_COHERENT that would mean that once you write to a DEVICE_COHERENT you could suddenly have a !DEVICE_COHERENT page. PageAnon() pages don't have a mapping, thus they can only be found in MAP_ANON VMAs or in MAP_SHARED VMAs with MAP_PRIVATE. They can only be found via a page table, and not looked up via the page cache (excluding the swap cache). So if we have PageAnon() pages on ZONE_DEVICE, they generally have the exact same semantics as !ZONE_DEVICE pages, but the way they "appear" in the page tables the allocation/freeing path differs -- I guess :) ... and as we want pinning semantics to be different we have to touch GUP. I'm not sure what AMDs plan is here, is there an expecation that a GPU driver will somehow stuff these pages into an existing anonymous memory VMA or do they always come from a driver originated VMA? My understanding is that a driver can just decide to replace "ordinary" PageAnon() pages e.g., in a MAP_ANON VMA by these pages. Hopefully AMD can clarify. Yes. DEVICE_COHERENT pages are very similar to DEVICE_PRIVATE. They are in normal anonymous VMAs (e.g. the application called mmap(..., MAP_ANONYMOUS, ...)). You get DEVICE_PRIVATE/COHERENT pages as a result of the driver migrating normal anonymous pages into device memory. The main difference is, that DEVICE_PRIVATE pages aren't host accessible, while DEVICE_COHERENT pages are host-accessible and can be mapped in the CPU page table or DMA-mapped by peer devices. That's why GUP needs to deal with them. Regards, Felix
Re: [PATCH v4 2/2] drm/msm/dp: enable widebus feature for display port
On 15/02/2022 21:39, Kuogee Hsieh wrote:
On 2/15/2022 10:30 AM, Dmitry Baryshkov wrote:
On Tue, 15 Feb 2022 at 20:49, Kuogee Hsieh
wrote:
On 2/15/2022 5:34 AM, Dmitry Baryshkov wrote:
On 15/02/2022 01:39, Kuogee Hsieh wrote:
Widebus feature will transmit two pixel data per pixel clock to
interface.
This feature now is required to be enabled to easy migrant to higher
resolution applications in future. However since some legacy chipsets
does not support this feature, this feature is enabled base on chip's
hardware revision.
changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
-- enable widebus feature base on chip hardware revision
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 36
+++--
drivers/gpu/drm/msm/dp/dp_catalog.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++
drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 30
drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
drivers/gpu/drm/msm/msm_drv.h | 6 +
10 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0c22839..b2d23c2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2167,8 +2167,10 @@ int dpu_encoder_setup(struct drm_device *dev,
struct drm_encoder *enc,
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
- else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) {
dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
+ dpu_enc->wide_bus_en = msm_dp_wide_bus_enable(dpu_enc->dp);
+ }
Please revert the order of the patches and move this chunk into the
DPU patch.
There is chicken and egg issue here.
I think we should create an communication channel to pass dp/dsi driver
info to drm, such as wide_bus_en and dsc compression, etc.
By function calls like this will create more unnecessary tangles between
driver and drm layer.
I'm fine with the function call. I'm suggesting another thing. If you
change the order of these two patches (#1 - dp, #2 - dpu), the
msm_dp_wide_bus_enable() will get added in patch#1 and will be
available to be called in the patch#2.
By doing that, we have wile widebus enabled at (#1 -dp) patch but has no
timing engine changes at (#2 -dpu) to support it. This means (#1 -dp)
will break dp until (#2 -dpu) merged.
Is this ok?
Ah, I missed this. Please excuse me. It's fine to leave the order as is
then.
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 64f0b26..99d087e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -483,6 +483,27 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct
dp_catalog *dp_catalog,
}
/**
+ * dp_catalog_hw_revision() - retrieve DP hw revision
+ *
+ * @dp_catalog: DP catalog structure
+ *
+ * return: u32
+ *
+ * This function return the DP controller hw revision
+ *
+ */
+u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)
+{
+ u32 revision;
+ struct dp_catalog_private *catalog = container_of(dp_catalog,
+ struct dp_catalog_private, dp_catalog);
+
+ revision = dp_read_ahb(catalog, REG_DP_HW_VERSION);
+
+ return revision;
+}
+
+/**
* dp_catalog_ctrl_reset() - reset DP controller
*
* @dp_catalog: DP catalog structure
@@ -739,10 +760,11 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct
dp_catalog *dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, bool
wide_bus_en)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
+ u32 reg;
dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
dp_catalog->total);
@@ -751,7 +773,17 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
dp_catalog->width_blanking);
dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER,
dp_catalog->dp_active);
- dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, 0);
+
+ reg = dp_read_p0(c
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On Tue, 15 Feb 2022 at 20:42, Abhinav Kumar wrote:
> On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
> > On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
> >
> >>
> >>
> >> On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
> >>> From: Rob Clark
> >>>
> >>> Add SC8180x to the hardware catalog, for initial support for the
> >>> platform. Due to limitations in the DP driver only one of the four DP
> >>> interfaces is left enabled.
> >>>
> >>> The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
> >>> the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
> >>> is flagged appropriately to ensure widebus is disabled - for now.
> >>>
> >>> Signed-off-by: Rob Clark
> >>> [bjorn: Reworked intf and irq definitions]
> >>> Signed-off-by: Bjorn Andersson
> >>> ---
> >>>
> >>> Changes since v1:
> >>> - Dropped widebus flag
> >>>
> >>>.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
> >>>.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> >>>drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> >>>drivers/gpu/drm/msm/msm_drv.c | 1 +
> >>>4 files changed, 132 insertions(+)
> >>>
> >>> 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 aa75991903a6..7ac0fe32df49 100644
> >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>> @@ -90,6 +90,17 @@
> >>> BIT(MDP_INTF3_INTR) | \
> >>> BIT(MDP_INTF4_INTR))
> >>> +#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
> >>> + BIT(MDP_SSPP_TOP0_INTR2) | \
> >>> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> >>> + BIT(MDP_INTF0_INTR) | \
> >>> + BIT(MDP_INTF1_INTR) | \
> >>> + BIT(MDP_INTF2_INTR) | \
> >>> + BIT(MDP_INTF3_INTR) | \
> >>> + BIT(MDP_INTF4_INTR) | \
> >>> + BIT(MDP_INTF5_INTR) | \
> >>> + BIT(MDP_AD4_0_INTR) | \
> >>> + BIT(MDP_AD4_1_INTR))
> >>>#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
> >>>#define DEFAULT_DPU_LINE_WIDTH 2048
> >>> @@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
> >>> .max_vdeci_exp = MAX_VERT_DECIMATION,
> >>>};
> >>> +static const struct dpu_caps sc8180x_dpu_caps = {
> >>> + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> >>> + .max_mixer_blendstages = 0xb,
> >>> + .qseed_type = DPU_SSPP_SCALER_QSEED3,
> >>> + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
> >>> + .ubwc_version = DPU_HW_UBWC_VER_30,
> >>> + .has_src_split = true,
> >>> + .has_dim_layer = true,
> >>> + .has_idle_pc = true,
> >>> + .has_3d_merge = true,
> >>> + .max_linewidth = 4096,
> >>> + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> >>> + .max_hdeci_exp = MAX_HORZ_DECIMATION,
> >>> + .max_vdeci_exp = MAX_VERT_DECIMATION,
> >>> +};
> >>> +
> >>>static const struct dpu_caps sm8250_dpu_caps = {
> >>> .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> >>> .max_mixer_blendstages = 0xb,
> >>> @@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
> >>> },
> >>>};
> >>> +static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> >>> + {
> >>> + .name = "top_0", .id = MDP_TOP,
> >>> + .base = 0x0, .len = 0x45C,
> >>> + .features = 0,
> >>> + .highest_bank_bit = 0x3,
> >>> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
> >>> + .reg_off = 0x2AC, .bit_off = 0},
> >>> + .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
> >>> + .reg_off = 0x2B4, .bit_off = 0},
> >>> + .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
> >>> + .reg_off = 0x2BC, .bit_off = 0},
> >>> + .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
> >>> + .reg_off = 0x2C4, .bit_off = 0},
> >>> + .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> >>> + .reg_off = 0x2AC, .bit_off = 8},
> >>> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> >>> + .reg_off = 0x2B4, .bit_off = 8},
> >>> + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> >>> + .reg_off = 0x2BC, .bit_off = 8},
> >>> + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> >>> + .reg_off = 0x2C4, .bit_off = 8},
> >>> + },
> >>> +};
> >>> +
> >>>static const struct dpu_mdp_cfg sm8250_mdp[] = {
> >>> {
> >>> .name = "top_0", .id = MDP_TOP,
> >>> @@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
> >>> INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
> >>> INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
> >>>};
> >>> +static const struct dpu_intf_cfg sc8180x_intf[] = {
> >>> + INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
> >>> INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
> >>> + INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0,
Re: [PATCH v4 2/2] drm/msm/dp: enable widebus feature for display port
On 2/15/2022 10:30 AM, Dmitry Baryshkov wrote:
On Tue, 15 Feb 2022 at 20:49, Kuogee Hsieh wrote:
On 2/15/2022 5:34 AM, Dmitry Baryshkov wrote:
On 15/02/2022 01:39, Kuogee Hsieh wrote:
Widebus feature will transmit two pixel data per pixel clock to
interface.
This feature now is required to be enabled to easy migrant to higher
resolution applications in future. However since some legacy chipsets
does not support this feature, this feature is enabled base on chip's
hardware revision.
changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
-- enable widebus feature base on chip hardware revision
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 36
+++--
drivers/gpu/drm/msm/dp/dp_catalog.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_ctrl.c| 13 +++
drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 30
drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
drivers/gpu/drm/msm/msm_drv.h | 6 +
10 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0c22839..b2d23c2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2167,8 +2167,10 @@ int dpu_encoder_setup(struct drm_device *dev,
struct drm_encoder *enc,
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
-else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) {
dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
+dpu_enc->wide_bus_en = msm_dp_wide_bus_enable(dpu_enc->dp);
+}
Please revert the order of the patches and move this chunk into the
DPU patch.
There is chicken and egg issue here.
I think we should create an communication channel to pass dp/dsi driver
info to drm, such as wide_bus_en and dsc compression, etc.
By function calls like this will create more unnecessary tangles between
driver and drm layer.
I'm fine with the function call. I'm suggesting another thing. If you
change the order of these two patches (#1 - dp, #2 - dpu), the
msm_dp_wide_bus_enable() will get added in patch#1 and will be
available to be called in the patch#2.
By doing that, we have wile widebus enabled at (#1 -dp) patch but has no
timing engine changes at (#2 -dpu) to support it. This means (#1 -dp)
will break dp until (#2 -dpu) merged.
Is this ok?
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 64f0b26..99d087e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -483,6 +483,27 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct
dp_catalog *dp_catalog,
}
/**
+ * dp_catalog_hw_revision() - retrieve DP hw revision
+ *
+ * @dp_catalog: DP catalog structure
+ *
+ * return: u32
+ *
+ * This function return the DP controller hw revision
+ *
+ */
+u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)
+{
+u32 revision;
+struct dp_catalog_private *catalog = container_of(dp_catalog,
+struct dp_catalog_private, dp_catalog);
+
+revision = dp_read_ahb(catalog, REG_DP_HW_VERSION);
+
+return revision;
+}
+
+/**
* dp_catalog_ctrl_reset() - reset DP controller
*
* @dp_catalog: DP catalog structure
@@ -739,10 +760,11 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct
dp_catalog *dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, bool
wide_bus_en)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
+u32 reg;
dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
dp_catalog->total);
@@ -751,7 +773,17 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
dp_catalog->width_blanking);
dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER,
dp_catalog->dp_active);
-dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, 0);
+
+reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
+
+if (wide_bus_en)
+reg |= BIT(4);/* DATABUS_WIDEN */
+else
+reg &
Re: [PATCH v4 2/2] drm/msm/dp: enable widebus feature for display port
On Tue, 15 Feb 2022 at 20:49, Kuogee Hsieh wrote:
>
>
> On 2/15/2022 5:34 AM, Dmitry Baryshkov wrote:
> > On 15/02/2022 01:39, Kuogee Hsieh wrote:
> >> Widebus feature will transmit two pixel data per pixel clock to
> >> interface.
> >> This feature now is required to be enabled to easy migrant to higher
> >> resolution applications in future. However since some legacy chipsets
> >> does not support this feature, this feature is enabled base on chip's
> >> hardware revision.
> >>
> >> changes in v2:
> >> -- remove compression related code from timing
> >> -- remove op_info from struct msm_drm_private
> >> -- remove unnecessary wide_bus_en variables
> >> -- pass wide_bus_en into timing configuration by struct msm_dp
> >>
> >> Changes in v3:
> >> -- split patch into 3 patches
> >> -- enable widebus feature base on chip hardware revision
> >>
> >> Signed-off-by: Kuogee Hsieh
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
> >> drivers/gpu/drm/msm/dp/dp_catalog.c | 36
> >> +++--
> >> drivers/gpu/drm/msm/dp/dp_catalog.h | 3 ++-
> >> drivers/gpu/drm/msm/dp/dp_ctrl.c| 13 +++
> >> drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 +
> >> drivers/gpu/drm/msm/dp/dp_display.c | 30
> >>
> >> drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
> >> drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
> >> drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
> >> drivers/gpu/drm/msm/msm_drv.h | 6 +
> >> 10 files changed, 90 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index 0c22839..b2d23c2 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -2167,8 +2167,10 @@ int dpu_encoder_setup(struct drm_device *dev,
> >> struct drm_encoder *enc,
> >> timer_setup(&dpu_enc->vsync_event_timer,
> >> dpu_encoder_vsync_event_handler,
> >> 0);
> >> -else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
> >> +else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) {
> >> dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
> >> +dpu_enc->wide_bus_en = msm_dp_wide_bus_enable(dpu_enc->dp);
> >> +}
> >
> > Please revert the order of the patches and move this chunk into the
> > DPU patch.
>
> There is chicken and egg issue here.
>
> I think we should create an communication channel to pass dp/dsi driver
> info to drm, such as wide_bus_en and dsc compression, etc.
>
> By function calls like this will create more unnecessary tangles between
> driver and drm layer.
I'm fine with the function call. I'm suggesting another thing. If you
change the order of these two patches (#1 - dp, #2 - dpu), the
msm_dp_wide_bus_enable() will get added in patch#1 and will be
available to be called in the patch#2.
>
> >
> >> INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
> >> dpu_encoder_off_work);
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> index 64f0b26..99d087e 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> @@ -483,6 +483,27 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct
> >> dp_catalog *dp_catalog,
> >> }
> >> /**
> >> + * dp_catalog_hw_revision() - retrieve DP hw revision
> >> + *
> >> + * @dp_catalog: DP catalog structure
> >> + *
> >> + * return: u32
> >> + *
> >> + * This function return the DP controller hw revision
> >> + *
> >> + */
> >> +u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)
> >> +{
> >> +u32 revision;
> >> +struct dp_catalog_private *catalog = container_of(dp_catalog,
> >> +struct dp_catalog_private, dp_catalog);
> >> +
> >> +revision = dp_read_ahb(catalog, REG_DP_HW_VERSION);
> >> +
> >> +return revision;
> >> +}
> >> +
> >> +/**
> >>* dp_catalog_ctrl_reset() - reset DP controller
> >>*
> >>* @dp_catalog: DP catalog structure
> >> @@ -739,10 +760,11 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct
> >> dp_catalog *dp_catalog)
> >> }
> >> /* panel related catalog functions */
> >> -int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
> >> +int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, bool
> >> wide_bus_en)
> >> {
> >> struct dp_catalog_private *catalog = container_of(dp_catalog,
> >> struct dp_catalog_private, dp_catalog);
> >> +u32 reg;
> >> dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
> >> dp_catalog->total);
> >> @@ -751,7 +773,17 @@ int dp_catalog_panel_timing_cfg(struct
> >> dp_catalog *dp_catalog)
> >> dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
> >> dp_catalog->width_blanking);
> >> dp_write_link(cat
RE: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce format modifier for DG2 clear color
> -Original Message-
> From: Juha-Pekka Heikkila
> Sent: Tuesday, February 15, 2022 9:32 AM
> To: Chery, Nanley G ; Nanley Chery
> ; C, Ramalingam
> Cc: intel-gfx ; Auld, Matthew
> ; dri-devel
> Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce format
> modifier for DG2 clear color
>
> On 15.2.2022 18.44, Chery, Nanley G wrote:
> >
> >
> >> -Original Message-
> >> From: Juha-Pekka Heikkila
> >> Sent: Tuesday, February 15, 2022 8:15 AM
> >> To: Chery, Nanley G ; Nanley Chery
> >> ; C, Ramalingam
> >> Cc: intel-gfx ; Auld, Matthew
> >> ; dri-devel
> >> Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce
> >> format modifier for DG2 clear color
> >>
> >> On 15.2.2022 17.02, Chery, Nanley G wrote:
> >>>
> >>>
> -Original Message-
> From: Juha-Pekka Heikkila
> Sent: Tuesday, February 15, 2022 6:56 AM
> To: Nanley Chery ; C, Ramalingam
>
> Cc: intel-gfx ; Chery, Nanley G
> ; Auld, Matthew ;
> dri- devel
> Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce
> format modifier for DG2 clear color
>
> On 12.2.2022 3.19, Nanley Chery wrote:
> > On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C
> >
> wrote:
> >>
> >> From: Mika Kahola
> >>
> >> DG2 clear color render compression uses Tile4 layout. Therefore,
> >> we need to define a new format modifier for uAPI to support clear
> >> color
> rendering.
> >>
> >> v2:
> >> Display version is fixed. [Imre]
> >> KDoc is enhanced for cc modifier. [Nanley & Lionel]
> >>
> >> Signed-off-by: Mika Kahola
> >> cc: Anshuman Gupta
> >> Signed-off-by: Juha-Pekka Heikkilä
> >>
> >> Signed-off-by: Ramalingam C
> >> ---
> >> drivers/gpu/drm/i915/display/intel_fb.c| 8
> >> drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 -
> >> include/uapi/drm/drm_fourcc.h | 10 ++
> >> 3 files changed, 26 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> >> b/drivers/gpu/drm/i915/display/intel_fb.c
> >> index 4d4d01963f15..3df6ef5ffec5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> >> @@ -144,6 +144,12 @@ static const struct intel_modifier_desc
> intel_modifiers[] = {
> >>.modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS,
> >>.display_ver = { 13, 13 },
> >>.plane_caps = INTEL_PLANE_CAP_TILING_4 |
> >> INTEL_PLANE_CAP_CCS_MC,
> >> + }, {
> >> + .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC,
> >> + .display_ver = { 13, 13 },
> >> + .plane_caps = INTEL_PLANE_CAP_TILING_4 |
> >> + INTEL_PLANE_CAP_CCS_RC_CC,
> >> +
> >> + .ccs.cc_planes = BIT(1),
> >>}, {
> >>.modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS,
> >>.display_ver = { 13, 13 }, @@ -559,6 +565,7 @@
> >> intel_tile_width_bytes(const struct drm_framebuffer *fb, int
> color_plane)
> >>else
> >>return 512;
> >>case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
> >> + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
> >>case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
> >>case I915_FORMAT_MOD_4_TILED:
> >>/*
> >> @@ -763,6 +770,7 @@ unsigned int intel_surf_alignment(const
> >> struct
> drm_framebuffer *fb,
> >>case I915_FORMAT_MOD_Yf_TILED:
> >>return 1 * 1024 * 1024;
> >>case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
> >> + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
> >>case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
> >>return 16 * 1024;
> >>default:
> >> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> index c38ae0876c15..b4dced1907c5 100644
> >> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >> @@ -772,6 +772,8 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
> >>return PLANE_CTL_TILED_4 |
> >>PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE |
> >>PLANE_CTL_CLEAR_COLOR_DISABLE;
> >> + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
> >> + return PLANE_CTL_TILED_4 |
> >> + PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
> >>case I915_FORMAT_MOD_Y_TILED_CCS:
> >>case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> >>
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Tue, Feb 15, 2022 at 11:47:43AM -0600, Gustavo A. R. Silva wrote:
> There is a regular need in the kernel to provide a way to declare
> having a dynamically sized set of trailing elements in a structure.
> Kernel code should always use “flexible array members”[1] for these
> cases. The older style of one-element or zero-length arrays should
> no longer be used[2].
>
> This code was transformed with the help of Coccinelle:
> (next-20220214$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> script.cocci --include-headers --dir . > output.patch)
>
> @@
> identifier S, member, array;
> type T1, T2;
> @@
>
> struct S {
> ...
> T1 member;
> T2 array[
> - 0
> ];
> };
These all look trivially correct to me. Only two didn't have the end of
the struct visible in the patch, and checking those showed them to be
trailing members as well, so:
Reviewed-by: Kees Cook
--
Kees Cook
Re: [PATCH v2 1/8] drm/msm/dpu: fix dp audio condition
On 2/15/2022 6:16 AM, Dmitry Baryshkov wrote:
DP audio enablement code which is comparing intf_type,
DRM_MODE_ENCODER_TMDS (= 2) with DRM_MODE_CONNECTOR_DisplayPort (= 10).
Which would never succeed. Fix it to check for DRM_MODE_ENCODER_TMDS.
Fixes: d13e36d7d222 ("drm/msm/dp: add audio support for Display Port on MSM")
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 132844801e92..c59976deb1cb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1099,7 +1099,7 @@ static void _dpu_encoder_virt_enable_helper(struct
drm_encoder *drm_enc)
}
- if (dpu_enc->disp_info.intf_type == DRM_MODE_CONNECTOR_DisplayPort &&
+ if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_TMDS &&
dpu_enc->cur_master->hw_mdptop &&
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select)
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select(
Re: [PATCH v2 8/8] drm/msm/dpu: simplify intf allocation code
On 2/15/2022 6:16 AM, Dmitry Baryshkov wrote:
Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess,
which intf type we mean, pass INTF_DSI/INTF_DP directly.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++--
3 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index fa1dc088a672..597d40f78d38 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -490,7 +490,7 @@ void dpu_encoder_helper_split_config(
hw_mdptop = phys_enc->hw_mdptop;
disp_info = &dpu_enc->disp_info;
- if (disp_info->intf_type != DRM_MODE_ENCODER_DSI)
+ if (disp_info->intf_type != INTF_DSI)
return;
/**
@@ -552,7 +552,7 @@ static struct msm_display_topology dpu_encoder_get_topology(
else
topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
+ if (dpu_enc->disp_info.intf_type == INTF_DSI) {
if (dpu_kms->catalog->dspp &&
(dpu_kms->catalog->dspp_count >= topology.num_lm))
topology.num_dspp = topology.num_lm;
@@ -1074,7 +1074,7 @@ static void _dpu_encoder_virt_enable_helper(struct
drm_encoder *drm_enc)
}
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_TMDS &&
+ if (dpu_enc->disp_info.intf_type == INTF_DP &&
dpu_enc->cur_master->hw_mdptop &&
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select)
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select(
@@ -1082,7 +1082,7 @@ static void _dpu_encoder_virt_enable_helper(struct
drm_encoder *drm_enc)
_dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info);
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI &&
+ if (dpu_enc->disp_info.intf_type == INTF_DSI &&
!WARN_ON(dpu_enc->num_phys_encs == 0)) {
unsigned bpc = dpu_enc->connector->display_info.bpc;
for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
@@ -1949,7 +1949,6 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
{
int ret = 0;
int i = 0;
- enum dpu_intf_type intf_type = INTF_NONE;
struct dpu_enc_phys_init_params phys_params;
if (!dpu_enc) {
@@ -1965,15 +1964,6 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
phys_params.parent_ops = &dpu_encoder_parent_ops;
phys_params.enc_spinlock = &dpu_enc->enc_spinlock;
- switch (disp_info->intf_type) {
- case DRM_MODE_ENCODER_DSI:
- intf_type = INTF_DSI;
- break;
- case DRM_MODE_ENCODER_TMDS:
- intf_type = INTF_DP;
- break;
- }
-
WARN_ON(disp_info->num_of_h_tiles < 1);
DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", disp_info->num_of_h_tiles);
@@ -2005,11 +1995,11 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
i, controller_id, phys_params.split_role);
phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
-
intf_type,
-
controller_id);
+ disp_info->intf_type,
+ controller_id);
if (phys_params.intf_idx == INTF_MAX) {
DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id
%d\n",
- intf_type, controller_id);
+ disp_info->intf_type,
controller_id);
ret = -EINVAL;
}
@@ -2092,7 +2082,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
timer_setup(&dpu_enc->frame_done_timer,
dpu_encoder_frame_done_timeout, 0);
- if (disp_info->intf_type == DRM_MODE_ENCODER_DSI)
+ if (disp_info->intf_type == INTF_DSI)
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index ebe3944355bb..3891bcbbe5a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -36,7 +36,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
/**
* struct msm_display_info - defines displ
Re: [PATCH v4 2/2] drm/msm/dp: enable widebus feature for display port
On 2/15/2022 5:34 AM, Dmitry Baryshkov wrote:
On 15/02/2022 01:39, Kuogee Hsieh wrote:
Widebus feature will transmit two pixel data per pixel clock to
interface.
This feature now is required to be enabled to easy migrant to higher
resolution applications in future. However since some legacy chipsets
does not support this feature, this feature is enabled base on chip's
hardware revision.
changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
-- enable widebus feature base on chip hardware revision
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 36
+++--
drivers/gpu/drm/msm/dp/dp_catalog.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++
drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 30
drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
drivers/gpu/drm/msm/msm_drv.h | 6 +
10 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0c22839..b2d23c2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2167,8 +2167,10 @@ int dpu_encoder_setup(struct drm_device *dev,
struct drm_encoder *enc,
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
- else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) {
dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
+ dpu_enc->wide_bus_en = msm_dp_wide_bus_enable(dpu_enc->dp);
+ }
Please revert the order of the patches and move this chunk into the
DPU patch.
There is chicken and egg issue here.
I think we should create an communication channel to pass dp/dsi driver
info to drm, such as wide_bus_en and dsc compression, etc.
By function calls like this will create more unnecessary tangles between
driver and drm layer.
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 64f0b26..99d087e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -483,6 +483,27 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct
dp_catalog *dp_catalog,
}
/**
+ * dp_catalog_hw_revision() - retrieve DP hw revision
+ *
+ * @dp_catalog: DP catalog structure
+ *
+ * return: u32
+ *
+ * This function return the DP controller hw revision
+ *
+ */
+u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)
+{
+ u32 revision;
+ struct dp_catalog_private *catalog = container_of(dp_catalog,
+ struct dp_catalog_private, dp_catalog);
+
+ revision = dp_read_ahb(catalog, REG_DP_HW_VERSION);
+
+ return revision;
+}
+
+/**
* dp_catalog_ctrl_reset() - reset DP controller
*
* @dp_catalog: DP catalog structure
@@ -739,10 +760,11 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct
dp_catalog *dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, bool
wide_bus_en)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
+ u32 reg;
dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
dp_catalog->total);
@@ -751,7 +773,17 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
dp_catalog->width_blanking);
dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER,
dp_catalog->dp_active);
- dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, 0);
+
+ reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
+
+ if (wide_bus_en)
+ reg |= BIT(4); /* DATABUS_WIDEN */
+ else
+ reg &= ~BIT(4);
+
+ DRM_DEBUG_DP("wide_bus_en=%d reg=%x\n", wide_bus_en, reg);
+
+ dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, reg);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 7dea101..a3a0129 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -95,6 +95,7 @@ void dp_catalog_ctrl_config_misc(struct dp_catalog
*dp_catalog, u32 cc, u32 tb);
void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32
rate,
Re: [PATCH v2 00/12] iio: buffer-dma: write() and new DMABUF based API
Hi Jonathan, Le dim., févr. 13 2022 at 18:46:16 +, Jonathan Cameron a écrit : On Mon, 7 Feb 2022 12:59:21 + Paul Cercueil wrote: Hi Jonathan, This is the V2 of my patchset that introduces a new userspace interface based on DMABUF objects to complement the fileio API, and adds write() support to the existing fileio API. Hi Paul, It's been a little while. Perhaps you could summarize the various view points around the appropriateness of using DMABUF for this? I appreciate it is a tricky topic to distil into a brief summary but I know I would find it useful even if no one else does! So we want to have a high-speed interface where buffers of samples are passed around between IIO devices and other devices (e.g. USB or network), or made available to userspace without copying the data. DMABUF is, at least in theory, exactly what we need. Quoting the documentation (https://www.kernel.org/doc/html/v5.15/driver-api/dma-buf.html): "The dma-buf subsystem provides the framework for sharing buffers for hardware (DMA) access across multiple device drivers and subsystems, and for synchronizing asynchronous hardware access. This is used, for example, by drm “prime” multi-GPU support, but is of course not limited to GPU use cases." The problem is that right now DMABUF is only really used by DRM, and to quote Daniel, "dma-buf looks like something super generic and useful, until you realize that there's a metric ton of gpu/accelerator bagage piled in". Still, it seems to be the only viable option. We could add a custom buffer-passing interface, but that would mean implementing the same buffer-passing interface on the network and USB stacks, and before we know it we re-invented DMABUFs. Cheers, -Paul Changes since v1: - the patches that were merged in v1 have been (obviously) dropped from this patchset; - the patch that was setting the write-combine cache setting has been dropped as well, as it was simply not useful. - [01/12]: * Only remove the outgoing queue, and keep the incoming queue, as we want the buffer to start streaming data as soon as it is enabled. * Remove IIO_BLOCK_STATE_DEQUEUED, since it is now functionally the same as IIO_BLOCK_STATE_DONE. - [02/12]: * Fix block->state not being reset in iio_dma_buffer_request_update() for output buffers. * Only update block->bytes_used once and add a comment about why we update it. * Add a comment about why we're setting a different state for output buffers in iio_dma_buffer_request_update() * Remove useless cast to bool (!!) in iio_dma_buffer_io() - [05/12]: Only allow the new IOCTLs on the buffer FD created with IIO_BUFFER_GET_FD_IOCTL(). - [12/12]: * Explicitly state that the new interface is optional and is not implemented by all drivers. * The IOCTLs can now only be called on the buffer FD returned by IIO_BUFFER_GET_FD_IOCTL. * Move the page up a bit in the index since it is core stuff and not driver-specific. The patches not listed here have not been modified since v1. Cheers, -Paul Alexandru Ardelean (1): iio: buffer-dma: split iio_dma_buffer_fileio_free() function Paul Cercueil (11): iio: buffer-dma: Get rid of outgoing queue iio: buffer-dma: Enable buffer write support iio: buffer-dmaengine: Support specifying buffer direction iio: buffer-dmaengine: Enable write support iio: core: Add new DMABUF interface infrastructure iio: buffer-dma: Use DMABUFs instead of custom solution iio: buffer-dma: Implement new DMABUF based userspace API iio: buffer-dmaengine: Support new DMABUF based userspace API iio: core: Add support for cyclic buffers iio: buffer-dmaengine: Add support for cyclic buffers Documentation: iio: Document high-speed DMABUF based API Documentation/driver-api/dma-buf.rst | 2 + Documentation/iio/dmabuf_api.rst | 94 +++ Documentation/iio/index.rst | 2 + drivers/iio/adc/adi-axi-adc.c | 3 +- drivers/iio/buffer/industrialio-buffer-dma.c | 610 ++ .../buffer/industrialio-buffer-dmaengine.c| 42 +- drivers/iio/industrialio-buffer.c | 60 ++ include/linux/iio/buffer-dma.h| 38 +- include/linux/iio/buffer-dmaengine.h | 5 +- include/linux/iio/buffer_impl.h | 8 + include/uapi/linux/iio/buffer.h | 30 + 11 files changed, 749 insertions(+), 145 deletions(-) create mode 100644 Documentation/iio/dmabuf_api.rst
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On 2/15/2022 9:28 AM, Bjorn Andersson wrote:
On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
From: Rob Clark
Add SC8180x to the hardware catalog, for initial support for the
platform. Due to limitations in the DP driver only one of the four DP
interfaces is left enabled.
The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
is flagged appropriately to ensure widebus is disabled - for now.
Signed-off-by: Rob Clark
[bjorn: Reworked intf and irq definitions]
Signed-off-by: Bjorn Andersson
---
Changes since v1:
- Dropped widebus flag
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
drivers/gpu/drm/msm/msm_drv.c | 1 +
4 files changed, 132 insertions(+)
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 aa75991903a6..7ac0fe32df49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -90,6 +90,17 @@
BIT(MDP_INTF3_INTR) | \
BIT(MDP_INTF4_INTR))
+#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+ BIT(MDP_SSPP_TOP0_INTR2) | \
+ BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+ BIT(MDP_INTF0_INTR) | \
+ BIT(MDP_INTF1_INTR) | \
+ BIT(MDP_INTF2_INTR) | \
+ BIT(MDP_INTF3_INTR) | \
+ BIT(MDP_INTF4_INTR) | \
+ BIT(MDP_INTF5_INTR) | \
+ BIT(MDP_AD4_0_INTR) | \
+ BIT(MDP_AD4_1_INTR))
#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH 2048
@@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
.max_vdeci_exp = MAX_VERT_DECIMATION,
};
+static const struct dpu_caps sc8180x_dpu_caps = {
+ .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+ .max_mixer_blendstages = 0xb,
+ .qseed_type = DPU_SSPP_SCALER_QSEED3,
+ .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
+ .ubwc_version = DPU_HW_UBWC_VER_30,
+ .has_src_split = true,
+ .has_dim_layer = true,
+ .has_idle_pc = true,
+ .has_3d_merge = true,
+ .max_linewidth = 4096,
+ .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+ .max_hdeci_exp = MAX_HORZ_DECIMATION,
+ .max_vdeci_exp = MAX_VERT_DECIMATION,
+};
+
static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
@@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
},
};
+static const struct dpu_mdp_cfg sc8180x_mdp[] = {
+ {
+ .name = "top_0", .id = MDP_TOP,
+ .base = 0x0, .len = 0x45C,
+ .features = 0,
+ .highest_bank_bit = 0x3,
+ .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
+ .reg_off = 0x2AC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
+ .reg_off = 0x2B4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
+ .reg_off = 0x2BC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
+ .reg_off = 0x2C4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
+ .reg_off = 0x2AC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
+ .reg_off = 0x2B4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .reg_off = 0x2BC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .reg_off = 0x2C4, .bit_off = 8},
+ },
+};
+
static const struct dpu_mdp_cfg sm8250_mdp[] = {
{
.name = "top_0", .id = MDP_TOP,
@@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
+static const struct dpu_intf_cfg sc8180x_intf[] = {
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
+ INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 28, 29),
+ /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until
this is supported */
+ INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 30, 31),
+ INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7180_MAS
Re: [PATCH v4 2/2] drm/msm/dp: enable widebus feature for display port
On 2/14/2022 8:22 PM, Bjorn Andersson wrote:
On Mon 14 Feb 16:39 CST 2022, Kuogee Hsieh wrote:
Widebus feature will transmit two pixel data per pixel clock to interface.
This feature now is required to be enabled to easy migrant to higher
resolution applications in future. However since some legacy chipsets
does not support this feature, this feature is enabled base on chip's
hardware revision.
changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
-- enable widebus feature base on chip hardware revision
Signed-off-by: Kuogee Hsieh
Tested-by: Bjorn Andersson
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 36 +++--
drivers/gpu/drm/msm/dp/dp_catalog.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_ctrl.c| 13 +++
drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 30
drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
drivers/gpu/drm/msm/msm_drv.h | 6 +
10 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0c22839..b2d23c2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2167,8 +2167,10 @@ int dpu_encoder_setup(struct drm_device *dev, struct
drm_encoder *enc,
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
- else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) {
dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
+ dpu_enc->wide_bus_en = msm_dp_wide_bus_enable(dpu_enc->dp);
+ }
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 64f0b26..99d087e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -483,6 +483,27 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct
dp_catalog *dp_catalog,
}
/**
+ * dp_catalog_hw_revision() - retrieve DP hw revision
+ *
+ * @dp_catalog: DP catalog structure
+ *
+ * return: u32
Q: What's 2+2?
A: Integer
This should say:
Return: the controller hardware revision
+ *
+ * This function return the DP controller hw revision
That's what "Return:" in the kernel-doc is supposed to clarify...
https://docs.kernel.org/doc-guide/kernel-doc.html is good to read.
+ *
+ */
+u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)
+{
+ u32 revision;
+ struct dp_catalog_private *catalog = container_of(dp_catalog,
+ struct dp_catalog_private, dp_catalog);
+
+ revision = dp_read_ahb(catalog, REG_DP_HW_VERSION);
There's no need for a local variable here, just:
return dp_read_ahb();
+
+ return revision;
+}
+
+/**
* dp_catalog_ctrl_reset() - reset DP controller
*
* @dp_catalog: DP catalog structure
@@ -739,10 +760,11 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct dp_catalog
*dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, bool
wide_bus_en)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
+ u32 reg;
dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
dp_catalog->total);
@@ -751,7 +773,17 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
dp_catalog->width_blanking);
dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_catalog->dp_active);
- dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, 0);
+
+ reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
+
+ if (wide_bus_en)
+ reg |= BIT(4); /* DATABUS_WIDEN */
#define DATABUS_WIDEN BIT(4)
Would save you the need for writing that comment.
+ else
+ reg &= ~BIT(4);
+
+ DRM_DEBUG_DP("wide_bus_en=%d reg=%x\n", wide_bus_en, reg);
+
+ dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, reg);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 7dea101..a3a0129 100644
--- a/drivers/gpu/d
[PATCH][next] treewide: Replace zero-length arrays with flexible-array members
There is a regular need in the kernel to provide a way to declare
having a dynamically sized set of trailing elements in a structure.
Kernel code should always use “flexible array members”[1] for these
cases. The older style of one-element or zero-length arrays should
no longer be used[2].
This code was transformed with the help of Coccinelle:
(next-20220214$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
script.cocci --include-headers --dir . > output.patch)
@@
identifier S, member, array;
type T1, T2;
@@
struct S {
...
T1 member;
T2 array[
- 0
];
};
UAPI and wireless changes were intentionally excluded from this patch
and will be sent out separately.
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2]
https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/78
Signed-off-by: Gustavo A. R. Silva
---
Hi all,
I'm expecting to carry this patch in my tree, so it'd be great to
get some Acks. And given the size of the patch, I'm only sending
this to mailing lists.
Thanks!
arch/alpha/include/asm/hwrpb.h| 2 +-
arch/ia64/include/asm/sal.h | 2 +-
arch/s390/include/asm/ccwgroup.h | 2 +-
arch/s390/include/asm/chsc.h | 2 +-
arch/s390/include/asm/eadm.h | 2 +-
arch/s390/include/asm/fcx.h | 4 ++--
arch/s390/include/asm/idals.h | 2 +-
arch/s390/include/asm/sclp.h | 2 +-
arch/s390/include/asm/sysinfo.h | 6 +++---
arch/sh/include/asm/thread_info.h | 2 +-
arch/sparc/include/asm/vio.h | 10 +-
arch/um/include/shared/net_kern.h | 2 +-
arch/x86/include/asm/microcode_amd.h | 2 +-
arch/x86/include/asm/microcode_intel.h| 4 ++--
arch/x86/include/asm/pci.h| 2 +-
arch/x86/include/asm/pci_x86.h| 2 +-
arch/xtensa/include/asm/bootparam.h | 2 +-
drivers/crypto/caam/pdb.h | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 2 +-
drivers/gpu/drm/nouveau/include/nvfw/hs.h | 2 +-
.../hwtracing/coresight/coresight-config.h| 2 +-
drivers/misc/bcm-vk/bcm_vk.h | 2 +-
.../misc/habanalabs/include/common/cpucp_if.h | 6 +++---
.../habanalabs/include/gaudi/gaudi_packets.h | 4 ++--
.../habanalabs/include/goya/goya_packets.h| 4 ++--
.../net/ethernet/freescale/enetc/enetc_hw.h | 2 +-
drivers/net/ethernet/i825xx/sun3_82586.h | 2 +-
.../net/ethernet/marvell/octeontx2/af/npc.h | 6 +++---
drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h | 2 +-
drivers/net/ethernet/ti/davinci_mdio.c| 2 +-
drivers/scsi/dpt/dpti_i2o.h | 2 +-
drivers/scsi/elx/libefc_sli/sli4.h| 20 +--
drivers/scsi/mpi3mr/mpi3mr.h | 2 +-
drivers/scsi/qla2xxx/qla_bsg.h| 4 ++--
drivers/scsi/qla2xxx/qla_def.h| 2 +-
drivers/scsi/qla2xxx/qla_edif_bsg.h | 4 ++--
drivers/scsi/qla2xxx/qla_fw.h | 2 +-
drivers/scsi/qla4xxx/ql4_fw.h | 2 +-
drivers/staging/r8188eu/include/ieee80211.h | 6 +++---
drivers/staging/r8188eu/include/rtw_cmd.h | 10 +-
drivers/staging/rtl8712/rtl871x_cmd.h | 8
drivers/staging/rtl8723bs/include/ieee80211.h | 2 +-
drivers/staging/rtl8723bs/include/rtw_cmd.h | 2 +-
.../include/linux/raspberrypi/vchiq.h | 2 +-
drivers/visorbus/vbuschannel.h| 2 +-
fs/cifs/ntlmssp.h | 2 +-
fs/ext4/fast_commit.h | 4 ++--
fs/ksmbd/ksmbd_netlink.h | 2 +-
fs/ksmbd/ntlmssp.h| 6 +++---
fs/ksmbd/smb2pdu.h| 8
fs/ksmbd/transport_rdma.c | 2 +-
fs/ksmbd/xattr.h | 2 +-
fs/xfs/scrub/attr.h | 2 +-
include/acpi/actbl2.h | 2 +-
include/asm-generic/tlb.h | 4 ++--
include/linux/greybus/greybus_manifest.h | 4 ++--
include/linux/greybus/hd.h| 2 +-
include/linux/greybus/module.h| 2 +-
include/linux/i3c/ccc.h | 6 +++---
include/linux/mlx5/mlx5_ifc_fpga.h| 2 +-
include/linux/platform_data/brcmfmac.h| 2 +-
.../linux/platform_data/cros_ec_commands.h| 2 +-
include/net/bluetooth/mgmt.h | 2 +-
include/net/ioam6.h | 2 +-
include/sound/sof/channel_map.h | 4 ++--
scripts/dtc/libfdt/fdt.h | 4 ++--
sound/soc/intel/atom/sst-mfld-dsp.h | 4 ++--
sound/soc/intel/skylake/skl-topology.h| 2 +-
tools/lib/perf/include/perf/event.h | 2 +-
69 files ch
Re: [PATCH v6 25/35] iommu/mediatek: Migrate to aggregate driver
On 10/02/2022 12:03, Yong Wu wrote: > On Thu, 2022-01-27 at 12:01 -0800, Stephen Boyd wrote: >> Use an aggregate driver instead of component ops so that we can get >> proper driver probe ordering of the aggregate device with respect to >> all >> the component devices that make up the aggregate device. >> >> Cc: Yong Wu >> Cc: Joerg Roedel >> Cc: Will Deacon >> Cc: Daniel Vetter >> Cc: "Rafael J. Wysocki" >> Cc: Rob Clark >> Cc: Russell King >> Cc: Saravana Kannan >> Signed-off-by: Stephen Boyd > > + Krzysztof > > The memory/mtk-smi.c is expected to get Ack from Krzysztof. > Please resend with cc-ing me. I don't have the patch in my mailbox (wes not on address list) and I also cannot find it in linux-arm-kernel. Best regards, Krzysztof
Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce format modifier for DG2 clear color
On 15.2.2022 18.44, Chery, Nanley G wrote:
-Original Message-
From: Juha-Pekka Heikkila
Sent: Tuesday, February 15, 2022 8:15 AM
To: Chery, Nanley G ; Nanley Chery
; C, Ramalingam
Cc: intel-gfx ; Auld, Matthew
; dri-devel
Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce format
modifier for DG2 clear color
On 15.2.2022 17.02, Chery, Nanley G wrote:
-Original Message-
From: Juha-Pekka Heikkila
Sent: Tuesday, February 15, 2022 6:56 AM
To: Nanley Chery ; C, Ramalingam
Cc: intel-gfx ; Chery, Nanley G
; Auld, Matthew ;
dri- devel
Subject: Re: [Intel-gfx] [PATCH v5 16/19] uapi/drm/dg2: Introduce
format modifier for DG2 clear color
On 12.2.2022 3.19, Nanley Chery wrote:
On Tue, Feb 1, 2022 at 2:42 AM Ramalingam C
wrote:
From: Mika Kahola
DG2 clear color render compression uses Tile4 layout. Therefore, we
need to define a new format modifier for uAPI to support clear
color
rendering.
v2:
Display version is fixed. [Imre]
KDoc is enhanced for cc modifier. [Nanley & Lionel]
Signed-off-by: Mika Kahola
cc: Anshuman Gupta
Signed-off-by: Juha-Pekka Heikkilä
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/display/intel_fb.c| 8
drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 -
include/uapi/drm/drm_fourcc.h | 10 ++
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 4d4d01963f15..3df6ef5ffec5 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -144,6 +144,12 @@ static const struct intel_modifier_desc
intel_modifiers[] = {
.modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS,
.display_ver = { 13, 13 },
.plane_caps = INTEL_PLANE_CAP_TILING_4 |
INTEL_PLANE_CAP_CCS_MC,
+ }, {
+ .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC,
+ .display_ver = { 13, 13 },
+ .plane_caps = INTEL_PLANE_CAP_TILING_4 |
+ INTEL_PLANE_CAP_CCS_RC_CC,
+
+ .ccs.cc_planes = BIT(1),
}, {
.modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS,
.display_ver = { 13, 13 }, @@ -559,6 +565,7 @@
intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
else
return 512;
case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
+ case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
case I915_FORMAT_MOD_4_TILED:
/*
@@ -763,6 +770,7 @@ unsigned int intel_surf_alignment(const struct
drm_framebuffer *fb,
case I915_FORMAT_MOD_Yf_TILED:
return 1 * 1024 * 1024;
case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
+ case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
return 16 * 1024;
default:
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index c38ae0876c15..b4dced1907c5 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -772,6 +772,8 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
return PLANE_CTL_TILED_4 |
PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE |
PLANE_CTL_CLEAR_COLOR_DISABLE;
+ case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
+ return PLANE_CTL_TILED_4 |
+ PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
case I915_FORMAT_MOD_Y_TILED_CCS:
case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
return PLANE_CTL_TILED_Y |
PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
@@ -2358,10 +2360,15 @@ skl_get_initial_plane_config(struct
intel_crtc
*crtc,
break;
case PLANE_CTL_TILED_YF: /* aka PLANE_CTL_TILED_4 on XE_LPD+ */
if (HAS_4TILE(dev_priv)) {
- if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
+ u32 rc_mask =
PLANE_CTL_RENDER_DECOMPRESSION_ENABLE |
+
+ PLANE_CTL_CLEAR_COLOR_DISABLE;
+
+ if ((val & rc_mask) == rc_mask)
fb->modifier =
I915_FORMAT_MOD_4_TILED_DG2_RC_CCS;
else if (val & PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE)
fb->modifier =
I915_FORMAT_MOD_4_TILED_DG2_MC_CCS;
+ else if (val & PLANE_CTL_RENDER_DECOMPRESSION_ENABLE)
+ fb->modifier =
+ I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC;
else
fb->modifier = I915_FORMAT_MOD_4_TILED;
} else {
diff --git a/include/uapi/drm/drm_fourcc.h
b/include/ua
Re: [PATCH v2 2/2] drm/msm/dpu: Add SC8180x to hw catalog
On Tue 15 Feb 11:14 CST 2022, Abhinav Kumar wrote:
>
>
> On 2/14/2022 8:33 PM, Bjorn Andersson wrote:
> > From: Rob Clark
> >
> > Add SC8180x to the hardware catalog, for initial support for the
> > platform. Due to limitations in the DP driver only one of the four DP
> > interfaces is left enabled.
> >
> > The SC8180x platform supports the newly added DPU_INTF_WIDEBUS flag and
> > the Windows-on-Snapdragon bootloader leaves the widebus bit set, so this
> > is flagged appropriately to ensure widebus is disabled - for now.
> >
> > Signed-off-by: Rob Clark
> > [bjorn: Reworked intf and irq definitions]
> > Signed-off-by: Bjorn Andersson
> > ---
> >
> > Changes since v1:
> > - Dropped widebus flag
> >
> > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 129 ++
> > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> > drivers/gpu/drm/msm/msm_drv.c | 1 +
> > 4 files changed, 132 insertions(+)
> >
> > 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 aa75991903a6..7ac0fe32df49 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -90,6 +90,17 @@
> > BIT(MDP_INTF3_INTR) | \
> > BIT(MDP_INTF4_INTR))
> > +#define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
> > + BIT(MDP_SSPP_TOP0_INTR2) | \
> > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> > + BIT(MDP_INTF0_INTR) | \
> > + BIT(MDP_INTF1_INTR) | \
> > + BIT(MDP_INTF2_INTR) | \
> > + BIT(MDP_INTF3_INTR) | \
> > + BIT(MDP_INTF4_INTR) | \
> > + BIT(MDP_INTF5_INTR) | \
> > + BIT(MDP_AD4_0_INTR) | \
> > + BIT(MDP_AD4_1_INTR))
> > #define DEFAULT_PIXEL_RAM_SIZE(50 * 1024)
> > #define DEFAULT_DPU_LINE_WIDTH2048
> > @@ -225,6 +236,22 @@ static const struct dpu_caps sm8150_dpu_caps = {
> > .max_vdeci_exp = MAX_VERT_DECIMATION,
> > };
> > +static const struct dpu_caps sc8180x_dpu_caps = {
> > + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > + .max_mixer_blendstages = 0xb,
> > + .qseed_type = DPU_SSPP_SCALER_QSEED3,
> > + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
> > + .ubwc_version = DPU_HW_UBWC_VER_30,
> > + .has_src_split = true,
> > + .has_dim_layer = true,
> > + .has_idle_pc = true,
> > + .has_3d_merge = true,
> > + .max_linewidth = 4096,
> > + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> > + .max_hdeci_exp = MAX_HORZ_DECIMATION,
> > + .max_vdeci_exp = MAX_VERT_DECIMATION,
> > +};
> > +
> > static const struct dpu_caps sm8250_dpu_caps = {
> > .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > .max_mixer_blendstages = 0xb,
> > @@ -293,6 +320,31 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
> > },
> > };
> > +static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> > + {
> > + .name = "top_0", .id = MDP_TOP,
> > + .base = 0x0, .len = 0x45C,
> > + .features = 0,
> > + .highest_bank_bit = 0x3,
> > + .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
> > + .reg_off = 0x2AC, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
> > + .reg_off = 0x2B4, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
> > + .reg_off = 0x2BC, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
> > + .reg_off = 0x2C4, .bit_off = 0},
> > + .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> > + .reg_off = 0x2AC, .bit_off = 8},
> > + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> > + .reg_off = 0x2B4, .bit_off = 8},
> > + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> > + .reg_off = 0x2BC, .bit_off = 8},
> > + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> > + .reg_off = 0x2C4, .bit_off = 8},
> > + },
> > +};
> > +
> > static const struct dpu_mdp_cfg sm8250_mdp[] = {
> > {
> > .name = "top_0", .id = MDP_TOP,
> > @@ -861,6 +913,16 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
> > INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
> > INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
> > };
> > +static const struct dpu_intf_cfg sc8180x_intf[] = {
> > + INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
> > INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
> > + INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
> > MDP_SSPP_TOP0_INTR, 26, 27),
> > + INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK,
> > MDP_SSPP_TOP0_INTR, 28, 29),
> > + /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until
> > this is supported */
> > + INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999
[PATCH 2/6] drm/ttm: add resource iterator v3
Instead of duplicating that at different places add an iterator over all
the resources in a resource manager.
v2: add lockdep annotation and kerneldoc
v3: fix various bugs pointed out by Felix
Signed-off-by: Christian König
Tested-by: Bas Nieuwenhuizen
Reviewed-by: Daniel Vetter (v3)
---
drivers/gpu/drm/ttm/ttm_bo.c | 42 ++--
drivers/gpu/drm/ttm/ttm_device.c | 26 +++
drivers/gpu/drm/ttm/ttm_resource.c | 51 ++
include/drm/ttm/ttm_resource.h | 23 ++
4 files changed, 102 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb0fa932d495..11e698e3374c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -579,38 +579,30 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource_cursor cursor;
struct ttm_resource *res;
bool locked = false;
- unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(res, &man->lru[i], lru) {
- bool busy;
-
- bo = res->bo;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
- &locked, &busy)) {
- if (busy && !busy_bo && ticket !=
- dma_resv_locking_ctx(bo->base.resv))
- busy_bo = bo;
- continue;
- }
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
- break;
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
+ &locked, &busy)) {
+ if (busy && !busy_bo && ticket !=
+ dma_resv_locking_ctx(res->bo->base.resv))
+ busy_bo = res->bo;
+ continue;
}
- /* If the inner loop terminated early, we have our candidate */
- if (&res->lru != &man->lru[i])
- break;
-
- bo = NULL;
+ if (!ttm_bo_get_unless_zero(res->bo)) {
+ if (locked)
+ dma_resv_unlock(res->bo->base.resv);
+ continue;
+ }
+ bo = res->bo;
+ break;
}
if (!bo) {
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index ba35887147ba..a0562ab386f5 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout);
int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
gfp_t gfp_flags)
{
+ struct ttm_resource_cursor cursor;
struct ttm_resource_manager *man;
- struct ttm_buffer_object *bo;
struct ttm_resource *res;
- unsigned i, j;
+ unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
@@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct
ttm_operation_ctx *ctx,
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(res, &man->lru[j], lru) {
- uint32_t num_pages;
-
- bo = res->bo;
- num_pages = PFN_UP(bo->base.size);
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ struct ttm_buffer_object *bo = res->bo;
+ uint32_t num_pages = PFN_UP(bo->base.size);
- ret = ttm_bo_swapout(bo, ctx, gfp_flags);
- /* ttm_bo_swapout has dropped the lru_lock */
- if (!ret)
- return num_pages;
- if (ret != -EBUSY)
- return ret;
- }
+ ret = ttm_bo_swapout(bo, ctx, gfp_flags);
+ /* ttm_bo_swapout has dropped the lru_lock */
+ if (!ret)
+ return num_pages;
+ if (ret != -EBUSY)
+ return ret;
