Re: [Freedreno] [PATCH v6 06/15] drm/msm/a6xx: Introduce GMU wrapper support
On Sat, Apr 01, 2023 at 01:54:43PM +0200, Konrad Dybcio wrote: > Some (particularly SMD_RPM, a.k.a non-RPMh) SoCs implement A6XX GPUs > but don't implement the associated GMUs. This is due to the fact that > the GMU directly pokes at RPMh. Sadly, this means we have to take care > of enabling & scaling power rails, clocks and bandwidth ourselves. > > Reuse existing Adreno-common code and modify the deeply-GMU-infused > A6XX code to facilitate these GPUs. This involves if-ing out lots > of GMU callbacks and introducing a new type of GMU - GMU wrapper (it's > the actual name that Qualcomm uses in their downstream kernels). > > This is essentially a register region which is convenient to model > as a device. We'll use it for managing the GDSCs. The register > layout matches the actual GMU_CX/GX regions on the "real GMU" devices > and lets us reuse quite a bit of gmu_read/write/rmw calls. << I sent a reply to this patch earlier, but not sure where it went. Still figuring out Mutt... >> Only convenience I found is that we can reuse gmu register ops in a few places (< 10 I think). If we just model this as another gpu memory region, I think it will help to keep gmu vs gmu-wrapper/no-gmu architecture code with clean separation. Also, it looks like we need to keep a dummy gmu platform device in the devicetree with the current approach. That doesn't sound right. > > Signed-off-by: Konrad Dybcio > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 72 +++- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 255 > +--- > drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 8 +- > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 6 + > 6 files changed, 318 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 87babbb2a19f..b1acdb027205 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -1469,6 +1469,7 @@ static int a6xx_gmu_get_irq(struct a6xx_gmu *gmu, > struct platform_device *pdev, > > void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) > { > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > struct platform_device *pdev = to_platform_device(gmu->dev); > > @@ -1494,10 +1495,12 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) > gmu->mmio = NULL; > gmu->rscc = NULL; > > - a6xx_gmu_memory_free(gmu); > + if (!adreno_has_gmu_wrapper(adreno_gpu)) { > + a6xx_gmu_memory_free(gmu); > > - free_irq(gmu->gmu_irq, gmu); > - free_irq(gmu->hfi_irq, gmu); > + free_irq(gmu->gmu_irq, gmu); > + free_irq(gmu->hfi_irq, gmu); > + } > > /* Drop reference taken in of_find_device_by_node */ > put_device(gmu->dev); > @@ -1516,6 +1519,69 @@ static int cxpd_notifier_cb(struct notifier_block *nb, > return 0; > } > > +int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node > *node) > +{ > + struct platform_device *pdev = of_find_device_by_node(node); > + struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > + int ret; > + > + if (!pdev) > + return -ENODEV; > + > + gmu->dev = &pdev->dev; > + > + of_dma_configure(gmu->dev, node, true); why setup dma for a device that is not actually present? > + > + pm_runtime_enable(gmu->dev); > + > + /* Mark legacy for manual SPTPRAC control */ > + gmu->legacy = true; > + > + /* Map the GMU registers */ > + gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu"); > + if (IS_ERR(gmu->mmio)) { > + ret = PTR_ERR(gmu->mmio); > + goto err_mmio; > + } > + > + gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx"); > + if (IS_ERR(gmu->cxpd)) { > + ret = PTR_ERR(gmu->cxpd); > + goto err_mmio; > + } > + > + if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) { > + ret = -ENODEV; > + goto detach_cxpd; > + } > + > + init_completion(&gmu->pd_gate); > + complete_all(&gmu->pd_gate); > + gmu->pd_nb.notifier_call = cxpd_notifier_cb; > + > + /* Get a link to the GX power domain to reset the GPU */ > + gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx"); > + if (IS_ERR(gmu->gxpd)) { > + ret = PTR_ERR(gmu->gxpd); > + goto err_mmio; > + } > + > + gmu->initialized = true; > + > + return 0; > + > +detach_cxpd: > + dev_pm_domain_detach(gmu->cxpd, false); > + > +err_mmio: > + iounmap(gmu->mmio); > + > + /* Drop reference taken in of_find_device_by_node */ > + put_device(gmu->dev); > + > + return ret; > +} > + > int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node) > { > struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > diff --git a/drivers/gpu
Re: [Freedreno] [PATCH v2 8/9] drm/fdinfo: Add comm/cmdline override fields
On 01/05/2023 17:58, Rob Clark wrote: On Fri, Apr 28, 2023 at 4:05 AM Tvrtko Ursulin wrote: On 27/04/2023 18:53, Rob Clark wrote: From: Rob Clark These are useful in particular for VM scenarios where the process which has opened to drm device file is just a proxy for the real user in a VM guest. Signed-off-by: Rob Clark --- Documentation/gpu/drm-usage-stats.rst | 18 ++ drivers/gpu/drm/drm_file.c| 15 +++ include/drm/drm_file.h| 19 +++ 3 files changed, 52 insertions(+) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index 58dc0d3f8c58..e4877cf8089c 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -73,6 +73,24 @@ scope of each device, in which case `drm-pdev` shall be present as well. Userspace should make sure to not double account any usage statistics by using the above described criteria in order to associate data to individual clients. +- drm-comm-override: + +Returns the client executable override string. Some drivers support letting +userspace override this in cases where the userspace is simply a "proxy". +Such as is the case with virglrenderer drm native context, where the host +process is just forwarding command submission, etc, from guest userspace. +This allows the proxy to make visible the executable name of the actual +app in the VM guest. + +- drm-cmdline-override: + +Returns the client cmdline override string. Some drivers support letting +userspace override this in cases where the userspace is simply a "proxy". +Such as is the case with virglrenderer drm native context, where the host +process is just forwarding command submission, etc, from guest userspace. +This allows the proxy to make visible the cmdline of the actual app in the +VM guest. Perhaps it would be okay to save space here by not repeating the description, like: drm-comm-override: drm-cmdline-override: Long description blah blah... This allows the proxy to make visible the _executable name *and* command line_ blah blah.. + Utilization ^^^ diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 9321eb0bf020..d7514c313af1 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -178,6 +178,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) spin_lock_init(&file->master_lookup_lock); mutex_init(&file->event_read_lock); + mutex_init(&file->override_lock); + if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_open(dev, file); @@ -292,6 +294,8 @@ void drm_file_free(struct drm_file *file) WARN_ON(!list_empty(&file->event_list)); put_pid(file->pid); + kfree(file->override_comm); + kfree(file->override_cmdline); kfree(file); } @@ -995,6 +999,17 @@ void drm_show_fdinfo(struct seq_file *m, struct file *f) PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); } + mutex_lock(&file->override_lock); You could add a fast unlocked check before taking the mutex for no risk apart a transient false negative. For 99.% of userspace it would mean no pointless lock/unlock cycle. I'm not sure I get your point? This needs to be serialized against userspace setting the override values if (file->override_comm || file->override_cmdline) { mutex_lock(&file->override_lock); if (file->override_comm) drm_printf(&p, "drm-comm-override:\t%s\n", file->override_comm); if (file->override_cmdline) drm_printf(&p, "drm-cmdline-override:\t%s\n", file->override_cmdline); mutext_unlock(&file->override_lock); } No risk apart for a transient false negative (which is immaterial for userspace since fdinfo reads are not ordered versus the override setting anyway) and 99.9% of deployments can get by not needing to pointlessly cycle the lock. + if (file->override_comm) { + drm_printf(&p, "drm-comm-override:\t%s\n", +file->override_comm); + } + if (file->override_cmdline) { + drm_printf(&p, "drm-cmdline-override:\t%s\n", +file->override_cmdline); + } + mutex_unlock(&file->override_lock); + if (dev->driver->show_fdinfo) dev->driver->show_fdinfo(&p, file); } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 1339e925af52..604d05fa6f0c 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -370,6 +370,25 @@ struct drm_file { */ struct drm_prime_file_private prime; + /** + * @comm: Overridden task comm + * + * Accessed under override_lock + */ + char *override_comm; + + /** + * @cmdline: Overridden task cmdline + * + * Accessed under override_lock + */ + ch
Re: [Freedreno] [PATCH v2 5/9] drm: Add fdinfo memory stats
On 28/04/2023 15:45, Rob Clark wrote: On Fri, Apr 28, 2023 at 3:56 AM Tvrtko Ursulin wrote: On 27/04/2023 18:53, Rob Clark wrote: From: Rob Clark Add support to dump GEM stats to fdinfo. v2: Fix typos, change size units to match docs, use div_u64 v3: Do it in core v4: more kerneldoc Signed-off-by: Rob Clark Reviewed-by: Emil Velikov Reviewed-by: Daniel Vetter --- Documentation/gpu/drm-usage-stats.rst | 54 +++ drivers/gpu/drm/drm_file.c| 99 ++- include/drm/drm_file.h| 19 + include/drm/drm_gem.h | 30 4 files changed, 189 insertions(+), 13 deletions(-) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index 552195fb1ea3..bfc14150452c 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -52,6 +52,9 @@ String shall contain the name this driver registered as via the respective Optional fully standardised keys +Identification +^^ + - drm-pdev: For PCI devices this should contain the PCI slot address of the device in @@ -69,6 +72,9 @@ scope of each device, in which case `drm-pdev` shall be present as well. Userspace should make sure to not double account any usage statistics by using the above described criteria in order to associate data to individual clients. +Utilization +^^^ + - drm-engine-: ns GPUs usually contain multiple execution engines. Each shall be given a stable @@ -93,18 +99,6 @@ exported engine corresponds to a group of identical hardware engines. In the absence of this tag parser shall assume capacity of one. Zero capacity is not allowed. -- drm-memory-: [KiB|MiB] - -Each possible memory type which can be used to store buffer objects by the -GPU in question shall be given a stable and unique name to be returned as the -string here. - -Value shall reflect the amount of storage currently consumed by the buffer -object belong to this client, in the respective memory region. - -Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' -indicating kibi- or mebi-bytes. - - drm-cycles-: Engine identifier string must be the same as the one specified in the @@ -126,6 +120,42 @@ percentage utilization of the engine, whereas drm-engine- only reflects time active without considering what frequency the engine is operating as a percentage of it's maximum frequency. +Memory +^^ + +- drm-memory-: [KiB|MiB] + +Each possible memory type which can be used to store buffer objects by the +GPU in question shall be given a stable and unique name to be returned as the +string here. The name "memory" is reserved to refer to normal system memory. How is the name memory reserved, I mean when which part of the key? Obviously amdgpu exposes drm-memory-vram so it can't mean system memory there. [Comes back later] Ah I see.. you meant the _region_ name "memory" is reserved. Which applies to the below keys, not the one above. Hmm.. So for multi-region drivers you meant like: right, I thought "drm-memory-memory" sounded silly, and otherwise "memory" fit elsewhere as below, so "memory" seemed like a reasonable region name ;-) drm-total-memory: drm-total-vram: Etc. Okay I think that works. All prefixes "drm-$category" become reserved ones effectively but I think that is okay. + +Value shall reflect the amount of storage currently consumed by the buffer +object belong to this client, in the respective memory region. OMG it is all my fault for mentioning buffer objects here... :) Maybe just fix the plural while moving. Or maybe there is time to s/buffer objects/memory/ too? Why not I think. It would leave things more future proof. + +Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' +indicating kibi- or mebi-bytes. + +- drm-shared-: [KiB|MiB] + +The total size of buffers that are shared with another file (ie. have more +than a single handle). + +- drm-private-: [KiB|MiB] + +The total size of buffers that are not shared with another file. You went back to private + shared for a specific reason? I thought we agreed total + shared can be less confusing. opps, yes, I forgot to update the rst + +- drm-resident-: [KiB|MiB] + +The total size of buffers that are resident in system memory. "..resident in the specified memory region."? + +- drm-purgeable-: [KiB|MiB] + +The total size of buffers that are purgeable. + +- drm-active-: [KiB|MiB] + +The total size of buffers that are active on one or more rings. Under utilisation we used 'engines' so introducing 'rings' at least needs clarification, maybe a terminology chapter? Or just use engines for consistency? using "engines" works for me + Implementation Details == diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 6d5bdd684ae2..9321eb0bf02
Re: [Freedreno] [PATCH v6 06/15] drm/msm/a6xx: Introduce GMU wrapper support
On 2.05.2023 09:49, Akhil P Oommen wrote: > On Sat, Apr 01, 2023 at 01:54:43PM +0200, Konrad Dybcio wrote: >> Some (particularly SMD_RPM, a.k.a non-RPMh) SoCs implement A6XX GPUs >> but don't implement the associated GMUs. This is due to the fact that >> the GMU directly pokes at RPMh. Sadly, this means we have to take care >> of enabling & scaling power rails, clocks and bandwidth ourselves. >> >> Reuse existing Adreno-common code and modify the deeply-GMU-infused >> A6XX code to facilitate these GPUs. This involves if-ing out lots >> of GMU callbacks and introducing a new type of GMU - GMU wrapper (it's >> the actual name that Qualcomm uses in their downstream kernels). >> >> This is essentially a register region which is convenient to model >> as a device. We'll use it for managing the GDSCs. The register >> layout matches the actual GMU_CX/GX regions on the "real GMU" devices >> and lets us reuse quite a bit of gmu_read/write/rmw calls. > << I sent a reply to this patch earlier, but not sure where it went. > Still figuring out Mutt... >> Answered it here: https://lore.kernel.org/linux-arm-msm/4d3000c1-c3f9-0bfd-3eb3-23393f9a8...@linaro.org/ I don't think I see any new comments in this "reply revision" (heh), so please check that one out. > > Only convenience I found is that we can reuse gmu register ops in a few > places (< 10 I think). If we just model this as another gpu memory > region, I think it will help to keep gmu vs gmu-wrapper/no-gmu > architecture code with clean separation. Also, it looks like we need to > keep a dummy gmu platform device in the devicetree with the current > approach. That doesn't sound right. That's correct, but.. if we switch away from that, VDD_GX/VDD_CX will need additional, gmuwrapper-configuration specific code anyway, as OPP & genpd will no longer make use of the default behavior which only gets triggered if there's a single power-domains=<> entry, afaicu. If nothing else, this is a very convenient way to model a part of the GPU (as that's essentially what GMU_CX is, to my understanding) and the bindings people didn't shoot me in the head for proposing this, so I assume it'd be cool to pursue this.. Konrad >> >> Signed-off-by: Konrad Dybcio >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 72 +++- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 255 >> +--- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + >> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +- >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 8 +- >> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 6 + >> 6 files changed, 318 insertions(+), 38 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> index 87babbb2a19f..b1acdb027205 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> @@ -1469,6 +1469,7 @@ static int a6xx_gmu_get_irq(struct a6xx_gmu *gmu, >> struct platform_device *pdev, >> >> void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) >> { >> +struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; >> struct a6xx_gmu *gmu = &a6xx_gpu->gmu; >> struct platform_device *pdev = to_platform_device(gmu->dev); >> >> @@ -1494,10 +1495,12 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) >> gmu->mmio = NULL; >> gmu->rscc = NULL; >> >> -a6xx_gmu_memory_free(gmu); >> +if (!adreno_has_gmu_wrapper(adreno_gpu)) { >> +a6xx_gmu_memory_free(gmu); >> >> -free_irq(gmu->gmu_irq, gmu); >> -free_irq(gmu->hfi_irq, gmu); >> +free_irq(gmu->gmu_irq, gmu); >> +free_irq(gmu->hfi_irq, gmu); >> +} >> >> /* Drop reference taken in of_find_device_by_node */ >> put_device(gmu->dev); >> @@ -1516,6 +1519,69 @@ static int cxpd_notifier_cb(struct notifier_block *nb, >> return 0; >> } >> >> +int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node >> *node) >> +{ >> +struct platform_device *pdev = of_find_device_by_node(node); >> +struct a6xx_gmu *gmu = &a6xx_gpu->gmu; >> +int ret; >> + >> +if (!pdev) >> +return -ENODEV; >> + >> +gmu->dev = &pdev->dev; >> + >> +of_dma_configure(gmu->dev, node, true); > why setup dma for a device that is not actually present? >> + >> +pm_runtime_enable(gmu->dev); >> + >> +/* Mark legacy for manual SPTPRAC control */ >> +gmu->legacy = true; >> + >> +/* Map the GMU registers */ >> +gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu"); >> +if (IS_ERR(gmu->mmio)) { >> +ret = PTR_ERR(gmu->mmio); >> +goto err_mmio; >> +} >> + >> +gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx"); >> +if (IS_ERR(gmu->cxpd)) { >> +ret = PTR_ERR(gmu->cxpd); >> +goto err_mmio; >> +} >> + >> +if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) { >> +ret = -ENODEV; >> +goto detach_cxpd;
Re: [Freedreno] [PATCH 3/9] drm/msm/dpu: fix the condition for (not) applying QoS to CURSOR SSPP
On 02/05/2023 03:56, Jeykumar Sankaran wrote: On 4/30/2023 1:57 PM, Dmitry Baryshkov wrote: The function dpu_plane_sspp_update_pipe() contains code to skip enabling the QoS and OT limitis for CURSOR pipes. However all DPU since sdm845 repurpose DMA SSPP for the cursor planes because they lack the real CURSOR SSPP. Fix the condition to actually check that the plane is CURSOR or not. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 43d9fbc0c687..36f6eb71fef8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1124,7 +1124,8 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, _dpu_plane_set_qos_lut(plane, pipe, fmt, pipe_cfg); _dpu_plane_set_danger_lut(plane, pipe, fmt); - if (plane->type != DRM_PLANE_TYPE_CURSOR) { + if (pipe->sspp->idx == SSPP_CURSOR0 || + pipe->sspp->idx == SSPP_CURSOR1) { Isn't this differ from the current sequence: The existing sequence programs QOS for all the non-cursor SSPP's. This patch programs QOS only for CURSOR SSPP's. Thanks for the catch! I was thinking about inverting the condition and ended up overengineering it. If DMA SSPP's are used for cursor planes, we should ideally remove this check. Unfortunately, we also support 8998 (and patches to use CURSOR SSPP were posted to the mailing list). The plan is to also support some of 1.x MDP5/DPU units (e.g. 8996), which would also make use of origin CURSOR planes. So we can not drop this. I'll post v2 fixing the issue. Jeykumar S. _dpu_plane_set_qos_ctrl(plane, pipe, true, DPU_PLANE_QOS_PANIC_CTRL); _dpu_plane_set_ot_limit(plane, pipe, pipe_cfg, frame_rate); } -- With best wishes Dmitry
[Freedreno] [PATCH v2 1/9] drm/msm/dpu: fix SSPP register definitions
Reorder SSPP register definitions to sort them in the ascending order. Move register bitfields after the register definitions. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 66 +++-- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index 6b68ec5c7a5a..1bf717290dab 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -26,45 +26,18 @@ #define SSPP_SRC_FORMAT0x30 #define SSPP_SRC_UNPACK_PATTERN0x34 #define SSPP_SRC_OP_MODE 0x38 - -/* SSPP_MULTIRECT*/ -#define SSPP_SRC_SIZE_REC1 0x16C -#define SSPP_SRC_XY_REC1 0x168 -#define SSPP_OUT_SIZE_REC1 0x160 -#define SSPP_OUT_XY_REC1 0x164 -#define SSPP_SRC_FORMAT_REC1 0x174 -#define SSPP_SRC_UNPACK_PATTERN_REC1 0x178 -#define SSPP_SRC_OP_MODE_REC1 0x17C -#define SSPP_MULTIRECT_OPMODE 0x170 -#define SSPP_SRC_CONSTANT_COLOR_REC1 0x180 -#define SSPP_EXCL_REC_SIZE_REC10x184 -#define SSPP_EXCL_REC_XY_REC1 0x188 - -#define MDSS_MDP_OP_DEINTERLACEBIT(22) -#define MDSS_MDP_OP_DEINTERLACE_ODDBIT(23) -#define MDSS_MDP_OP_IGC_ROM_1 BIT(18) -#define MDSS_MDP_OP_IGC_ROM_0 BIT(17) -#define MDSS_MDP_OP_IGC_EN BIT(16) -#define MDSS_MDP_OP_FLIP_UDBIT(14) -#define MDSS_MDP_OP_FLIP_LRBIT(13) -#define MDSS_MDP_OP_BWC_EN BIT(0) -#define MDSS_MDP_OP_PE_OVERRIDEBIT(31) -#define MDSS_MDP_OP_BWC_LOSSLESS (0 << 1) -#define MDSS_MDP_OP_BWC_Q_HIGH (1 << 1) -#define MDSS_MDP_OP_BWC_Q_MED (2 << 1) - #define SSPP_SRC_CONSTANT_COLOR0x3c #define SSPP_EXCL_REC_CTL 0x40 #define SSPP_UBWC_STATIC_CTRL 0x44 -#define SSPP_FETCH_CONFIG 0x048 +#define SSPP_FETCH_CONFIG 0x48 #define SSPP_DANGER_LUT0x60 #define SSPP_SAFE_LUT 0x64 #define SSPP_CREQ_LUT 0x68 #define SSPP_QOS_CTRL 0x6C -#define SSPP_DECIMATION_CONFIG 0xB4 #define SSPP_SRC_ADDR_SW_STATUS0x70 #define SSPP_CREQ_LUT_00x74 #define SSPP_CREQ_LUT_10x78 +#define SSPP_DECIMATION_CONFIG 0xB4 #define SSPP_SW_PIX_EXT_C0_LR 0x100 #define SSPP_SW_PIX_EXT_C0_TB 0x104 #define SSPP_SW_PIX_EXT_C0_REQ_PIXELS 0x108 @@ -81,11 +54,33 @@ #define SSPP_TRAFFIC_SHAPER_PREFILL0x150 #define SSPP_TRAFFIC_SHAPER_REC1_PREFILL 0x154 #define SSPP_TRAFFIC_SHAPER_REC1 0x158 +#define SSPP_OUT_SIZE_REC1 0x160 +#define SSPP_OUT_XY_REC1 0x164 +#define SSPP_SRC_XY_REC1 0x168 +#define SSPP_SRC_SIZE_REC1 0x16C +#define SSPP_MULTIRECT_OPMODE 0x170 +#define SSPP_SRC_FORMAT_REC1 0x174 +#define SSPP_SRC_UNPACK_PATTERN_REC1 0x178 +#define SSPP_SRC_OP_MODE_REC1 0x17C +#define SSPP_SRC_CONSTANT_COLOR_REC1 0x180 +#define SSPP_EXCL_REC_SIZE_REC10x184 +#define SSPP_EXCL_REC_XY_REC1 0x188 #define SSPP_EXCL_REC_SIZE 0x1B4 #define SSPP_EXCL_REC_XY 0x1B8 -#define SSPP_VIG_OP_MODE 0x0 -#define SSPP_VIG_CSC_10_OP_MODE0x0 -#define SSPP_TRAFFIC_SHAPER_BPC_MAX0xFF + +/* SSPP_SRC_OP_MODE & OP_MODE_REC1 */ +#define MDSS_MDP_OP_DEINTERLACEBIT(22) +#define MDSS_MDP_OP_DEINTERLACE_ODDBIT(23) +#define MDSS_MDP_OP_IGC_ROM_1 BIT(18) +#define MDSS_MDP_OP_IGC_ROM_0 BIT(17) +#define MDSS_MDP_OP_IGC_EN BIT(16) +#define MDSS_MDP_OP_FLIP_UDBIT(14) +#define MDSS_MDP_OP_FLIP_LRBIT(13) +#define MDSS_MDP_OP_BWC_EN BIT(0) +#define MDSS_MDP_OP_PE_OVERRIDEBIT(31) +#define MDSS_MDP_OP_BWC_LOSSLESS (0 << 1) +#define MDSS_MDP_OP_BWC_Q_HIGH (1 << 1) +#define MDSS_MDP_OP_BWC_Q_MED (2 << 1) /* SSPP_QOS_CTRL */ #define SSPP_QOS_CTRL_VBLANK_ENBIT(16) @@ -96,6 +91,7 @@ #define SSPP_QOS_CTRL_CREQ_VBLANK_OFF 20 /* DPU_SSPP_SCALER_QSEED2 */ +#define SSPP_VIG_OP_MODE 0x0 #define SCALE_CONFIG 0x04 #define COMP0_3_PHASE_STEP_X 0x10 #define COMP0_3_PHASE_STEP_Y 0x14 @@ -107,6 +103,12 @@ #define COMP1_2_INIT_PHASE_Y 0x2C #define VIG_0_QSEED2_SHARP 0x30 +/* DPU_SSPP_CSC_10BIT space */ +#define SSPP_VIG_CSC_10_OP_MODE0x0 + +/* SSPP_TRAFFIC_SHAPER and _REC1 */ +#define SSPP_TRAFFI
[Freedreno] [PATCH v2 0/9] drm/msm/dpu: simplify QoS/CDP programming
Merge SSPP and WB code programming QoS and CDP. This allows us to drop intermediate structures and duplicate code. Changes since v1: - Fixed kerneldoc for _dpu_plane_set_qos_ctrl() - Fixed danger_safe_en programming conditions (Jeykumar) - Simplified the code surrounding setup_cdp() calls (Jeykumar) Dmitry Baryshkov (9): drm/msm/dpu: fix SSPP register definitions drm/msm/dpu: simplify CDP programming drm/msm/dpu: fix the condition for (not) applying QoS to CURSOR SSPP drm/msm/dpu: rearrange QoS setting code drm/msm/dpu: drop DPU_PLANE_QOS_VBLANK_CTRL drm/msm/dpu: simplify qos_ctrl handling drm/msm/dpu: drop DPU_PLANE_QOS_PANIC_CTRL drm/msm/dpu: remove struct dpu_hw_pipe_qos_cfg drm/msm/dpu: use common helper for WB and SSPP QoS setup .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 21 +-- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 4 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 142 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 52 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 52 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 32 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 48 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 27 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 165 +- 9 files changed, 194 insertions(+), 349 deletions(-) -- 2.39.2
[Freedreno] [PATCH v2 3/9] drm/msm/dpu: fix the condition for (not) applying QoS to CURSOR SSPP
The function dpu_plane_sspp_update_pipe() contains code to skip enabling the QoS and OT limitis for CURSOR pipes. However all DPU since sdm845 repurpose DMA SSPP for the cursor planes because they lack the real CURSOR SSPP. Fix the condition to actually check that the plane is CURSOR or not. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 3b210320ea62..b8ed7247a6af 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1126,7 +1126,8 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, _dpu_plane_set_qos_lut(plane, pipe, fmt, pipe_cfg); _dpu_plane_set_danger_lut(plane, pipe, fmt); - if (plane->type != DRM_PLANE_TYPE_CURSOR) { + if (pipe->sspp->idx != SSPP_CURSOR0 && + pipe->sspp->idx != SSPP_CURSOR1) { _dpu_plane_set_qos_ctrl(plane, pipe, true, DPU_PLANE_QOS_PANIC_CTRL); _dpu_plane_set_ot_limit(plane, pipe, pipe_cfg, frame_rate); } -- 2.39.2
[Freedreno] [PATCH v2 4/9] drm/msm/dpu: rearrange QoS setting code
Slightly rearrainge code in dpu_plane_sspp_update_pipe() to group QoS/LUT related functions. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index b8ed7247a6af..586f089756fa 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1079,10 +1079,10 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, pipe->sspp->ops.setup_sourceaddress(pipe, layout); } - _dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL); - /* override for color fill */ if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { + _dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL); + /* skip remaining processing on color fill */ return; } @@ -1125,12 +1125,14 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, _dpu_plane_set_qos_lut(plane, pipe, fmt, pipe_cfg); _dpu_plane_set_danger_lut(plane, pipe, fmt); + _dpu_plane_set_qos_ctrl(plane, pipe, + pipe->sspp->idx != SSPP_CURSOR0 && + pipe->sspp->idx != SSPP_CURSOR1, + DPU_PLANE_QOS_PANIC_CTRL); if (pipe->sspp->idx != SSPP_CURSOR0 && - pipe->sspp->idx != SSPP_CURSOR1) { - _dpu_plane_set_qos_ctrl(plane, pipe, true, DPU_PLANE_QOS_PANIC_CTRL); + pipe->sspp->idx != SSPP_CURSOR1) _dpu_plane_set_ot_limit(plane, pipe, pipe_cfg, frame_rate); - } if (pstate->needs_qos_remap) _dpu_plane_set_qos_remap(plane, pipe); -- 2.39.2
[Freedreno] [PATCH v2 2/9] drm/msm/dpu: simplify CDP programming
Get rid of intermediatory configuration structure and defines. Pass the format and the enablement bit directly to the new helper. The WB_CDP_CNTL register ignores BIT(2), so we can write it for both SSPP and WB CDP settings. Signed-off-by: Dmitry Baryshkov --- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 17 --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 17 --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 14 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 19 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 19 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 11 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 +++--- 8 files changed, 45 insertions(+), 89 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index bac4aa807b4b..e7b65f6f53d6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -140,7 +140,6 @@ static void dpu_encoder_phys_wb_setup_fb(struct dpu_encoder_phys *phys_enc, struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc); struct dpu_hw_wb *hw_wb; struct dpu_hw_wb_cfg *wb_cfg; - struct dpu_hw_cdp_cfg cdp_cfg; if (!phys_enc || !phys_enc->dpu_kms || !phys_enc->dpu_kms->catalog) { DPU_ERROR("invalid encoder\n"); @@ -163,18 +162,10 @@ static void dpu_encoder_phys_wb_setup_fb(struct dpu_encoder_phys *phys_enc, hw_wb->ops.setup_outformat(hw_wb, wb_cfg); if (hw_wb->ops.setup_cdp) { - memset(&cdp_cfg, 0, sizeof(struct dpu_hw_cdp_cfg)); - - cdp_cfg.enable = phys_enc->dpu_kms->catalog->perf->cdp_cfg - [DPU_PERF_CDP_USAGE_NRT].wr_enable; - cdp_cfg.ubwc_meta_enable = - DPU_FORMAT_IS_UBWC(wb_cfg->dest.format); - cdp_cfg.tile_amortize_enable = - DPU_FORMAT_IS_UBWC(wb_cfg->dest.format) || - DPU_FORMAT_IS_TILE(wb_cfg->dest.format); - cdp_cfg.preload_ahead = DPU_WB_CDP_PRELOAD_AHEAD_64; - - hw_wb->ops.setup_cdp(hw_wb, &cdp_cfg); + const struct dpu_perf_cfg *perf = phys_enc->dpu_kms->catalog->perf; + + hw_wb->ops.setup_cdp(hw_wb, wb_cfg->dest.format, + perf->cdp_cfg[DPU_PERF_CDP_USAGE_NRT].wr_enable); } if (hw_wb->ops.setup_outaddress) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index 1bf717290dab..731199030336 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -592,13 +592,13 @@ static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_sspp *ctx, } static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe, - struct dpu_hw_cdp_cfg *cfg) + const struct dpu_format *fmt, + bool enable) { struct dpu_hw_sspp *ctx = pipe->sspp; - u32 cdp_cntl = 0; u32 cdp_cntl_offset = 0; - if (!ctx || !cfg) + if (!ctx) return; if (pipe->multirect_index == DPU_SSPP_RECT_SOLO || @@ -607,16 +607,7 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe, else cdp_cntl_offset = SSPP_CDP_CNTL_REC1; - if (cfg->enable) - cdp_cntl |= BIT(0); - if (cfg->ubwc_meta_enable) - cdp_cntl |= BIT(1); - if (cfg->tile_amortize_enable) - cdp_cntl |= BIT(2); - if (cfg->preload_ahead == DPU_SSPP_CDP_PRELOAD_AHEAD_64) - cdp_cntl |= BIT(3); - - DPU_REG_WRITE(&ctx->hw, cdp_cntl_offset, cdp_cntl); + dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable); } static void _setup_layer_ops(struct dpu_hw_sspp *c, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index 7a8d11ba618d..86bf4b2cda77 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -177,14 +177,6 @@ struct dpu_hw_pipe_qos_cfg { bool danger_safe_en; }; -/** - * enum CDP preload ahead address size - */ -enum { - DPU_SSPP_CDP_PRELOAD_AHEAD_32, - DPU_SSPP_CDP_PRELOAD_AHEAD_64 -}; - /** * struct dpu_hw_pipe_ts_cfg - traffic shaper configuration * @size: size to prefill in bytes, or zero to disable @@ -331,10 +323,12 @@ struct dpu_hw_sspp_ops { /** * setup_cdp - setup client driven prefetch * @pipe: Pointer to software pipe context -* @cfg: Pointer to cdp configuration +* @fmt: format used by the sw pipe +* @enable: whether the CD
[Freedreno] [PATCH v2 9/9] drm/msm/dpu: use common helper for WB and SSPP QoS setup
Rework SSPP and WB code to use common helper for programming QoS settings. Signed-off-by: Dmitry Baryshkov --- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 31 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 19 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 31 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 29 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 16 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 85 +++ 8 files changed, 100 insertions(+), 136 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index e7b65f6f53d6..023a9c4ad1db 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -102,7 +102,7 @@ static void dpu_encoder_phys_wb_set_qos_remap( static void dpu_encoder_phys_wb_set_qos(struct dpu_encoder_phys *phys_enc) { struct dpu_hw_wb *hw_wb; - struct dpu_hw_wb_qos_cfg qos_cfg; + struct dpu_hw_qos_cfg qos_cfg; const struct dpu_mdss_cfg *catalog; const struct dpu_qos_lut_tbl *qos_lut_tb; @@ -115,7 +115,7 @@ static void dpu_encoder_phys_wb_set_qos(struct dpu_encoder_phys *phys_enc) hw_wb = phys_enc->hw_wb; - memset(&qos_cfg, 0, sizeof(struct dpu_hw_wb_qos_cfg)); + memset(&qos_cfg, 0, sizeof(struct dpu_hw_qos_cfg)); qos_cfg.danger_safe_en = true; qos_cfg.danger_lut = catalog->perf->danger_lut_tbl[DPU_QOS_LUT_USAGE_NRT]; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index 2533c4629021..c35e9faf2460 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -541,30 +541,15 @@ static void dpu_hw_sspp_setup_solidfill(struct dpu_sw_pipe *pipe, u32 color) color); } -static void dpu_hw_sspp_setup_danger_safe_lut(struct dpu_hw_sspp *ctx, - u32 danger_lut, - u32 safe_lut) +static void dpu_hw_sspp_setup_qos_lut(struct dpu_hw_sspp *ctx, + struct dpu_hw_qos_cfg *cfg) { - if (!ctx) - return; - - DPU_REG_WRITE(&ctx->hw, SSPP_DANGER_LUT, danger_lut); - DPU_REG_WRITE(&ctx->hw, SSPP_SAFE_LUT, safe_lut); -} - -static void dpu_hw_sspp_setup_creq_lut(struct dpu_hw_sspp *ctx, - u64 creq_lut) -{ - if (!ctx) + if (!ctx || !cfg) return; - if (ctx->cap && test_bit(DPU_SSPP_QOS_8LVL, &ctx->cap->features)) { - DPU_REG_WRITE(&ctx->hw, SSPP_CREQ_LUT_0, creq_lut); - DPU_REG_WRITE(&ctx->hw, SSPP_CREQ_LUT_1, - creq_lut >> 32); - } else { - DPU_REG_WRITE(&ctx->hw, SSPP_CREQ_LUT, creq_lut); - } + _dpu_hw_setup_qos_lut(&ctx->hw, SSPP_DANGER_LUT, + test_bit(DPU_SSPP_QOS_8LVL, &ctx->cap->features), + cfg); } static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_sspp *ctx, @@ -606,9 +591,7 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c, c->ops.setup_pe = dpu_hw_sspp_setup_pe_config; if (test_bit(DPU_SSPP_QOS, &features)) { - c->ops.setup_danger_safe_lut = - dpu_hw_sspp_setup_danger_safe_lut; - c->ops.setup_creq_lut = dpu_hw_sspp_setup_creq_lut; + c->ops.setup_qos_lut = dpu_hw_sspp_setup_qos_lut; c->ops.setup_qos_ctrl = dpu_hw_sspp_setup_qos_ctrl; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index 4278c421b6ac..085f34bc6b88 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -254,25 +254,14 @@ struct dpu_hw_sspp_ops { void (*setup_sharpening)(struct dpu_hw_sspp *ctx, struct dpu_hw_sharp_cfg *cfg); - /** -* setup_danger_safe_lut - setup danger safe LUTs -* @ctx: Pointer to pipe context -* @danger_lut: LUT for generate danger level based on fill level -* @safe_lut: LUT for generate safe level based on fill level -* -*/ - void (*setup_danger_safe_lut)(struct dpu_hw_sspp *ctx, - u32 danger_lut, - u32 safe_lut); /** -* setup_creq_lut - setup CREQ LUT +* setup_qos_lut - setup QoS LUTs * @ctx: Pointer to pipe context -* @creq_lut: LUT for generate creq level based on fill level -* +* @cfg: LUT configuration */ - void (*setup_creq_lut)(struct dpu_hw_sspp *ctx, - u64 creq_lut); + void (*setup_qos_lut)(struc
[Freedreno] [PATCH v2 6/9] drm/msm/dpu: simplify qos_ctrl handling
After removal of DPU_PLANE_QOS_VBLANK_CTRL, several fields of struct dpu_hw_pipe_qos_cfg are fixed to false/0. Drop them from the structure (and drop the corresponding code from the functions). The DPU_PLANE_QOS_VBLANK_AMORTIZE flag is also removed, since it is now a NOP. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 10 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 6 -- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 17 ++--- 3 files changed, 2 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index b198def5534b..341e3a8fc927 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -575,16 +575,6 @@ static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_sspp *ctx, if (!ctx) return; - if (cfg->vblank_en) { - qos_ctrl |= ((cfg->creq_vblank & - SSPP_QOS_CTRL_CREQ_VBLANK_MASK) << - SSPP_QOS_CTRL_CREQ_VBLANK_OFF); - qos_ctrl |= ((cfg->danger_vblank & - SSPP_QOS_CTRL_DANGER_VBLANK_MASK) << - SSPP_QOS_CTRL_DANGER_VBLANK_OFF); - qos_ctrl |= SSPP_QOS_CTRL_VBLANK_EN; - } - if (cfg->danger_safe_en) qos_ctrl |= SSPP_QOS_CTRL_DANGER_SAFE_EN; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index 86bf4b2cda77..aaf6f41d546c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -165,15 +165,9 @@ struct dpu_sw_pipe_cfg { /** * struct dpu_hw_pipe_qos_cfg : Source pipe QoS configuration - * @creq_vblank: creq value generated to vbif during vertical blanking - * @danger_vblank: danger value generated during vertical blanking - * @vblank_en: enable creq_vblank and danger_vblank during vblank * @danger_safe_en: enable danger safe generation */ struct dpu_hw_pipe_qos_cfg { - u32 creq_vblank; - u32 danger_vblank; - bool vblank_en; bool danger_safe_en; }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 3cb891917b65..0ed350776775 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -73,12 +73,9 @@ static const uint32_t qcom_compressed_supported_formats[] = { /** * enum dpu_plane_qos - Different qos configurations for each pipe * - * @DPU_PLANE_QOS_VBLANK_AMORTIZE: Enables Amortization within pipe. - * this configuration is mutually exclusive from VBLANK_CTRL. * @DPU_PLANE_QOS_PANIC_CTRL: Setup panic for the pipe. */ enum dpu_plane_qos { - DPU_PLANE_QOS_VBLANK_AMORTIZE = BIT(1), DPU_PLANE_QOS_PANIC_CTRL = BIT(2), }; @@ -359,25 +356,15 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane, memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg)); - if (flags & DPU_PLANE_QOS_VBLANK_AMORTIZE) { - pipe_qos_cfg.vblank_en = false; - pipe_qos_cfg.creq_vblank = 0; /* clear vblank bits */ - } - if (flags & DPU_PLANE_QOS_PANIC_CTRL) pipe_qos_cfg.danger_safe_en = enable; - if (!pdpu->is_rt_pipe) { - pipe_qos_cfg.vblank_en = false; + if (!pdpu->is_rt_pipe) pipe_qos_cfg.danger_safe_en = false; - } - DPU_DEBUG_PLANE(pdpu, "pnum:%d ds:%d vb:%d pri[0x%x, 0x%x] is_rt:%d\n", + DPU_DEBUG_PLANE(pdpu, "pnum:%d ds:%d is_rt:%d\n", pdpu->pipe - SSPP_VIG0, pipe_qos_cfg.danger_safe_en, - pipe_qos_cfg.vblank_en, - pipe_qos_cfg.creq_vblank, - pipe_qos_cfg.danger_vblank, pdpu->is_rt_pipe); pipe->sspp->ops.setup_qos_ctrl(pipe->sspp, -- 2.39.2
[Freedreno] [PATCH v2 7/9] drm/msm/dpu: drop DPU_PLANE_QOS_PANIC_CTRL
This flag is always passed to _dpu_plane_set_qos_ctrl(), so drop it and remove corresponding conditions from the mentioned function. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 +++ 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 0ed350776775..d1443c4b2915 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -70,15 +70,6 @@ static const uint32_t qcom_compressed_supported_formats[] = { DRM_FORMAT_P010, }; -/** - * enum dpu_plane_qos - Different qos configurations for each pipe - * - * @DPU_PLANE_QOS_PANIC_CTRL: Setup panic for the pipe. - */ -enum dpu_plane_qos { - DPU_PLANE_QOS_PANIC_CTRL = BIT(2), -}; - /* * struct dpu_plane - local dpu plane structure * @aspace: address space pointer @@ -349,15 +340,14 @@ static void _dpu_plane_set_danger_lut(struct drm_plane *plane, */ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane, struct dpu_sw_pipe *pipe, - bool enable, u32 flags) + bool enable) { struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_hw_pipe_qos_cfg pipe_qos_cfg; memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg)); - if (flags & DPU_PLANE_QOS_PANIC_CTRL) - pipe_qos_cfg.danger_safe_en = enable; + pipe_qos_cfg.danger_safe_en = enable; if (!pdpu->is_rt_pipe) pipe_qos_cfg.danger_safe_en = false; @@ -1058,7 +1048,7 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, /* override for color fill */ if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { - _dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL); + _dpu_plane_set_qos_ctrl(plane, pipe, false); /* skip remaining processing on color fill */ return; @@ -1104,8 +1094,7 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, _dpu_plane_set_danger_lut(plane, pipe, fmt); _dpu_plane_set_qos_ctrl(plane, pipe, pipe->sspp->idx != SSPP_CURSOR0 && - pipe->sspp->idx != SSPP_CURSOR1, - DPU_PLANE_QOS_PANIC_CTRL); + pipe->sspp->idx != SSPP_CURSOR1); if (pipe->sspp->idx != SSPP_CURSOR0 && pipe->sspp->idx != SSPP_CURSOR1) @@ -1224,10 +1213,10 @@ static void dpu_plane_destroy(struct drm_plane *plane) if (pdpu) { pstate = to_dpu_plane_state(plane->state); - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, false, DPU_PLANE_QOS_PANIC_CTRL); + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, false); if (pstate->r_pipe.sspp) - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, false, DPU_PLANE_QOS_PANIC_CTRL); + _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, false); mutex_destroy(&pdpu->lock); @@ -1384,9 +1373,9 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) return; pm_runtime_get_sync(&dpu_kms->pdev->dev); - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable, DPU_PLANE_QOS_PANIC_CTRL); + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable); if (pstate->r_pipe.sspp) - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable, DPU_PLANE_QOS_PANIC_CTRL); + _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable); pm_runtime_put_sync(&dpu_kms->pdev->dev); } #endif -- 2.39.2
[Freedreno] [PATCH v2 8/9] drm/msm/dpu: remove struct dpu_hw_pipe_qos_cfg
Now as the struct dpu_hw_pipe_qos_cfg consists of only one bool field, drop the structure and use corresponding bool directly. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 10 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 13 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 11 +++ 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index 341e3a8fc927..2533c4629021 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -568,17 +568,13 @@ static void dpu_hw_sspp_setup_creq_lut(struct dpu_hw_sspp *ctx, } static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_sspp *ctx, - struct dpu_hw_pipe_qos_cfg *cfg) + bool danger_safe_en) { - u32 qos_ctrl = 0; - if (!ctx) return; - if (cfg->danger_safe_en) - qos_ctrl |= SSPP_QOS_CTRL_DANGER_SAFE_EN; - - DPU_REG_WRITE(&ctx->hw, SSPP_QOS_CTRL, qos_ctrl); + DPU_REG_WRITE(&ctx->hw, SSPP_QOS_CTRL, + danger_safe_en ? SSPP_QOS_CTRL_DANGER_SAFE_EN : 0); } static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index aaf6f41d546c..4278c421b6ac 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -163,14 +163,6 @@ struct dpu_sw_pipe_cfg { struct drm_rect dst_rect; }; -/** - * struct dpu_hw_pipe_qos_cfg : Source pipe QoS configuration - * @danger_safe_en: enable danger safe generation - */ -struct dpu_hw_pipe_qos_cfg { - bool danger_safe_en; -}; - /** * struct dpu_hw_pipe_ts_cfg - traffic shaper configuration * @size: size to prefill in bytes, or zero to disable @@ -285,11 +277,10 @@ struct dpu_hw_sspp_ops { /** * setup_qos_ctrl - setup QoS control * @ctx: Pointer to pipe context -* @cfg: Pointer to pipe QoS configuration -* +* @danger_safe_en: flags controlling enabling of danger/safe QoS/LUT */ void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx, - struct dpu_hw_pipe_qos_cfg *cfg); + bool danger_safe_en); /** * setup_histogram - setup histograms diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index d1443c4b2915..c8837d0aa0c3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -343,22 +343,17 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane, bool enable) { struct dpu_plane *pdpu = to_dpu_plane(plane); - struct dpu_hw_pipe_qos_cfg pipe_qos_cfg; - - memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg)); - - pipe_qos_cfg.danger_safe_en = enable; if (!pdpu->is_rt_pipe) - pipe_qos_cfg.danger_safe_en = false; + enable = false; DPU_DEBUG_PLANE(pdpu, "pnum:%d ds:%d is_rt:%d\n", pdpu->pipe - SSPP_VIG0, - pipe_qos_cfg.danger_safe_en, + enable, pdpu->is_rt_pipe); pipe->sspp->ops.setup_qos_ctrl(pipe->sspp, - &pipe_qos_cfg); + enable); } /** -- 2.39.2
[Freedreno] [PATCH v2 5/9] drm/msm/dpu: drop DPU_PLANE_QOS_VBLANK_CTRL
Drop support for DPU_PLANE_QOS_VBLANK_CTRL flag. It is not used both in upstream driver and in vendor SDE driver. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c| 8 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 10 -- 3 files changed, 22 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 b2831b45ac64..d47e7061a36b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -371,8 +371,6 @@ struct dpu_caps { /** * struct dpu_sspp_sub_blks : SSPP sub-blocks * common: Pointer to common configurations shared by sub blocks - * @creq_vblank: creq priority during vertical blanking - * @danger_vblank: danger priority during vertical blanking * @maxdwnscale: max downscale ratio supported(without DECIMATION) * @maxupscale: maxupscale ratio supported * @smart_dma_priority: hw priority of rect1 of multirect pipe @@ -387,8 +385,6 @@ struct dpu_caps { * @dpu_rotation_cfg: inline rotation configuration */ struct dpu_sspp_sub_blks { - u32 creq_vblank; - u32 danger_vblank; u32 maxdwnscale; u32 maxupscale; u32 smart_dma_priority; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index 731199030336..b198def5534b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -697,14 +697,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms, 0400, debugfs_root, (u32 *) &cfg->clk_ctrl); - debugfs_create_x32("creq_vblank", - 0600, - debugfs_root, - (u32 *) &sblk->creq_vblank); - debugfs_create_x32("danger_vblank", - 0600, - debugfs_root, - (u32 *) &sblk->danger_vblank); return 0; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 586f089756fa..3cb891917b65 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -73,13 +73,11 @@ static const uint32_t qcom_compressed_supported_formats[] = { /** * enum dpu_plane_qos - Different qos configurations for each pipe * - * @DPU_PLANE_QOS_VBLANK_CTRL: Setup VBLANK qos for the pipe. * @DPU_PLANE_QOS_VBLANK_AMORTIZE: Enables Amortization within pipe. * this configuration is mutually exclusive from VBLANK_CTRL. * @DPU_PLANE_QOS_PANIC_CTRL: Setup panic for the pipe. */ enum dpu_plane_qos { - DPU_PLANE_QOS_VBLANK_CTRL = BIT(0), DPU_PLANE_QOS_VBLANK_AMORTIZE = BIT(1), DPU_PLANE_QOS_PANIC_CTRL = BIT(2), }; @@ -361,15 +359,7 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane, memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg)); - if (flags & DPU_PLANE_QOS_VBLANK_CTRL) { - pipe_qos_cfg.creq_vblank = pipe->sspp->cap->sblk->creq_vblank; - pipe_qos_cfg.danger_vblank = - pipe->sspp->cap->sblk->danger_vblank; - pipe_qos_cfg.vblank_en = enable; - } - if (flags & DPU_PLANE_QOS_VBLANK_AMORTIZE) { - /* this feature overrules previous VBLANK_CTRL */ pipe_qos_cfg.vblank_en = false; pipe_qos_cfg.creq_vblank = 0; /* clear vblank bits */ } -- 2.39.2
[Freedreno] [PATCH 1/2] iommu/arm-smmu-qcom: Fix missing adreno_smmu's
From: Rob Clark When the special handling of qcom,adreno-smmu was moved into qcom_smmu_create(), it was overlooked that we didn't have all the required entries in qcom_smmu_impl_of_match. So we stopped getting adreno_smmu_priv on sc7180, breaking per-process pgtables. Fixes: 30b912a03d91 ("iommu/arm-smmu-qcom: Move the qcom,adreno-smmu check into qcom_smmu_create") Signed-off-by: Rob Clark --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index d1b296b95c86..88c89424485b 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -512,20 +512,25 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { { .compatible = "qcom,sm6115-smmu-500", .data = &qcom_smmu_500_impl0_data}, { .compatible = "qcom,sm6125-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm6350-smmu-v2", .data = &qcom_smmu_v2_data }, { .compatible = "qcom,sm6350-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm6375-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm8150-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, + /* +* Should come after the qcom,smmu-500 fallback so smmu-500 variants of +* adreno-smmu get qcom_adreno_smmu_500_impl: +*/ + { .compatible = "qcom,adreno-smmu", .data = &qcom_smmu_v2_data }, { } }; #ifdef CONFIG_ACPI static struct acpi_platform_list qcom_acpi_platlist[] = { { "LENOVO", "CB-01 ", 0x8180, ACPI_SIG_IORT, equal, "QCOM SMMU" }, { "QCOM ", "QCOMEDK2", 0x8180, ACPI_SIG_IORT, equal, "QCOM SMMU" }, { } }; #endif -- 2.39.2
[Freedreno] [PATCH 2/2] drm/msm: Be more shouty if per-process pgtables aren't working
From: Rob Clark Otherwise it is not always obvious if a dt or iommu change is causing us to fall back to global pgtable. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index 418e1e06cdde..1b7792d35860 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -224,24 +224,25 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev, struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent) { struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(parent->dev); struct msm_iommu *iommu = to_msm_iommu(parent); struct msm_iommu_pagetable *pagetable; const struct io_pgtable_cfg *ttbr1_cfg = NULL; struct io_pgtable_cfg ttbr0_cfg; int ret; + /* Get the pagetable configuration from the domain */ if (adreno_smmu->cookie) ttbr1_cfg = adreno_smmu->get_ttbr1_cfg(adreno_smmu->cookie); - if (!ttbr1_cfg) + if (WARN_ON_ONCE(!ttbr1_cfg)) return ERR_PTR(-ENODEV); pagetable = kzalloc(sizeof(*pagetable), GFP_KERNEL); if (!pagetable) return ERR_PTR(-ENOMEM); msm_mmu_init(&pagetable->base, parent->dev, &pagetable_funcs, MSM_MMU_IOMMU_PAGETABLE); /* Clone the TTBR1 cfg as starting point for TTBR0 cfg: */ -- 2.39.2
Re: [Freedreno] [PATCH 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
On 5/1/2023 2:27 PM, Dmitry Baryshkov wrote: On 02/05/2023 00:22, Abhinav Kumar wrote: On 5/1/2023 1:45 PM, Dmitry Baryshkov wrote: On 01/05/2023 22:58, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is no reason to split the dpu_encoder interface into separate _init() and _setup() phases. Merge them into a single function. I think the reason for having this split was to pass a valid encoder to the interface_modeset_init() and then do the rest of encoder initialization after modeset_init(). Looking at the current code, one issue i am seeing is that you will now initialize the dpu_encoder's msm_display_info along with dpu_encoder_init(). Most of it is fine but in the case of bonded_dsi(), I see an issue. The info.num_of_h_tiles++ happens after the modeset_init() of the second dsi but now it has been moved earlier. If for some reason, msm_dsi_modeset_init() fails for the second DSI, num_of_h_tiles will still be 2 now. If msm_dsi_modeset_init() fails, the function will err out and fail dpu_kms initialization. So it's not important, what is the value of num_h_tiles in this case. But I still feel the msm_display_info should be saved in the dpu encoder after the modeset_init() and not before. That way if some display interface specific init is done in the modeset_init(), we save the info after that. Up to now we have been using 'poll' model, e.g. we specifically asked for the DSC info from the DSI host rather than making msm_dsi set it. So far I don't see a good reason why this should be changed. Ok got it, so my concern came from the fact that we individually poll each feature today but lets say the number of features keeps growing we will have to combine them all into xxx_xxx_get_disp_info() which fills up all the fields of the display_info in one go. But yes, as long as we do that before calling dpu_encoder_init() it should be fine. Hence, Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH 2/7] drm/msm/dpu: drop dpu_encoder_early_unregister
On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is no need to clean up debugfs manually, it will be done by the DRM core on device deregistration. Signed-off-by: Dmitry Baryshkov --- There are two reasons to have the debugfs removed in the early_unregister: 1) Today, registration happens in late_register(), hence to balance the the call in _dpu_encoder_init_debugfs(), this one is present. 2) In drm_modeset_register_all(), if drm_connector_register_all() fails, it calls drm_encoder_unregister_all() first which calls early_unregister(). So to balance these out, dont we need to keep it? drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 32785cb1b079..8c45c949ec39 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2154,13 +2154,6 @@ static int dpu_encoder_late_register(struct drm_encoder *encoder) return _dpu_encoder_init_debugfs(encoder); } -static void dpu_encoder_early_unregister(struct drm_encoder *encoder) -{ - struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder); - - debugfs_remove_recursive(dpu_enc->debugfs_root); -} - static int dpu_encoder_virt_add_phys_encs( struct msm_display_info *disp_info, struct dpu_encoder_virt *dpu_enc, @@ -2374,7 +2367,6 @@ static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = { static const struct drm_encoder_funcs dpu_encoder_funcs = { .destroy = dpu_encoder_destroy, .late_register = dpu_encoder_late_register, - .early_unregister = dpu_encoder_early_unregister, }; struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
Re: [Freedreno] [PATCH 2/7] drm/msm/dpu: drop dpu_encoder_early_unregister
On Tue, 2 May 2023 at 23:45, Abhinav Kumar wrote: > > > > On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: > > There is no need to clean up debugfs manually, it will be done by the > > DRM core on device deregistration. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > There are two reasons to have the debugfs removed in the early_unregister: > > 1) Today, registration happens in late_register(), hence to balance the > the call in _dpu_encoder_init_debugfs(), this one is present. > > 2) In drm_modeset_register_all(), if drm_connector_register_all() fails, > it calls drm_encoder_unregister_all() first which calls early_unregister(). > > So to balance these out, dont we need to keep it? Please correct me if I'm wrong, drm_debugfs_cleanup() should take care of that. > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 > > 1 file changed, 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > index 32785cb1b079..8c45c949ec39 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > @@ -2154,13 +2154,6 @@ static int dpu_encoder_late_register(struct > > drm_encoder *encoder) > > return _dpu_encoder_init_debugfs(encoder); > > } > > > > -static void dpu_encoder_early_unregister(struct drm_encoder *encoder) > > -{ > > - struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder); > > - > > - debugfs_remove_recursive(dpu_enc->debugfs_root); > > -} > > - > > static int dpu_encoder_virt_add_phys_encs( > > struct msm_display_info *disp_info, > > struct dpu_encoder_virt *dpu_enc, > > @@ -2374,7 +2367,6 @@ static const struct drm_encoder_helper_funcs > > dpu_encoder_helper_funcs = { > > static const struct drm_encoder_funcs dpu_encoder_funcs = { > > .destroy = dpu_encoder_destroy, > > .late_register = dpu_encoder_late_register, > > - .early_unregister = dpu_encoder_early_unregister, > > }; > > > > struct drm_encoder *dpu_encoder_init(struct drm_device *dev, -- With best wishes Dmitry
Re: [Freedreno] [PATCH 2/7] drm/msm/dpu: drop dpu_encoder_early_unregister
On 5/2/2023 1:54 PM, Dmitry Baryshkov wrote: On Tue, 2 May 2023 at 23:45, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is no need to clean up debugfs manually, it will be done by the DRM core on device deregistration. Signed-off-by: Dmitry Baryshkov --- There are two reasons to have the debugfs removed in the early_unregister: 1) Today, registration happens in late_register(), hence to balance the the call in _dpu_encoder_init_debugfs(), this one is present. 2) In drm_modeset_register_all(), if drm_connector_register_all() fails, it calls drm_encoder_unregister_all() first which calls early_unregister(). So to balance these out, dont we need to keep it? Please correct me if I'm wrong, drm_debugfs_cleanup() should take care of that. Not from what I am seeing, drm_debugfs_cleanup() is getting called from the error handling path of drm_dev_register() and separately from drm_dev_unregister() but not from the error handling path of drm_modeset_register_all(). So that case will be unbalanced with this change. drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 32785cb1b079..8c45c949ec39 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2154,13 +2154,6 @@ static int dpu_encoder_late_register(struct drm_encoder *encoder) return _dpu_encoder_init_debugfs(encoder); } -static void dpu_encoder_early_unregister(struct drm_encoder *encoder) -{ - struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder); - - debugfs_remove_recursive(dpu_enc->debugfs_root); -} - static int dpu_encoder_virt_add_phys_encs( struct msm_display_info *disp_info, struct dpu_encoder_virt *dpu_enc, @@ -2374,7 +2367,6 @@ static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = { static const struct drm_encoder_funcs dpu_encoder_funcs = { .destroy = dpu_encoder_destroy, .late_register = dpu_encoder_late_register, - .early_unregister = dpu_encoder_early_unregister, }; struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
[Freedreno] [PATCH v3 0/7] add DSC 1.2 dpu supports
This series adds the DPU side changes to support DSC 1.2 encoder. This was validated with both DSI DSC 1.2 panel and DP DSC 1.2 monitor. The DSI and DP parts will be pushed later on top of this change. This seriel is rebase on [1], [2] and catalog fixes from [3]. [1]: https://patchwork.freedesktop.org/series/116851/ [2]: https://patchwork.freedesktop.org/series/116615/ [3]: https://patchwork.freedesktop.org/series/112332/ Abhinav Kumar (2): drm/msm/dpu: add dsc blocks for remaining chipsets in catalog drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets Kuogee Hsieh (5): drm/msm/dpu: add DPU_PINGPONG_DSC feature bit drm/msm/dpu: add DPU_PINGPONG_DSC bits into PP_BLK and PP_BLK_TE marcos drm/msm/dpu: add PINGPONG_NONE to disconnect DSC from PINGPONG drm/msm/dpu: add support for DSC encoder v1.2 engine drm/msm/dpu: separate DSC flush update out of interface drivers/gpu/drm/msm/Makefile | 1 + .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h| 19 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 8 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 26 +- .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 35 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 26 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h | 4 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +- .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h| 2 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 + .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 16 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 35 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 36 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 7 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 15 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 383 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c| 9 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +- 25 files changed, 649 insertions(+), 82 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[Freedreno] [PATCH v3 1/7] drm/msm/dpu: add dsc blocks for remaining chipsets in catalog
From: Abhinav Kumar There are some platforms has DSC blocks but it is not declared at catalog. For completeness, this patch adds DSC blocks for platforms which missed them. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++ drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h index 2b3ae84..17f821c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h @@ -126,6 +126,11 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = { DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), }; +static const struct dpu_dsc_cfg msm8998_dsc[] = { + DSC_BLK("dsc_0", DSC_0, 0x8, 0), + DSC_BLK("dsc_1", DSC_1, 0x80400, 0), +}; + static const struct dpu_dspp_cfg msm8998_dspp[] = { DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK, &msm8998_dspp_sblk), @@ -191,6 +196,8 @@ const struct dpu_mdss_cfg dpu_msm8998_cfg = { .dspp = msm8998_dspp, .pingpong_count = ARRAY_SIZE(msm8998_pp), .pingpong = msm8998_pp, + .dsc_count = ARRAY_SIZE(msm8998_dsc), + .dsc = msm8998_dsc, .intf_count = ARRAY_SIZE(msm8998_intf), .intf = msm8998_intf, .vbif_count = ARRAY_SIZE(msm8998_vbif), diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h index e3bdfe7..5bb9882 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h @@ -142,6 +142,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), }; +static const struct dpu_dsc_cfg sc8180x_dsc[] = { + DSC_BLK("dsc_0", DSC_0, 0x8, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)), +}; + static const struct dpu_intf_cfg sc8180x_intf[] = { INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -192,6 +201,8 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = { .mixer = sc8180x_lm, .pingpong_count = ARRAY_SIZE(sc8180x_pp), .pingpong = sc8180x_pp, + .dsc_count = ARRAY_SIZE(sc8180x_dsc), + .dsc = sc8180x_dsc, .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d), .merge_3d = sc8180x_merge_3d, .intf_count = ARRAY_SIZE(sc8180x_intf), -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[Freedreno] [PATCH v3 2/7] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit
Legacy DPU requires PP block to be involved during DSC setting up. This patch adds DDPU_PINGPONG_DSC feature bit to indicate that both dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_enable() pingpong ops functions are required to complete DSC data path set up and start DSC engine. Reported-by : Marijn Suijten Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++--- 2 files changed, 8 insertions(+), 3 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 71584cd..c07a6b6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -144,6 +144,7 @@ enum { * @DPU_PINGPONG_SPLIT PP block supports split fifo * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo * @DPU_PINGPONG_DITHER,Dither blocks + * @DPU_PINGPONG_DSC, PP ops functions required for DSC * @DPU_PINGPONG_MAX */ enum { @@ -152,6 +153,7 @@ enum { DPU_PINGPONG_SPLIT, DPU_PINGPONG_SLAVE, DPU_PINGPONG_DITHER, + DPU_PINGPONG_DSC, DPU_PINGPONG_MAX }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c index 3822e06..f255a04 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c @@ -264,9 +264,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c, c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config; c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr; c->ops.get_line_count = dpu_hw_pp_get_line_count; - c->ops.setup_dsc = dpu_hw_pp_setup_dsc; - c->ops.enable_dsc = dpu_hw_pp_dsc_enable; - c->ops.disable_dsc = dpu_hw_pp_dsc_disable; + + if (features & BIT(DPU_PINGPONG_DSC)) { + c->ops.setup_dsc = dpu_hw_pp_setup_dsc; + c->ops.enable_dsc = dpu_hw_pp_dsc_enable; + c->ops.disable_dsc = dpu_hw_pp_dsc_disable; + } if (test_bit(DPU_PINGPONG_DITHER, &features)) c->ops.setup_dither = dpu_hw_pp_setup_dither; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[Freedreno] [PATCH v3 3/7] drm/msm/dpu: add DPU_PINGPONG_DSC bits into PP_BLK and PP_BLK_TE marcos
At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate pingpong ops functions are required to complete DSC data path setup if this chipset has DSC hardware block presented. This patch add DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC hardware block presented. Signed-off-by: Kuogee Hsieh --- .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h| 12 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 8 +++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 26 ++ .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 24 ++-- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 26 ++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h | 4 ++-- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +- .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 8 +++ 9 files changed, 54 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h index 17f821c..b7cd746 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h @@ -112,16 +112,16 @@ static const struct dpu_lm_cfg msm8998_lm[] = { }; static const struct dpu_pingpong_cfg msm8998_pp[] = { - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, BIT(DPU_PINGPONG_DSC), 0, + sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, BIT(DPU_PINGPONG_DSC), 0, + sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, sdm845_pp_sblk, + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, sdm845_pp_sblk, + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h index ceca741..bd9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h @@ -110,16 +110,16 @@ static const struct dpu_lm_cfg sdm845_lm[] = { }; static const struct dpu_pingpong_cfg sdm845_pp[] = { - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk_te, + PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te, + PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, sdm845_pp_sblk, + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, sdm845_pp_sblk, + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h index 42b0e58..3a7dffa 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h @@ -128,24 +128,22 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = { }; static const struct dpu_pingpong_cfg sm8150_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x7, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK("pingpong_0", PINGPONG_0, 0x7, BIT(DPU_PINGPONG_DSC), MERGE_3D_0, + sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INT
[Freedreno] [PATCH v3 4/7] drm/msm/dpu: add PINGPONG_NONE to disconnect DSC from PINGPONG
During DSC setup, the crossbar mux need to be programmed to engage DSC to specified PINGPONG. Hence during tear down, the crossbar mux need to be reset to disengage DSC from PINGPONG. This patch add PINGPONG_NONE to serve as disable to reset crossbar mux. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 7 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 ++- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1dc5dbe..d9ad334 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1839,7 +1839,7 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc, hw_pp->ops.setup_dsc(hw_pp); if (hw_dsc->ops.dsc_bind_pingpong_blk) - hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, true, hw_pp->idx); + hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, hw_pp->idx); if (hw_pp->ops.enable_dsc) hw_pp->ops.enable_dsc(hw_pp); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c index 4a6bbcc..3e68d47 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c @@ -157,7 +157,6 @@ static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc, static void dpu_hw_dsc_bind_pingpong_blk( struct dpu_hw_dsc *hw_dsc, - bool enable, const enum dpu_pingpong pp) { struct dpu_hw_blk_reg_map *c = &hw_dsc->hw; @@ -166,13 +165,13 @@ static void dpu_hw_dsc_bind_pingpong_blk( dsc_ctl_offset = DSC_CTL(hw_dsc->idx); - if (enable) + if (pp) mux_cfg = (pp - PINGPONG_0) & 0x7; DRM_DEBUG_KMS("%s dsc:%d %s pp:%d\n", - enable ? "Binding" : "Unbinding", + pp ? "Binding" : "Unbinding", hw_dsc->idx - DSC_0, - enable ? "to" : "from", + pp ? "to" : "from", pp - PINGPONG_0); DPU_REG_WRITE(c, dsc_ctl_offset, mux_cfg); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h index 287ec5f..138080a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h @@ -44,7 +44,6 @@ struct dpu_hw_dsc_ops { struct drm_dsc_config *dsc); void (*dsc_bind_pingpong_blk)(struct dpu_hw_dsc *hw_dsc, - bool enable, enum dpu_pingpong pp); }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index 2d9192a..56826a9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -191,7 +191,8 @@ enum dpu_dsc { }; enum dpu_pingpong { - PINGPONG_0 = 1, + PINGPONG_NONE, + PINGPONG_0, PINGPONG_1, PINGPONG_2, PINGPONG_3, -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[Freedreno] [PATCH v3 5/7] drm/msm/dpu: add support for DSC encoder v1.2 engine
Add support for DSC 1.2 by providing the necessary hooks to program the DPU DSC 1.2 encoder. Changes in v3: -- fixed kernel test rebot report that "__iomem *off" is declared but not used at dpu_hw_dsc_config_1_2() -- unrolling thresh loops Reported-by: kernel test robot Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 34 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 383 + drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +- 5 files changed, 435 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index b814fc8..b9af5e4 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_hw_catalog.o \ disp/dpu1/dpu_hw_ctl.o \ disp/dpu1/dpu_hw_dsc.o \ + disp/dpu1/dpu_hw_dsc_1_2.o \ disp/dpu1/dpu_hw_interrupts.o \ disp/dpu1/dpu_hw_intf.o \ disp/dpu1/dpu_hw_lm.o \ 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 c07a6b6..b410a85 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ @@ -243,12 +243,20 @@ enum { }; /** - * DSC features + * DSC sub-blocks/features * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets *the pixel output from this DSC. + * @DPU_DSC_HW_REV_1_1DSC block supports dsc 1.1 only + * @DPU_DSC_HW_REV_1_2DSC block supports dsc 1.1 and 1.2 + * @DPU_DSC_NATIVE_422_EN Supports native422 and native420 encoding + * @DPU_DSC_MAX */ enum { DPU_DSC_OUTPUT_CTRL = 0x1, + DPU_DSC_HW_REV_1_1, + DPU_DSC_HW_REV_1_2, + DPU_DSC_NATIVE_422_EN, + DPU_DSC_MAX }; /** @@ -313,6 +321,14 @@ struct dpu_pp_blk { }; /** + * struct dpu_dsc_blk - DSC Encoder sub-blk information + * @info: HW register and features supported by this sub-blk + */ +struct dpu_dsc_blk { + DPU_HW_SUBBLK_INFO; +}; + +/** * enum dpu_qos_lut_usage - define QoS LUT use cases */ enum dpu_qos_lut_usage { @@ -461,6 +477,17 @@ struct dpu_pingpong_sub_blks { }; /** + * struct dpu_dsc_sub_blks - DSC sub-blks + * @enc: DSC encoder sub block + * @ctl: DSC controller sub block + * + */ +struct dpu_dsc_sub_blks { + struct dpu_dsc_blk enc; + struct dpu_dsc_blk ctl; +}; + +/** * dpu_clk_ctrl_type - Defines top level clock control signals */ enum dpu_clk_ctrl_type { @@ -614,10 +641,13 @@ struct dpu_merge_3d_cfg { * struct dpu_dsc_cfg - information of DSC blocks * @id enum identifying this block * @base register offset of this block + * @len: length of hardware block * @features bit mask identifying sub-blocks/features + * @sblk sub-blocks information */ struct dpu_dsc_cfg { DPU_HW_BLK_INFO; + const struct dpu_dsc_sub_blks *sblk; }; /** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h index 138080a..bdff74d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h @@ -1,5 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -/* Copyright (c) 2020-2022, Linaro Limited */ +/* + * Copyright (c) 2020-2022, Linaro Limited + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved + */ #ifndef _DPU_HW_DSC_H #define _DPU_HW_DSC_H @@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg, void __iomem *addr); /** + * dpu_hw_dsc_init_1_2 - initializes the v1.2 DSC hw driver block + * @cfg: DSC catalog entry for which driver object is required + * @addr: Mapped register io address of MDP + * Returns: Error code or allocated dpu_hw_dsc context + */ +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg, + void __iomem *addr); + +/** * dpu_hw_dsc_destroy - destroys dsc driver context * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c new file mode 100644 index ..617c7f3 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c @@ -0,0 +1,383 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020-2021, The Linux Foundat
[Freedreno] [PATCH v3 6/7] drm/msm/dpu: separate DSC flush update out of interface
Current DSC flush update is piggyback inside dpu_hw_ctl_intf_cfg_v1(). This patch separates DSC flush away from dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1() to handle both per DSC engine and DSC flush bits at same time to make it consistent with the location of flush programming of other dpu sub blocks. Signed-off-by: Kuogee Hsieh Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index d9ad334..71db23e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1823,12 +1823,18 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc, return DIV_ROUND_UP(total_pixels, dsc->slice_width); } -static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc, +static void dpu_encoder_dsc_pipe_cfg(struct dpu_encoder_virt *dpu_enc, +struct dpu_hw_dsc *hw_dsc, struct dpu_hw_pingpong *hw_pp, struct drm_dsc_config *dsc, u32 common_mode, u32 initial_lines) { + struct dpu_encoder_phys *cur_master = dpu_enc->cur_master; + struct dpu_hw_ctl *ctl; + + ctl = cur_master->hw_ctl; + if (hw_dsc->ops.dsc_config) hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode, initial_lines); @@ -1843,6 +1849,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc, if (hw_pp->ops.enable_dsc) hw_pp->ops.enable_dsc(hw_pp); + + if (ctl->ops.update_pending_flush_dsc) + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); } static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, @@ -1887,7 +1896,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) - dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines); + dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc, + dsc_common_mode, initial_lines); } void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index 4f7cfa9..832a6a7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx) CTL_DSPP_n_FLUSH(dspp - DSPP_0), ctx->pending_dspp_flush_mask[dspp - DSPP_0]); } + + if (ctx->pending_flush_mask & BIT(DSC_IDX)) + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, + ctx->pending_dsc_flush_mask); + DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask); } @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx, ctx->pending_flush_mask |= BIT(MERGE_3D_IDX); } +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx, + enum dpu_dsc dsc_num) +{ + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0); + ctx->pending_flush_mask |= BIT(DSC_IDX); +} + static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx, enum dpu_dspp dspp, u32 dspp_sub_blk) { @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx, if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features))) mode_sel = CTL_DEFAULT_GROUP_ID << 28; - if (cfg->dsc) - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc); - if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD) mode_sel |= BIT(17); @@ -524,10 +533,8 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx, if (cfg->merge_3d) DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE, BIT(cfg->merge_3d - MERGE_3D_0)); - if (cfg->dsc) { - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX); + if (cfg->dsc) DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc); - } } static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx, @@ -630,6 +637,9 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops, ops->update_pending_flush_merge_3d = dpu_hw_ctl_update_pending_flush_merge_3d_v1; ops->update_pending_flush_wb = dpu_hw_ctl_update
[Freedreno] [PATCH v3 7/7] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
From: Abhinav Kumar Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and feature flag information. Each display compression engine (DCE) contains dual hard slice DSC encoders so both share same base address but with its own different sub block address. Signed-off-by: Abhinav Kumar Signed-off-by: Kuogee Hsieh --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 +++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++ .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 +++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 27 -- 6 files changed, 90 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h index 4f6a965..f98c2a5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x5), }; +/* + * NOTE: Each display compression engine (DCE) contains dual hard + * slice DSC encoders so both share same base address but with + * its own different sub block address. + */ +static const struct dpu_dsc_cfg sm8350_dsc[] = { + DSC_BLK_1_2("dce_0", DSC_0, 0x8, 0x100, 0, dsc_sblk_0), + DSC_BLK_1_2("dce_0", DSC_1, 0x8, 0x100, 0, dsc_sblk_1), + DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), + DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1), +}; + static const struct dpu_intf_cfg sm8350_intf[] = { INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -205,6 +217,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = { .dspp = sm8350_dspp, .pingpong_count = ARRAY_SIZE(sm8350_pp), .pingpong = sm8350_pp, + .dsc = sm8350_dsc, + .dsc_count = ARRAY_SIZE(sm8350_dsc), .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d), .merge_3d = sm8350_merge_3d, .intf_count = ARRAY_SIZE(sm8350_intf), diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 6b2c7ea..3fd0498a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = { PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1), }; +/* NOTE: sc7280 only has one dsc hard slice encoder */ +static const struct dpu_dsc_cfg sc7280_dsc[] = { + DSC_BLK_1_2("dce_0", DSC_0, 0x8, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), +}; + static const struct dpu_intf_cfg sc7280_intf[] = { INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -142,6 +147,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mixer = sc7280_lm, .pingpong_count = ARRAY_SIZE(sc7280_pp), .pingpong = sc7280_pp, + .dsc_count = ARRAY_SIZE(sc7280_dsc), + .dsc = sc7280_dsc, .intf_count = ARRAY_SIZE(sc7280_intf), .intf = sc7280_intf, .vbif_count = ARRAY_SIZE(sdm845_vbif), diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h index 706d0f1..ce583eb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x5), }; +/* + * NOTE: Each display compression engine (DCE) contains dual hard + * slice DSC encoders so both share same base address but with + * its own different sub block address. + */ +static const struct dpu_dsc_cfg sc8280xp_dsc[] = { + DSC_BLK_1_2("dce_0", DSC_0, 0x8, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), + DSC_BLK_1_2("dce_0", DSC_1, 0x8, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1), + DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), + DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1), + DSC_BLK_1_2("dce_2", DSC_4, 0x82000, 0x100, 0, dsc_sblk_0), + DSC_BLK_1_2("dce_2", DSC_5, 0x82000, 0x100, 0, dsc_sblk_1), +}; + /* TODO: INTF 3,
Re: [Freedreno] [PATCH 2/7] drm/msm/dpu: drop dpu_encoder_early_unregister
On 02/05/2023 23:59, Abhinav Kumar wrote: On 5/2/2023 1:54 PM, Dmitry Baryshkov wrote: On Tue, 2 May 2023 at 23:45, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is no need to clean up debugfs manually, it will be done by the DRM core on device deregistration. Signed-off-by: Dmitry Baryshkov --- There are two reasons to have the debugfs removed in the early_unregister: 1) Today, registration happens in late_register(), hence to balance the the call in _dpu_encoder_init_debugfs(), this one is present. 2) In drm_modeset_register_all(), if drm_connector_register_all() fails, it calls drm_encoder_unregister_all() first which calls early_unregister(). So to balance these out, dont we need to keep it? Please correct me if I'm wrong, drm_debugfs_cleanup() should take care of that. Not from what I am seeing, drm_debugfs_cleanup() is getting called from the error handling path of drm_dev_register() and separately from drm_dev_unregister() but not from the error handling path of drm_modeset_register_all(). So that case will be unbalanced with this change. But for the practical reasons the drm_modeset_register_all() can not fail. But I see your point. I'll check why drm_dev_register() doesn't respect an error return from drm_modeset_register_all(). Until that point we can drop this patch. drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 32785cb1b079..8c45c949ec39 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2154,13 +2154,6 @@ static int dpu_encoder_late_register(struct drm_encoder *encoder) return _dpu_encoder_init_debugfs(encoder); } -static void dpu_encoder_early_unregister(struct drm_encoder *encoder) -{ - struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder); - - debugfs_remove_recursive(dpu_enc->debugfs_root); -} - static int dpu_encoder_virt_add_phys_encs( struct msm_display_info *disp_info, struct dpu_encoder_virt *dpu_enc, @@ -2374,7 +2367,6 @@ static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = { static const struct drm_encoder_funcs dpu_encoder_funcs = { .destroy = dpu_encoder_destroy, .late_register = dpu_encoder_late_register, - .early_unregister = dpu_encoder_early_unregister, }; struct drm_encoder *dpu_encoder_init(struct drm_device *dev, -- With best wishes Dmitry
Re: [Freedreno] [PATCH v3 0/7] add DSC 1.2 dpu supports
On 03/05/2023 00:02, Kuogee Hsieh wrote: This series adds the DPU side changes to support DSC 1.2 encoder. This was validated with both DSI DSC 1.2 panel and DP DSC 1.2 monitor. The DSI and DP parts will be pushed later on top of this change. This seriel is rebase on [1], [2] and catalog fixes from [3]. Changelog? -- With best wishes Dmitry
Re: [Freedreno] [PATCH v3 1/7] drm/msm/dpu: add dsc blocks for remaining chipsets in catalog
On 03/05/2023 00:02, Kuogee Hsieh wrote: From: Abhinav Kumar There are some platforms has DSC blocks but it is not declared at catalog. For completeness, this patch adds DSC blocks for platforms which missed them. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++ drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++ 2 files changed, 18 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [Freedreno] [PATCH v3 2/7] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit
On 03/05/2023 00:02, Kuogee Hsieh wrote: Legacy DPU requires PP block to be involved during DSC setting up. This patch adds DDPU_PINGPONG_DSC feature bit to indicate that both DPU_PINGPONG_DSC dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_enable() pingpong ops functions are required to complete DSC data path set up and start DSC engine. Nit: as these ops were already present, I'd say that the lack of the flag means that these operations are not supported and must not be called for DSC setup/teardown. Nevertheless: Reviewed-by: Dmitry Baryshkov Reported-by : Marijn Suijten Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++--- 2 files changed, 8 insertions(+), 3 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 71584cd..c07a6b6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -144,6 +144,7 @@ enum { * @DPU_PINGPONG_SPLIT PP block supports split fifo * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo * @DPU_PINGPONG_DITHER,Dither blocks + * @DPU_PINGPONG_DSC, PP ops functions required for DSC * @DPU_PINGPONG_MAX */ enum { @@ -152,6 +153,7 @@ enum { DPU_PINGPONG_SPLIT, DPU_PINGPONG_SLAVE, DPU_PINGPONG_DITHER, + DPU_PINGPONG_DSC, DPU_PINGPONG_MAX }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c index 3822e06..f255a04 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c @@ -264,9 +264,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c, c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config; c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr; c->ops.get_line_count = dpu_hw_pp_get_line_count; - c->ops.setup_dsc = dpu_hw_pp_setup_dsc; - c->ops.enable_dsc = dpu_hw_pp_dsc_enable; - c->ops.disable_dsc = dpu_hw_pp_dsc_disable; + + if (features & BIT(DPU_PINGPONG_DSC)) { + c->ops.setup_dsc = dpu_hw_pp_setup_dsc; + c->ops.enable_dsc = dpu_hw_pp_dsc_enable; + c->ops.disable_dsc = dpu_hw_pp_dsc_disable; + } if (test_bit(DPU_PINGPONG_DITHER, &features)) c->ops.setup_dither = dpu_hw_pp_setup_dither; -- With best wishes Dmitry
Re: [Freedreno] [PATCH v3 4/7] drm/msm/dpu: add PINGPONG_NONE to disconnect DSC from PINGPONG
On 03/05/2023 00:02, Kuogee Hsieh wrote: During DSC setup, the crossbar mux need to be programmed to engage DSC to specified PINGPONG. Hence during tear down, the crossbar mux need to be reset to disengage DSC from PINGPONG. This patch add PINGPONG_NONE to serve as disable to reset crossbar mux. Signed-off-by: Kuogee Hsieh Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 7 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 ++- 4 files changed, 6 insertions(+), 7 deletions(-) -- With best wishes Dmitry
Re: [Freedreno] [PATCH 3/7] drm/msm/dpu: separate common function to init physical encoder
On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: Move common DPU physical encoder initialization code to the new function dpu_encoder_phys_init(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 31 +-- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 3 ++ .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 19 +++- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 20 +++- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 19 +++- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 8c45c949ec39..c60dce5861e2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2303,8 +2303,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, for (i = 0; i < dpu_enc->num_phys_encs; i++) { struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; - atomic_set(&phys->vsync_cnt, 0); - atomic_set(&phys->underrun_cnt, 0); if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx); @@ -2505,3 +2503,32 @@ unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc) return dpu_enc->dsc_mask; } + +int dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc, + struct dpu_enc_phys_init_params *p) +{ + int i; + + phys_enc->hw_mdptop = p->dpu_kms->hw_mdp; + phys_enc->intf_idx = p->intf_idx; + phys_enc->wb_idx = p->wb_idx; + phys_enc->parent = p->parent; + phys_enc->dpu_kms = p->dpu_kms; + phys_enc->split_role = p->split_role; + phys_enc->enc_spinlock = p->enc_spinlock; + phys_enc->enable_state = DPU_ENC_DISABLED; + + for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++) + phys_enc->irq[i] = -EINVAL; + + atomic_set(&phys_enc->vblank_refcount, 0); + atomic_set(&phys_enc->pending_kickoff_cnt, 0); + atomic_set(&phys_enc->pending_ctlstart_cnt, 0); + + atomic_set(&phys_enc->vsync_cnt, 0); + atomic_set(&phys_enc->underrun_cnt, 0); + + init_waitqueue_head(&phys_enc->pending_kickoff_wq); + + return 0; +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index 1d434b22180d..7019870215c0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -405,4 +405,7 @@ void dpu_encoder_frame_done_callback( struct drm_encoder *drm_enc, struct dpu_encoder_phys *ready_phys, u32 event); +int dpu_encoder_phys_init(struct dpu_encoder_phys *phys, + struct dpu_enc_phys_init_params *p); + #endif /* __dpu_encoder_phys_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 74470d068622..ce86b9ef6bf1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -759,7 +759,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( { struct dpu_encoder_phys *phys_enc = NULL; struct dpu_encoder_phys_cmd *cmd_enc = NULL; - int i, ret = 0; + int ret = 0; DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0); @@ -770,25 +770,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( return ERR_PTR(ret); } phys_enc = &cmd_enc->base; - phys_enc->hw_mdptop = p->dpu_kms->hw_mdp; - phys_enc->intf_idx = p->intf_idx; + + ret = dpu_encoder_phys_init(phys_enc, p); + if (ret) + return ERR_PTR(ret); dpu_encoder_phys_init() seems to always return 0, so we can make that void and drop ret and return here? dpu_encoder_phys_cmd_init_ops(&phys_enc->ops); - phys_enc->parent = p->parent; - phys_enc->dpu_kms = p->dpu_kms; - phys_enc->split_role = p->split_role; phys_enc->intf_mode = INTF_MODE_CMD; - phys_enc->enc_spinlock = p->enc_spinlock; cmd_enc->stream_sel = 0; - phys_enc->enable_state = DPU_ENC_DISABLED; - for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++) - phys_enc->irq[i] = -EINVAL; - atomic_set(&phys_enc->vblank_refcount, 0); - atomic_set(&phys_enc->pending_kickoff_cnt, 0); - atomic_set(&phys_enc->pending_ctlstart_cnt, 0); atomic_set(&cmd_enc->pending_vblank_cnt, 0); - init_waitqueue_head(&phys_enc->pending_kickoff_wq); init_waitqueue_head(&cmd_enc->pending_vblank_wq); DPU_DEBUG_CMDENC(cmd_enc, "created\n"); 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 3a374292f311..aca3849621e2 100644 --- a/drive
Re: [Freedreno] [PATCH 3/7] drm/msm/dpu: separate common function to init physical encoder
On 03/05/2023 00:33, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: Move common DPU physical encoder initialization code to the new function dpu_encoder_phys_init(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 31 +-- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 3 ++ .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 19 +++- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 20 +++- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 19 +++- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 8c45c949ec39..c60dce5861e2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2303,8 +2303,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, for (i = 0; i < dpu_enc->num_phys_encs; i++) { struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; - atomic_set(&phys->vsync_cnt, 0); - atomic_set(&phys->underrun_cnt, 0); if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx); @@ -2505,3 +2503,32 @@ unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc) return dpu_enc->dsc_mask; } + +int dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc, + struct dpu_enc_phys_init_params *p) +{ + int i; + + phys_enc->hw_mdptop = p->dpu_kms->hw_mdp; + phys_enc->intf_idx = p->intf_idx; + phys_enc->wb_idx = p->wb_idx; + phys_enc->parent = p->parent; + phys_enc->dpu_kms = p->dpu_kms; + phys_enc->split_role = p->split_role; + phys_enc->enc_spinlock = p->enc_spinlock; + phys_enc->enable_state = DPU_ENC_DISABLED; + + for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++) + phys_enc->irq[i] = -EINVAL; + + atomic_set(&phys_enc->vblank_refcount, 0); + atomic_set(&phys_enc->pending_kickoff_cnt, 0); + atomic_set(&phys_enc->pending_ctlstart_cnt, 0); + + atomic_set(&phys_enc->vsync_cnt, 0); + atomic_set(&phys_enc->underrun_cnt, 0); + + init_waitqueue_head(&phys_enc->pending_kickoff_wq); + + return 0; +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index 1d434b22180d..7019870215c0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -405,4 +405,7 @@ void dpu_encoder_frame_done_callback( struct drm_encoder *drm_enc, struct dpu_encoder_phys *ready_phys, u32 event); +int dpu_encoder_phys_init(struct dpu_encoder_phys *phys, + struct dpu_enc_phys_init_params *p); + #endif /* __dpu_encoder_phys_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 74470d068622..ce86b9ef6bf1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -759,7 +759,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( { struct dpu_encoder_phys *phys_enc = NULL; struct dpu_encoder_phys_cmd *cmd_enc = NULL; - int i, ret = 0; + int ret = 0; DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0); @@ -770,25 +770,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( return ERR_PTR(ret); } phys_enc = &cmd_enc->base; - phys_enc->hw_mdptop = p->dpu_kms->hw_mdp; - phys_enc->intf_idx = p->intf_idx; + + ret = dpu_encoder_phys_init(phys_enc, p); + if (ret) + return ERR_PTR(ret); dpu_encoder_phys_init() seems to always return 0, so we can make that void and drop ret and return here? I had in mind a possible error from INTF_n/WB_n -> hw_intf/hw_wb lookup, but at the end I got rid of that. So, yes, why not. dpu_encoder_phys_cmd_init_ops(&phys_enc->ops); - phys_enc->parent = p->parent; - phys_enc->dpu_kms = p->dpu_kms; - phys_enc->split_role = p->split_role; phys_enc->intf_mode = INTF_MODE_CMD; - phys_enc->enc_spinlock = p->enc_spinlock; cmd_enc->stream_sel = 0; - phys_enc->enable_state = DPU_ENC_DISABLED; - for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++) - phys_enc->irq[i] = -EINVAL; - atomic_set(&phys_enc->vblank_refcount, 0); - atomic_set(&phys_enc->pending_kickoff_cnt, 0); - atomic_set(&phys_enc->pending_ctlstart_cnt, 0); atomic_set(&cmd_enc->pending_vblank_cnt, 0); - init_waitqueue_head(&phys_enc->pending_kickoff_wq); init_waitqueue_head(&cmd_enc->pending_vblank_wq); DPU_DEBUG_CMDENC(cmd_enc, "created\n"); 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 3a374292f311..aca3849621e2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encode
Re: [Freedreno] [PATCH v3 5/7] drm/msm/dpu: add support for DSC encoder v1.2 engine
On 03/05/2023 00:03, Kuogee Hsieh wrote: Add support for DSC 1.2 by providing the necessary hooks to program the DPU DSC 1.2 encoder. Changes in v3: -- fixed kernel test rebot report that "__iomem *off" is declared but not used at dpu_hw_dsc_config_1_2() -- unrolling thresh loops Reported-by: kernel test robot Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 34 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 383 + drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +- 5 files changed, 435 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index b814fc8..b9af5e4 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_hw_catalog.o \ disp/dpu1/dpu_hw_ctl.o \ disp/dpu1/dpu_hw_dsc.o \ + disp/dpu1/dpu_hw_dsc_1_2.o \ disp/dpu1/dpu_hw_interrupts.o \ disp/dpu1/dpu_hw_intf.o \ disp/dpu1/dpu_hw_lm.o \ 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 c07a6b6..b410a85 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ @@ -243,12 +243,20 @@ enum { }; /** - * DSC features + * DSC sub-blocks/features * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets *the pixel output from this DSC. + * @DPU_DSC_HW_REV_1_1DSC block supports dsc 1.1 only Do we need a separate feature flag for this? IIRC, 1.1 is a common baseline. Do you plan to reuse the same interface for other compression blocks (e.g. FBC or VDC-M, if they were to be supported at some point)? + * @DPU_DSC_HW_REV_1_2DSC block supports dsc 1.1 and 1.2 + * @DPU_DSC_NATIVE_422_EN Supports native422 and native420 encoding + * @DPU_DSC_MAX */ enum { DPU_DSC_OUTPUT_CTRL = 0x1, + DPU_DSC_HW_REV_1_1, + DPU_DSC_HW_REV_1_2, + DPU_DSC_NATIVE_422_EN, + DPU_DSC_MAX }; /** @@ -313,6 +321,14 @@ struct dpu_pp_blk { }; /** + * struct dpu_dsc_blk - DSC Encoder sub-blk information + * @info: HW register and features supported by this sub-blk + */ +struct dpu_dsc_blk { + DPU_HW_SUBBLK_INFO; +}; + +/** * enum dpu_qos_lut_usage - define QoS LUT use cases */ enum dpu_qos_lut_usage { @@ -461,6 +477,17 @@ struct dpu_pingpong_sub_blks { }; /** + * struct dpu_dsc_sub_blks - DSC sub-blks + * @enc: DSC encoder sub block + * @ctl: DSC controller sub block + * + */ +struct dpu_dsc_sub_blks { + struct dpu_dsc_blk enc; + struct dpu_dsc_blk ctl; +}; + +/** * dpu_clk_ctrl_type - Defines top level clock control signals */ enum dpu_clk_ctrl_type { @@ -614,10 +641,13 @@ struct dpu_merge_3d_cfg { * struct dpu_dsc_cfg - information of DSC blocks * @id enum identifying this block * @base register offset of this block + * @len: length of hardware block * @features bit mask identifying sub-blocks/features + * @sblk sub-blocks information */ struct dpu_dsc_cfg { DPU_HW_BLK_INFO; + const struct dpu_dsc_sub_blks *sblk; }; /** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h index 138080a..bdff74d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h @@ -1,5 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -/* Copyright (c) 2020-2022, Linaro Limited */ +/* + * Copyright (c) 2020-2022, Linaro Limited + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved + */ #ifndef _DPU_HW_DSC_H #define _DPU_HW_DSC_H @@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg, void __iomem *addr); /** + * dpu_hw_dsc_init_1_2 - initializes the v1.2 DSC hw driver block + * @cfg: DSC catalog entry for which driver object is required + * @addr: Mapped register io address of MDP + * Returns: Error code or allocated dpu_hw_dsc context + */ +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg, + void __iomem *addr); + +/** * dpu_hw_dsc_destroy - destroys dsc driver context * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init */ diff --git
Re: [Freedreno] [PATCH v3 7/7] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
On 03/05/2023 00:03, Kuogee Hsieh wrote: From: Abhinav Kumar Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and feature flag information. Each display compression engine (DCE) contains dual hard slice DSC encoders so both share same base address but with its own different sub block address. Signed-off-by: Abhinav Kumar Signed-off-by: Kuogee Hsieh Reviewed-by: Dmitry Baryshkov Minor question below. --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 +++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++ .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 +++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 27 -- 6 files changed, 90 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h index 4f6a965..f98c2a5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x5), }; +/* + * NOTE: Each display compression engine (DCE) contains dual hard + * slice DSC encoders so both share same base address but with + * its own different sub block address. + */ +static const struct dpu_dsc_cfg sm8350_dsc[] = { + DSC_BLK_1_2("dce_0", DSC_0, 0x8, 0x100, 0, dsc_sblk_0), + DSC_BLK_1_2("dce_0", DSC_1, 0x8, 0x100, 0, dsc_sblk_1), + DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), + DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1), +}; + static const struct dpu_intf_cfg sm8350_intf[] = { INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -205,6 +217,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = { .dspp = sm8350_dspp, .pingpong_count = ARRAY_SIZE(sm8350_pp), .pingpong = sm8350_pp, + .dsc = sm8350_dsc, + .dsc_count = ARRAY_SIZE(sm8350_dsc), .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d), .merge_3d = sm8350_merge_3d, .intf_count = ARRAY_SIZE(sm8350_intf), diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 6b2c7ea..3fd0498a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = { PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1), }; +/* NOTE: sc7280 only has one dsc hard slice encoder */ +static const struct dpu_dsc_cfg sc7280_dsc[] = { + DSC_BLK_1_2("dce_0", DSC_0, 0x8, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), +}; + static const struct dpu_intf_cfg sc7280_intf[] = { INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -142,6 +147,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mixer = sc7280_lm, .pingpong_count = ARRAY_SIZE(sc7280_pp), .pingpong = sc7280_pp, + .dsc_count = ARRAY_SIZE(sc7280_dsc), + .dsc = sc7280_dsc, .intf_count = ARRAY_SIZE(sc7280_intf), .intf = sc7280_intf, .vbif_count = ARRAY_SIZE(sdm845_vbif), diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h index 706d0f1..ce583eb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x5), }; +/* + * NOTE: Each display compression engine (DCE) contains dual hard + * slice DSC encoders so both share same base address but with + * its own different sub block address. + */ +static const struct dpu_dsc_cfg sc8280xp_dsc[] = { + DSC_BLK_1_2("dce_0", DSC_0, 0x8, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), + DSC_BLK_1_2("dce_0", DSC_1, 0x8, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1), + DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0), + DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1), + DSC_BLK_1_2("dce_2", DSC_4, 0x82
Re: [Freedreno] [PATCH v3 3/7] drm/msm/dpu: add DPU_PINGPONG_DSC bits into PP_BLK and PP_BLK_TE marcos
On 03/05/2023 00:02, Kuogee Hsieh wrote: At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate pingpong ops functions are required to complete DSC data path setup if this chipset has DSC hardware block presented. This patch add DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC hardware block presented. Signed-off-by: Kuogee Hsieh --- .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h| 12 +- .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 8 +++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 26 ++ .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 24 ++-- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 26 ++ .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h | 4 ++-- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +- .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 8 +++ 9 files changed, 54 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h index 17f821c..b7cd746 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h @@ -112,16 +112,16 @@ static const struct dpu_lm_cfg msm8998_lm[] = { }; static const struct dpu_pingpong_cfg msm8998_pp[] = { - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, BIT(DPU_PINGPONG_DSC), 0, + sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, BIT(DPU_PINGPONG_DSC), 0, + sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, sdm845_pp_sblk, + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, sdm845_pp_sblk, + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), Just to doublecheck: why don't we have DPU_PINGPONG_DSC for PP_3/_4? We do have them on sdm845. Is it because we should not use DSC with thos PINGPONG blocks? }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h index ceca741..bd9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h @@ -110,16 +110,16 @@ static const struct dpu_lm_cfg sdm845_lm[] = { }; static const struct dpu_pingpong_cfg sdm845_pp[] = { - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk_te, + PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te, + PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, sdm845_pp_sblk, + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, sdm845_pp_sblk, + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, BIT(DPU_PINGPONG_DSC), 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), [skipped the rest, looks good to me] -- With best wishes Dmitry
Re: [Freedreno] [PATCH 4/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and use them to get the instance index. Signed-off-by: Dmitry Baryshkov --- From whatever I can see, this will not affect functionality of intf or wb and cleans it up well , so I am fine with this. Hence, Reviewed-by: Abhinav Kumar One minor comment/question. @@ -761,7 +761,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( struct dpu_encoder_phys_cmd *cmd_enc = NULL; int ret = 0; - DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0); Was it intentional to drop the index in this log? + DPU_DEBUG("intf\n"); cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL);
Re: [Freedreno] [PATCH 4/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
On Wed, 3 May 2023 at 02:04, Abhinav Kumar wrote: > > > > On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: > > Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and > > struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and > > use them to get the instance index. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > From whatever I can see, this will not affect functionality of intf or > wb and cleans it up well , so I am fine with this. Hence, > > Reviewed-by: Abhinav Kumar > > One minor comment/question. > > > > > @@ -761,7 +761,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( > > struct dpu_encoder_phys_cmd *cmd_enc = NULL; > > int ret = 0; > > > > - DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0); > > Was it intentional to drop the index in this log? We don't have p->intf_idx at this point. I think we can use p->hw_intf->idx instead, I'll fix that for v2. > > > + DPU_DEBUG("intf\n"); > > > > cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL); -- With best wishes Dmitry
Re: [Freedreno] [PATCH 4/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
On 5/2/2023 4:15 PM, Dmitry Baryshkov wrote: On Wed, 3 May 2023 at 02:04, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and use them to get the instance index. Signed-off-by: Dmitry Baryshkov --- From whatever I can see, this will not affect functionality of intf or wb and cleans it up well , so I am fine with this. Hence, Reviewed-by: Abhinav Kumar One minor comment/question. @@ -761,7 +761,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( struct dpu_encoder_phys_cmd *cmd_enc = NULL; int ret = 0; - DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0); Was it intentional to drop the index in this log? We don't have p->intf_idx at this point. I think we can use p->hw_intf->idx instead, I'll fix that for v2. Yes, I was aware that. In all other logs, intf_idx was replaced with hw_intf->idx except this one. So I was not sure if it was intentional or just removed accidentally. + DPU_DEBUG("intf\n"); cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL);
Re: [Freedreno] [PATCH v3 5/7] drm/msm/dpu: add support for DSC encoder v1.2 engine
On 5/2/2023 2:38 PM, Dmitry Baryshkov wrote: On 03/05/2023 00:03, Kuogee Hsieh wrote: Add support for DSC 1.2 by providing the necessary hooks to program the DPU DSC 1.2 encoder. Changes in v3: -- fixed kernel test rebot report that "__iomem *off" is declared but not used at dpu_hw_dsc_config_1_2() -- unrolling thresh loops Reported-by: kernel test robot Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 34 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 383 + drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +- 5 files changed, 435 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index b814fc8..b9af5e4 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_hw_catalog.o \ disp/dpu1/dpu_hw_ctl.o \ disp/dpu1/dpu_hw_dsc.o \ + disp/dpu1/dpu_hw_dsc_1_2.o \ disp/dpu1/dpu_hw_interrupts.o \ disp/dpu1/dpu_hw_intf.o \ disp/dpu1/dpu_hw_lm.o \ 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 c07a6b6..b410a85 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ @@ -243,12 +243,20 @@ enum { }; /** - * DSC features + * DSC sub-blocks/features * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets * the pixel output from this DSC. + * @DPU_DSC_HW_REV_1_1 DSC block supports dsc 1.1 only Do we need a separate feature flag for this? IIRC, 1.1 is a common baseline. Do you plan to reuse the same interface for other compression blocks (e.g. FBC or VDC-M, if they were to be supported at some point)? + * @DPU_DSC_HW_REV_1_2 DSC block supports dsc 1.1 and 1.2 + * @DPU_DSC_NATIVE_422_EN Supports native422 and native420 encoding + * @DPU_DSC_MAX */ enum { DPU_DSC_OUTPUT_CTRL = 0x1, + DPU_DSC_HW_REV_1_1, + DPU_DSC_HW_REV_1_2, + DPU_DSC_NATIVE_422_EN, + DPU_DSC_MAX }; /** @@ -313,6 +321,14 @@ struct dpu_pp_blk { }; /** + * struct dpu_dsc_blk - DSC Encoder sub-blk information + * @info: HW register and features supported by this sub-blk + */ +struct dpu_dsc_blk { + DPU_HW_SUBBLK_INFO; +}; + +/** * enum dpu_qos_lut_usage - define QoS LUT use cases */ enum dpu_qos_lut_usage { @@ -461,6 +477,17 @@ struct dpu_pingpong_sub_blks { }; /** + * struct dpu_dsc_sub_blks - DSC sub-blks + * @enc: DSC encoder sub block + * @ctl: DSC controller sub block + * + */ +struct dpu_dsc_sub_blks { + struct dpu_dsc_blk enc; + struct dpu_dsc_blk ctl; +}; + +/** * dpu_clk_ctrl_type - Defines top level clock control signals */ enum dpu_clk_ctrl_type { @@ -614,10 +641,13 @@ struct dpu_merge_3d_cfg { * struct dpu_dsc_cfg - information of DSC blocks * @id enum identifying this block * @base register offset of this block + * @len: length of hardware block * @features bit mask identifying sub-blocks/features + * @sblk sub-blocks information */ struct dpu_dsc_cfg { DPU_HW_BLK_INFO; + const struct dpu_dsc_sub_blks *sblk; }; /** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h index 138080a..bdff74d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h @@ -1,5 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -/* Copyright (c) 2020-2022, Linaro Limited */ +/* + * Copyright (c) 2020-2022, Linaro Limited + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved + */ #ifndef _DPU_HW_DSC_H #define _DPU_HW_DSC_H @@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg, void __iomem *addr); /** + * dpu_hw_dsc_init_1_2 - initializes the v1.2 DSC hw driver block + * @cfg: DSC catalog entry for which driver object is required + * @addr: Mapped register io address of MDP + * Returns: Error code or allocated dpu_hw_dsc context + */ +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg, + void __iomem *addr); + +/** * dpu_hw_dsc_destroy - destroys dsc driver context * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init */ diff --git a/drivers/
Re: [Freedreno] [PATCH 5/7] drm/msm/dpu: inline dpu_encoder_get_wb()
On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: The function dpu_encoder_get_wb() returns controller_id if the corresponding WB is present in the catalog. We can inline this function and rely on dpu_rm_get_wb() returning NULL for indices for which the WB is not present on the device. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--- 1 file changed, 2 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 4c85cbb030e4..507ff3f88c67 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1277,22 +1277,6 @@ static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, return INTF_MAX; } -static enum dpu_wb dpu_encoder_get_wb(const struct dpu_mdss_cfg *catalog, - enum dpu_intf_type type, u32 controller_id) -{ - int i = 0; - - if (type != INTF_WB) - return WB_MAX; - - for (i = 0; i < catalog->wb_count; i++) { - if (catalog->wb[i].id == controller_id) - return catalog->wb[i].id; - } - - return WB_MAX; -} - void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, struct dpu_encoder_phys *phy_enc) { @@ -2261,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, */ u32 controller_id = disp_info->h_tile_instance[i]; enum dpu_intf intf_idx; - enum dpu_wb wb_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2279,14 +2262,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, disp_info->intf_type, controller_id); - wb_idx = dpu_encoder_get_wb(dpu_kms->catalog, - disp_info->intf_type, controller_id); - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); - if (wb_idx >= WB_0 && wb_idx < WB_MAX) - phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, wb_idx); + if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) + phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); From what I see, with https://patchwork.freedesktop.org/patch/534776/?series=117146&rev=1 we are dropping those checks from the RM too, so we are going to rely totally on entering the values correctly in catalog from now on? if (!phys_params.hw_intf && !phys_params.hw_wb) { DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i);
Re: [Freedreno] [PATCH 4/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
On 03/05/2023 02:19, Abhinav Kumar wrote: On 5/2/2023 4:15 PM, Dmitry Baryshkov wrote: On Wed, 3 May 2023 at 02:04, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and use them to get the instance index. Signed-off-by: Dmitry Baryshkov --- From whatever I can see, this will not affect functionality of intf or wb and cleans it up well , so I am fine with this. Hence, Reviewed-by: Abhinav Kumar One minor comment/question. @@ -761,7 +761,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( struct dpu_encoder_phys_cmd *cmd_enc = NULL; int ret = 0; - DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0); Was it intentional to drop the index in this log? We don't have p->intf_idx at this point. I think we can use p->hw_intf->idx instead, I'll fix that for v2. Yes, I was aware that. In all other logs, intf_idx was replaced with hw_intf->idx except this one. So I was not sure if it was intentional or just removed accidentally. Most likely it was a leftover from the interim patch where I didn't have p->hw_intf. Initially I kept the code which manually set the phys->hw_intf (and hw_wb), but then it was just easier to pass that through the params and let the caller set it. + DPU_DEBUG("intf\n"); cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL); -- With best wishes Dmitry
Re: [Freedreno] [PATCH 5/7] drm/msm/dpu: inline dpu_encoder_get_wb()
On 03/05/2023 02:51, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: The function dpu_encoder_get_wb() returns controller_id if the corresponding WB is present in the catalog. We can inline this function and rely on dpu_rm_get_wb() returning NULL for indices for which the WB is not present on the device. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--- 1 file changed, 2 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 4c85cbb030e4..507ff3f88c67 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1277,22 +1277,6 @@ static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, return INTF_MAX; } -static enum dpu_wb dpu_encoder_get_wb(const struct dpu_mdss_cfg *catalog, - enum dpu_intf_type type, u32 controller_id) -{ - int i = 0; - - if (type != INTF_WB) - return WB_MAX; - - for (i = 0; i < catalog->wb_count; i++) { - if (catalog->wb[i].id == controller_id) - return catalog->wb[i].id; - } - - return WB_MAX; -} - void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, struct dpu_encoder_phys *phy_enc) { @@ -2261,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, */ u32 controller_id = disp_info->h_tile_instance[i]; enum dpu_intf intf_idx; - enum dpu_wb wb_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2279,14 +2262,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, disp_info->intf_type, controller_id); - wb_idx = dpu_encoder_get_wb(dpu_kms->catalog, - disp_info->intf_type, controller_id); - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); - if (wb_idx >= WB_0 && wb_idx < WB_MAX) - phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, wb_idx); + if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) + phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); From what I see, with https://patchwork.freedesktop.org/patch/534776/?series=117146&rev=1 we are dropping those checks from the RM too, so we are going to rely totally on entering the values correctly in catalog from now on? Yes. I see no reason to mistrust the kernel data itself. if (!phys_params.hw_intf && !phys_params.hw_wb) { DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i); -- With best wishes Dmitry
Re: [Freedreno] [PATCH 6/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is little sense to get intf index just to call dpu_rm_get_intf() on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 507ff3f88c67..b35e92c658ad 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1259,22 +1259,23 @@ static void dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc, mutex_unlock(&dpu_enc->enc_lock); } -static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, +static struct dpu_hw_intf *dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, + struct dpu_rm *dpu_rm, enum dpu_intf_type type, u32 controller_id) { int i = 0; if (type == INTF_WB) - return INTF_MAX; + return NULL; for (i = 0; i < catalog->intf_count; i++) { if (catalog->intf[i].type == type && catalog->intf[i].controller_id == controller_id) { - return catalog->intf[i].id; + return dpu_rm_get_intf(dpu_rm, catalog->intf[i].id); } Why has the for loop been retained in this function but not for writeback? is there any difference? Doesnt looks like there needs to be. } - return INTF_MAX; + return NULL; } void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, @@ -2244,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, * h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right */ u32 controller_id = disp_info->h_tile_instance[i]; - enum dpu_intf intf_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2258,12 +2258,9 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n", i, controller_id, phys_params.split_role); - intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, - disp_info->intf_type, - controller_id); - - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) - phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); + phys_params.hw_intf = dpu_encoder_get_intf(dpu_kms->catalog, &dpu_kms->rm, + disp_info->intf_type, + controller_id); if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); @@ -2287,7 +2284,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n"); break; } - unnecessary change? } mutex_unlock(&dpu_enc->enc_lock);
Re: [Freedreno] [PATCH 5/7] drm/msm/dpu: inline dpu_encoder_get_wb()
On 5/2/2023 4:54 PM, Dmitry Baryshkov wrote: On 03/05/2023 02:51, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: The function dpu_encoder_get_wb() returns controller_id if the corresponding WB is present in the catalog. We can inline this function and rely on dpu_rm_get_wb() returning NULL for indices for which the WB is not present on the device. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--- 1 file changed, 2 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 4c85cbb030e4..507ff3f88c67 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1277,22 +1277,6 @@ static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, return INTF_MAX; } -static enum dpu_wb dpu_encoder_get_wb(const struct dpu_mdss_cfg *catalog, - enum dpu_intf_type type, u32 controller_id) -{ - int i = 0; - - if (type != INTF_WB) - return WB_MAX; - - for (i = 0; i < catalog->wb_count; i++) { - if (catalog->wb[i].id == controller_id) - return catalog->wb[i].id; - } - - return WB_MAX; -} - void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, struct dpu_encoder_phys *phy_enc) { @@ -2261,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, */ u32 controller_id = disp_info->h_tile_instance[i]; enum dpu_intf intf_idx; - enum dpu_wb wb_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2279,14 +2262,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, disp_info->intf_type, controller_id); - wb_idx = dpu_encoder_get_wb(dpu_kms->catalog, - disp_info->intf_type, controller_id); - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); - if (wb_idx >= WB_0 && wb_idx < WB_MAX) - phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, wb_idx); + if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) + phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); From what I see, with https://patchwork.freedesktop.org/patch/534776/?series=117146&rev=1 we are dropping those checks from the RM too, so we are going to rely totally on entering the values correctly in catalog from now on? Yes. I see no reason to mistrust the kernel data itself. Alright, if thats the overall plan, this change itself is fine. Reviewed-by: Abhinav Kumar if (!phys_params.hw_intf && !phys_params.hw_wb) { DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i);
Re: [Freedreno] [PATCH 6/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
On 03/05/2023 02:57, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is little sense to get intf index just to call dpu_rm_get_intf() on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 507ff3f88c67..b35e92c658ad 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1259,22 +1259,23 @@ static void dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc, mutex_unlock(&dpu_enc->enc_lock); } -static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, +static struct dpu_hw_intf *dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, + struct dpu_rm *dpu_rm, enum dpu_intf_type type, u32 controller_id) { int i = 0; if (type == INTF_WB) - return INTF_MAX; + return NULL; for (i = 0; i < catalog->intf_count; i++) { if (catalog->intf[i].type == type && catalog->intf[i].controller_id == controller_id) { - return catalog->intf[i].id; + return dpu_rm_get_intf(dpu_rm, catalog->intf[i].id); } Why has the for loop been retained in this function but not for writeback? is there any difference? Doesnt looks like there needs to be. For writeback we always return controller_id (WB_2). For interfaces we have to map type+controller_id to the INTF instance. } - return INTF_MAX; + return NULL; } void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, @@ -2244,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, * h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right */ u32 controller_id = disp_info->h_tile_instance[i]; - enum dpu_intf intf_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2258,12 +2258,9 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n", i, controller_id, phys_params.split_role); - intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, - disp_info->intf_type, - controller_id); - - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) - phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); + phys_params.hw_intf = dpu_encoder_get_intf(dpu_kms->catalog, &dpu_kms->rm, + disp_info->intf_type, + controller_id); if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); @@ -2287,7 +2284,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n"); break; } - unnecessary change? ack, it sneaked in. I'll drop it for v2. } mutex_unlock(&dpu_enc->enc_lock); -- With best wishes Dmitry
Re: [Freedreno] [PATCH 5/7] drm/msm/dpu: inline dpu_encoder_get_wb()
On 03/05/2023 02:58, Abhinav Kumar wrote: On 5/2/2023 4:54 PM, Dmitry Baryshkov wrote: On 03/05/2023 02:51, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: The function dpu_encoder_get_wb() returns controller_id if the corresponding WB is present in the catalog. We can inline this function and rely on dpu_rm_get_wb() returning NULL for indices for which the WB is not present on the device. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--- 1 file changed, 2 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 4c85cbb030e4..507ff3f88c67 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1277,22 +1277,6 @@ static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, return INTF_MAX; } -static enum dpu_wb dpu_encoder_get_wb(const struct dpu_mdss_cfg *catalog, - enum dpu_intf_type type, u32 controller_id) -{ - int i = 0; - - if (type != INTF_WB) - return WB_MAX; - - for (i = 0; i < catalog->wb_count; i++) { - if (catalog->wb[i].id == controller_id) - return catalog->wb[i].id; - } - - return WB_MAX; -} - void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, struct dpu_encoder_phys *phy_enc) { @@ -2261,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, */ u32 controller_id = disp_info->h_tile_instance[i]; enum dpu_intf intf_idx; - enum dpu_wb wb_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2279,14 +2262,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, disp_info->intf_type, controller_id); - wb_idx = dpu_encoder_get_wb(dpu_kms->catalog, - disp_info->intf_type, controller_id); - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); - if (wb_idx >= WB_0 && wb_idx < WB_MAX) - phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, wb_idx); + if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) + phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); From what I see, with https://patchwork.freedesktop.org/patch/534776/?series=117146&rev=1 we are dropping those checks from the RM too, so we are going to rely totally on entering the values correctly in catalog from now on? Yes. I see no reason to mistrust the kernel data itself. Alright, if thats the overall plan, this change itself is fine. Yes, I think this is what we discussed some time ago for UBWC and QSEED programming. Reviewed-by: Abhinav Kumar if (!phys_params.hw_intf && !phys_params.hw_wb) { DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i); -- With best wishes Dmitry
Re: [Freedreno] [PATCH 7/7] drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set
On 01/05/2023 02:57, Dmitry Baryshkov wrote: The atomic_mode_set() callback only sets the phys_enc's IRQ data. As the INTF and WB are statically allocated to each encoder/phys_enc, drop the atomic_mode_set callback and set the IRQs during encoder init. Signed-off-by: Dmitry Baryshkov Please ignore this for now, I'd like to take another look on my own. I didn't test the CMD panels and they are going to break with this change. --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 -- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 5 - .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 20 +-- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 ++-- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 11 +- 5 files changed, 8 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index b35e92c658ad..509b4fc7dbc5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1106,8 +1106,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); phys->cached_mode = crtc_state->adjusted_mode; - if (phys->ops.atomic_mode_set) - phys->ops.atomic_mode_set(phys, crtc_state, conn_state); } } 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 1c096d9390d0..67c4b4e0975d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -68,8 +68,6 @@ struct dpu_encoder_phys; * @is_master:Whether this phys_enc is the current master *encoder. Can be switched at enable time. Based *on split_role and current mode (CMD/VID). - * @atomic_mode_set: DRM Call. Set a DRM mode. - * This likely caches the mode, for use at enable. * @enable: DRM Call. Enable a DRM mode. * @disable: DRM Call. Disable mode. * @atomic_check: DRM Call. Atomic check new DRM state. @@ -97,9 +95,6 @@ struct dpu_encoder_phys_ops { struct dentry *debugfs_root); void (*prepare_commit)(struct dpu_encoder_phys *encoder); bool (*is_master)(struct dpu_encoder_phys *encoder); - void (*atomic_mode_set)(struct dpu_encoder_phys *encoder, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state); void (*enable)(struct dpu_encoder_phys *encoder); void (*disable)(struct dpu_encoder_phys *encoder); int (*atomic_check)(struct dpu_encoder_phys *encoder, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 781290f17714..3ad03465acfe 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -139,20 +139,6 @@ static void dpu_encoder_phys_cmd_underrun_irq(void *arg, int irq_idx) dpu_encoder_underrun_callback(phys_enc->parent, phys_enc); } -static void dpu_encoder_phys_cmd_atomic_mode_set( - struct dpu_encoder_phys *phys_enc, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state) -{ - phys_enc->irq[INTR_IDX_CTL_START] = phys_enc->hw_ctl->caps->intr_start; - - phys_enc->irq[INTR_IDX_PINGPONG] = phys_enc->hw_pp->caps->intr_done; - - phys_enc->irq[INTR_IDX_RDPTR] = phys_enc->hw_pp->caps->intr_rdptr; - - phys_enc->irq[INTR_IDX_UNDERRUN] = phys_enc->hw_intf->cap->intr_underrun; -} - static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( struct dpu_encoder_phys *phys_enc) { @@ -736,7 +722,6 @@ static void dpu_encoder_phys_cmd_init_ops( struct dpu_encoder_phys_ops *ops) { ops->is_master = dpu_encoder_phys_cmd_is_master; - ops->atomic_mode_set = dpu_encoder_phys_cmd_atomic_mode_set; ops->enable = dpu_encoder_phys_cmd_enable; ops->disable = dpu_encoder_phys_cmd_disable; ops->destroy = dpu_encoder_phys_cmd_destroy; @@ -777,6 +762,11 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( dpu_encoder_phys_cmd_init_ops(&phys_enc->ops); phys_enc->intf_mode = INTF_MODE_CMD; + phys_enc->irq[INTR_IDX_CTL_START] = phys_enc->hw_ctl->caps->intr_start; + phys_enc->irq[INTR_IDX_PINGPONG] = phys_enc->hw_pp->caps->intr_done; + phys_enc->irq[INTR_IDX_RDPTR] = phys_enc->hw_pp->caps->intr_rdptr; + phys_enc->irq[INTR_IDX_UNDERRUN] = phys_enc->hw_intf->cap->intr_underrun; + cmd_enc->stream_sel = 0; atomic_set(&cmd_enc->pending_vblank_cnt, 0); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_
Re: [Freedreno] [PATCH 6/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
On 5/2/2023 4:58 PM, Dmitry Baryshkov wrote: On 03/05/2023 02:57, Abhinav Kumar wrote: On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote: There is little sense to get intf index just to call dpu_rm_get_intf() on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 507ff3f88c67..b35e92c658ad 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1259,22 +1259,23 @@ static void dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc, mutex_unlock(&dpu_enc->enc_lock); } -static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, +static struct dpu_hw_intf *dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog, + struct dpu_rm *dpu_rm, enum dpu_intf_type type, u32 controller_id) { int i = 0; if (type == INTF_WB) - return INTF_MAX; + return NULL; for (i = 0; i < catalog->intf_count; i++) { if (catalog->intf[i].type == type && catalog->intf[i].controller_id == controller_id) { - return catalog->intf[i].id; + return dpu_rm_get_intf(dpu_rm, catalog->intf[i].id); } Why has the for loop been retained in this function but not for writeback? is there any difference? Doesnt looks like there needs to be. For writeback we always return controller_id (WB_2). For interfaces we have to map type+controller_id to the INTF instance. Ah correct, got it now. With that minor comment fixed from below, Reviewed-by: Abhinav Kumar } - return INTF_MAX; + return NULL; } void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, @@ -2244,7 +2245,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, * h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right */ u32 controller_id = disp_info->h_tile_instance[i]; - enum dpu_intf intf_idx; if (disp_info->num_of_h_tiles > 1) { if (i == 0) @@ -2258,12 +2258,9 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n", i, controller_id, phys_params.split_role); - intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, - disp_info->intf_type, - controller_id); - - if (intf_idx >= INTF_0 && intf_idx < INTF_MAX) - phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx); + phys_params.hw_intf = dpu_encoder_get_intf(dpu_kms->catalog, &dpu_kms->rm, + disp_info->intf_type, + controller_id); if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX) phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id); @@ -2287,7 +2284,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n"); break; } - unnecessary change? ack, it sneaked in. I'll drop it for v2. } mutex_unlock(&dpu_enc->enc_lock);
[Freedreno] [PATCH 1/4] drm/msm/dsi: Adjust pclk rate for compression
Divide the pclk rate by the compression ratio when DSC is enabled Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi_host.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 43a5ec33eee8..35c69dbe5f6f 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -561,7 +561,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi) +static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, + struct drm_dsc_config *dsc, bool is_bonded_dsi) { unsigned long pclk_rate; @@ -576,6 +577,11 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool if (is_bonded_dsi) pclk_rate /= 2; + /* If DSC is enabled, divide pclk by compression ratio */ + if (dsc) + pclk_rate = DIV_ROUND_UP(pclk_rate, + dsc->bits_per_component * 3 / msm_dsc_get_bpp_int(dsc)); + return pclk_rate; } @@ -585,7 +591,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d struct msm_dsi_host *msm_host = to_msm_dsi_host(host); u8 lanes = msm_host->lanes; u32 bpp = dsi_get_bpp(msm_host->format); - unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi); + unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi); u64 pclk_bpp = (u64)pclk_rate * bpp; if (lanes == 0) { @@ -604,7 +610,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) { - msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi); + msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi); msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi, msm_host->mode); @@ -634,7 +640,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi) dsi_calc_pclk(msm_host, is_bonded_dsi); - pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp; + pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp; do_div(pclk_bpp, 8); msm_host->src_clk_rate = pclk_bpp; -- 2.40.1
[Freedreno] [PATCH 2/4] drm/msm/dsi: Fix compressed word count calculation
Currently, word count is calculated using slice_count. This is incorrect as downstream uses slice per packet, which is different from slice_count. Slice count represents the number of soft slices per interface, and its value will not always match that of slice per packet. For example, it is possible to have cases where there are multiple soft slices per interface but the panel specifies only one slice per packet. Thus, use the default value of one slice per packet and remove slice_count from the word count calculation. Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count") Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi_host.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 35c69dbe5f6f..b0d448ffb078 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -996,7 +996,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) if (!msm_host->dsc) wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1; else - wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1; + /* +* When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1. +* Currently, the driver only supports default value of slice_per_packet = 1 +* +* TODO: Expand drm_panel struct to hold slice_per_packet info +* and adjust DSC math to account for slice_per_packet. +*/ + wc = msm_host->dsc->slice_chunk_size + 1; dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL, DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) | -- 2.40.1
[Freedreno] [PATCH 3/4] drm/msm/dpu: Add has_data_compress to dpu_caps
Add data_compress feature to DPU HW catalog. In DPU 7.x and later, there is a DATA_COMPRESS register that must be set within the DPU INTF block for DSC to work. As core_rev (and related macros) was removed from the dpu_kms struct, the most straightforward way to indicate the presence of this register would be to have a flag in dpu_caps. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 1 + drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 1 + drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 1 + drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 6 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h index f98c2a5b0e87..4160a35ff20f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h @@ -15,6 +15,7 @@ static const struct dpu_caps sm8350_dpu_caps = { .has_dim_layer = true, .has_idle_pc = true, .has_3d_merge = true, + .has_data_compress = true, .max_linewidth = 4096, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 3fd0498ab420..23230841a0d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -13,6 +13,7 @@ static const struct dpu_caps sc7280_dpu_caps = { .qseed_type = DPU_SSPP_SCALER_QSEED4, .has_dim_layer = true, .has_idle_pc = true, + .has_data_compress = true, .max_linewidth = 2400, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h index ce583eb14b06..c990406e4bca 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h @@ -15,6 +15,7 @@ static const struct dpu_caps sc8280xp_dpu_caps = { .has_dim_layer = true, .has_idle_pc = true, .has_3d_merge = true, + .has_data_compress = true, .max_linewidth = 5120, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h index 3950e7b946a5..7094640e2fbf 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h @@ -15,6 +15,7 @@ static const struct dpu_caps sm8450_dpu_caps = { .has_dim_layer = true, .has_idle_pc = true, .has_3d_merge = true, + .has_data_compress = true, .max_linewidth = 5120, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h index 1b3f5424aea8..970049559e02 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h @@ -15,6 +15,7 @@ static const struct dpu_caps sm8550_dpu_caps = { .has_dim_layer = true, .has_idle_pc = true, .has_3d_merge = true, + .has_data_compress = true, .max_linewidth = 5120, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; 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 b410a85c109c..c5bbd4ad6da8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -380,6 +380,7 @@ struct dpu_rotation_cfg { * @has_dim_layer dim layer feature status * @has_idle_pcindicate if idle power collapse feature is supported * @has_3d_merge indicate if 3D merge is supported + * @has_data_compress indicate if data compression is supported * @max_linewidth max linewidth for sspp * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes * @max_hdeci_exp max horizontal decimation supported (max is 2^value) @@ -393,6 +394,7 @@ struct dpu_caps { bool has_dim_layer; bool has_idle_pc; bool has_3d_merge; + bool has_data_compress; /* SSPP limits */ u32 max_linewidth; u32 pixel_ram_size; -- 2.40.1
[Freedreno] [PATCH 4/4] drm/msm/dpu: Enable compression for command mode
Add a dpu_hw_intf op to enable data compression. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 7 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 ++ 3 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 74470d068622..4321a1aba17f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -72,6 +72,10 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg( phys_enc->hw_intf, true, phys_enc->hw_pp->idx); + + if (phys_enc->dpu_kms->catalog->caps->has_data_compress && + phys_enc->hw_intf->ops.enable_compression) + phys_enc->hw_intf->ops.enable_compression(phys_enc->hw_intf); } static void dpu_encoder_phys_cmd_pp_tx_done_irq(void *arg, int irq_idx) 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 671048a78801..4ce7ffdd7a05 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -64,10 +64,16 @@ #define INTF_CFG2_DATABUS_WIDENBIT(0) #define INTF_CFG2_DATA_HCTL_EN BIT(4) +#define INTF_CFG2_DCE_DATA_COMPRESSBIT(12) #define INTF_MISR_CTRL 0x180 #define INTF_MISR_SIGNATURE0x184 +static inline void dpu_hw_intf_enable_compression(struct dpu_hw_intf *ctx) +{ + DPU_REG_WRITE(&ctx->hw, INTF_CONFIG2, INTF_CFG2_DCE_DATA_COMPRESS); +} + static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, const struct intf_timing_params *p, const struct dpu_format *fmt) @@ -325,6 +331,7 @@ static void _setup_intf_ops(struct dpu_hw_intf_ops *ops, ops->bind_pingpong_blk = dpu_hw_intf_bind_pingpong_blk; ops->setup_misr = dpu_hw_intf_setup_misr; ops->collect_misr = dpu_hw_intf_collect_misr; + ops->enable_compression = dpu_hw_intf_enable_compression; } struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h index 102c4f0e812b..99528c735368 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h @@ -60,6 +60,7 @@ struct intf_status { * feed pixels to this interface * @setup_misr: enable/disable MISR * @collect_misr: read MISR signature + * @enable_compression: Enable data compression */ struct dpu_hw_intf_ops { void (*setup_timing_gen)(struct dpu_hw_intf *intf, @@ -82,6 +83,7 @@ struct dpu_hw_intf_ops { const enum dpu_pingpong pp); void (*setup_misr)(struct dpu_hw_intf *intf, bool enable, u32 frame_count); int (*collect_misr)(struct dpu_hw_intf *intf, u32 *misr_value); + void (*enable_compression)(struct dpu_hw_intf *intf); }; struct dpu_hw_intf { -- 2.40.1
[Freedreno] [PATCH 0/4] Add DSC v1.2 Support for DSI
This is a series of changes for DSI to enable support for DSC v1.2. This includes: 1) Dividing the pclk_rate by the compression ratio when DSC is enabled 2) Fixing the word count calculation for DSC 3) Setting the DATA_COMPRESS bit when DSC is enabled With these changes (and the dependency below), DSC v1.2 should work over DSI. Depends-on: "add DSC 1.2 dpu supports" [1] [1] https://patchwork.freedesktop.org/series/116789/ Signed-off-by: Jessica Zhang --- Jessica Zhang (4): drm/msm/dsi: Adjust pclk rate for compression drm/msm/dsi: Fix compressed word count calculation drm/msm/dpu: Add has_data_compress to dpu_caps drm/msm/dpu: Enable compression for command mode .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 1 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 1 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 1 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 1 + .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 7 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 ++ drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- 10 files changed, 38 insertions(+), 5 deletions(-) --- base-commit: a4e4b4826fe482d5f63a0232bc6588da2edfa45b change-id: 20230405-add-dsc-support-fe130ba49841 Best regards, -- Jessica Zhang