Re: [Intel-gfx] [PATCH] drm/i915: Make sure dsm_size has correct granularity
On 24.01.2023 16:00, Nirmoy Das wrote: DSM granularity is 1MB so make sure we stick to that. Cc: Matthew Auld Suggested-by: Lucas De Marchi Signed-off-by: Nirmoy Das --- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index 90a967374b1a..0c027263f09e 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -909,7 +909,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type, dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK; if (WARN_ON(lmem_size < dsm_base)) return ERR_PTR(-ENODEV); - dsm_size = lmem_size - dsm_base; + dsm_size = ALIGN_DOWN(lmem_size - dsm_base, 1 * SZ_1M); 1st seen "1 * SZ_1M", usually it is just SZ_1M, up to you. Reviewed-by: Andrzej Hajda Regards Andrzej } io_size = dsm_size;
Re: [PATCH v6 4/4] drm/msm: stop storing the array of CRTCs in struct msm_drm_private
On Wed, 25 Jan 2023 at 04:14, Abhinav Kumar wrote:
>
>
>
> On 6/17/2022 4:33 PM, Dmitry Baryshkov wrote:
> > The array of CRTC in the struct msm_drm_private duplicates a list of
> > CRTCs in the drm_device. Drop it and use the existing list for CRTC
> > enumeration.
> >
> > Reported-by: kernel test robot
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 2 +-
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
> > drivers/gpu/drm/msm/msm_drv.c| 44 +---
> > drivers/gpu/drm/msm/msm_drv.h| 3 +-
> > 5 files changed, 27 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index e23e2552e802..e79f0a8817ac 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -806,7 +806,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
> > *dpu_kms)
> > ret = PTR_ERR(crtc);
> > return ret;
> > }
> > - priv->crtcs[priv->num_crtcs++] = crtc;
> > + priv->num_crtcs++;
> > }
> >
> > /* All CRTCs are compatible with all encoders */
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > index fb48c8c19ec3..7449c1693e45 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > @@ -337,7 +337,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
> > goto fail;
> > }
> >
> > - priv->crtcs[priv->num_crtcs++] = crtc;
> > + priv->num_crtcs++;
> > }
> >
> > /*
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > index 3d5621a68f85..36808990f840 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > @@ -497,7 +497,7 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
> > DRM_DEV_ERROR(dev->dev, "failed to construct crtc %d
> > (%d)\n", i, ret);
> > goto fail;
> > }
> > - priv->crtcs[priv->num_crtcs++] = crtc;
> > + priv->num_crtcs++;
> > }
> >
> > /*
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 1aab6bf86278..567e77dae43b 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -149,7 +149,7 @@ static void msm_irq_uninstall(struct drm_device *dev)
> >
> > struct msm_vblank_work {
> > struct work_struct work;
> > - int crtc_id;
> > + struct drm_crtc *crtc;
> > bool enable;
> > struct msm_drm_private *priv;
> > };
> > @@ -162,15 +162,15 @@ static void vblank_ctrl_worker(struct work_struct
> > *work)
> > struct msm_kms *kms = priv->kms;
> >
>
> Is there any chance of vbl_work->crtc becoming NULL before this gets
> executed?
No. The worker is created in vblank_ctrl_queue_work. The
vbl_work->crtc is filled at the time of creation.
> So do we need to protect this like
>
> if (vbl_work->enable && vbl_work->crtc)
>
> Because the layers below this dont seem to have NULL protection.
>
>
> > if (vbl_work->enable)
> > - kms->funcs->enable_vblank(kms,
> > priv->crtcs[vbl_work->crtc_id]);
> > + kms->funcs->enable_vblank(kms, vbl_work->crtc);
> > else
> > - kms->funcs->disable_vblank(kms,
> > priv->crtcs[vbl_work->crtc_id]);
> > + kms->funcs->disable_vblank(kms, vbl_work->crtc);
> >
> > kfree(vbl_work);
> > }
> >
> > static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
> > - int crtc_id, bool enable)
> > + struct drm_crtc *crtc, bool enable)
> > {
> > struct msm_vblank_work *vbl_work;
> >
> > @@ -180,7 +180,7 @@ static int vblank_ctrl_queue_work(struct
> > msm_drm_private *priv,
> >
> > INIT_WORK(_work->work, vblank_ctrl_worker);
> >
> > - vbl_work->crtc_id = crtc_id;
> > + vbl_work->crtc = crtc;
> > vbl_work->enable = enable;
> > vbl_work->priv = priv;
> >
> > @@ -354,7 +354,8 @@ static int msm_drm_init(struct device *dev, const
> > struct drm_driver *drv)
> > struct msm_drm_private *priv = dev_get_drvdata(dev);
> > struct drm_device *ddev;
> > struct msm_kms *kms;
> > - int ret, i;
> > + struct drm_crtc *crtc;
> > + int ret;
> >
> > if (drm_firmware_drivers_only())
> > return -ENODEV;
> > @@ -427,20 +428,23 @@ static int msm_drm_init(struct device *dev, const
> > struct drm_driver *drv)
> > ddev->mode_config.funcs = _config_funcs;
> > ddev->mode_config.helper_private = _config_helper_funcs;
> >
> > -
Re: [PATCH v3 2/2] dt-bindings: display: simple-framebuffer: Document physical width and height properties
On 24/01/2023 23:41, Rayyan Ansari wrote: > Document the optional width-mm and height-mm simple-framebuffer > properties. As pointed in previous discussion - you should have panel node and take the properties from it. The physical dimensions are not properties of framebuffer device. These are properties of panel. Best regards, Krzysztof
Re: [PATCH v2 2/2] dt-bindings: display: simple-framebuffer: Document physical width and height properties
On 24/01/2023 23:19, Rayyan Ansari wrote: > On 23/01/2023 17:53, Rob Herring wrote: >> On Sun, Jan 22, 2023 at 05:25:38PM +, Rayyan Ansari wrote: >>> On 22/01/2023 15:36, Rob Herring wrote: On Sat, Jan 21, 2023 at 9:36 AM Rayyan Ansari wrote: > Why do you need this change? The 'simple-framebuffer' contains data on how the bootloader configured the display. The bootloader doesn't configure the display size, so this information doesn't belong here. The information should already be in the panel node, so also no point in duplicating it here. > Signed-off-by: Rayyan Ansari > --- >.../devicetree/bindings/display/simple-framebuffer.yaml | 8 >1 file changed, 8 insertions(+) >>> >>> Hi Rob, >>> >>> There is the usecase that Hans has mentioned, but I have also mentioned >>> another usecase previously. >>> >>> Adding the width-mm and height-mm properties allows user interfaces such as >>> Phosh (https://puri.sm/posts/phosh-overview/) to scale correctly to the >>> screen. In my case, a panel node is not available and the aforementioned >>> interface is in fact running on the SimpleDRM driver (which binds to the >>> simple-framebuffer device). >> >> Why is the panel node not available? Why not add it? Presumably it is >> not there because you aren't (yet) using the simple-panel driver (and >> others that would need). But presumably you will eventually as I'd >> imagine turning the screen off and back on might be a desired feature. > > It requires more than using the simple-panel driver: first the SoC side > display hardware needs to be brought up, then a panel driver that > implements the proper DCS initialisation sequence needs to be written > (which is currently not fully known). > >> >> So why add a temporary DT property that's tied to your *current* kernel? > >> The DT should not be tightly coupled to the kernel. > > I'm not sure what you mean by it being "tightly coupled" to the kernel. It means that you used current Linux driver support (or lack) for some hardware as an argument for bindings. If you add later the driver, the bindings should be changed? Answer is: not. Bindings should be independent of Linux drivers, thus whatever kernel is missing now, is not an argument in favor of this property. Best regards, Krzysztof
Re: [RESEND PATCH v11 13/18] drm: exynos: dsi: Add Exynos based host irq hooks
On Wed, Jan 25, 2023 at 2:54 AM Jagan Teki wrote: > > On Wed, Jan 25, 2023 at 2:54 AM Jagan Teki wrote: > > > > On Wed, Jan 25, 2023 at 2:42 AM Marek Vasut wrote: > > > > > > On 1/24/23 22:01, Jagan Teki wrote: > > > > On Wed, Jan 25, 2023 at 2:18 AM Marek Vasut wrote: > > > >> > > > >> On 1/23/23 16:12, Jagan Teki wrote: > > > >>> Enable and disable of te_gpio's are Exynos platform specific > > > >>> irq handling, so add the exynos based irq operations and hook > > > >>> them for exynos plat_data. > > > >> > > > >> If this is just an optional generic GPIO IRQ, why not keep it in the > > > >> core code ? TE (tearing enable?) should be available on MX8M too. > > > > > > > > So far the discussion (since from initial versions) with Marek > > > > Szyprowski, seems to be available in Exynos. So, I keep it separate > > > > from the DSIM core. > > > > > > Isn't TE a generic GPIO IRQ ? If so, it is available also on i.MX8M . > > I will check this. In order to use TE_GPIO we need te handler implementation, right now Exynos CRTC DRM drivers have implementation for this. That is the main reason to keep the TE_GPIO handling in exynos, maybe if we handle that generically then it is a viable option to move TE_GPIO to the DSIM core. Jagan.
Re: Amdgpu module is references even after unbinding the vtcon
Am 24.01.23 um 17:15 schrieb Thomas Zimmermann: Hi Am 24.01.23 um 15:12 schrieb Christian König: Hi Thomas, we ran into a problem with the general fbcon/fbdev implementation and though that you might have some idea. What happens is the following: 1. We load amdgpu and get our normal fbcon. 2. fbcon allocates a dump BO as backing store for the console. 3. GDM/X/Applications start, new framebuffers are created BOs imported, exported etc... 4. Somehow X or GDM iterated over all the framebuffer objects the kernels knows about and export them as DMA-buf. 5. Application/X/GDM are stopped, handles closed, framebuffers released etc... 6. We unbind vtcon. At this point the amdgpu module usually has a reference count of 0 and can be unloaded, but since GDM/X/Whoever iterated over all the known framebuffers and exported them as DMA-buf (for whatever reason idk) we now still have an exported DMA-buf and with it a reference to the module. Any idea how we could prevent that? No real clue, sorry. But does it change if you use a shadow buffer on top of the amdgpu BO? Set prefer_shadow_fbdev = 1. [1] I once tried to run generic fbdev without prefer_shadow_fbdev and it never worked. I suspected that some reference counting got wrong, but could never pin it down. Maybe your issue is similar. That said, generic fbdev is not so super-optimal for TTM-based drivers. I'm working on improving that, but it's not there yet. If I'm not completely mistaken this problem is unrelated to TTM and could happen with any generic fbdev based driver which supports DMA-buf. Does VKMS could be used with generic fbdev as well? If yes I think I can stitch together a test case demonstrating the issue. Regards, Christian. Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.0/source/include/drm/drm_mode_config.h#L890 Thanks, Christian.
Re: [PATCH] drm/msm/dpu: disable features unsupported by QCM2290
Hi,
On Wed, 25 Jan 2023 at 02:22, Abhinav Kumar wrote:
> On 1/24/2023 12:22 AM, Dmitry Baryshkov wrote:
> > On 24/01/2023 03:32, Abhinav Kumar wrote:
> >> On 1/22/2023 11:11 PM, Dmitry Baryshkov wrote:
> >>> QCM2290 doesn't seem to support reg-dma, smart-dma, UBWC, CDP, exclusion
> >>> rectangles and CSC. Drop corresponding features being incorrectly
> >>> enabled for qcm2290.
> >>>
> >>
> >> Can you please point me to which vendor DT you are referring to for this?
> >>
> >> CSC is supported on the VIG SSPPs from what I can see.
> >
> > https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi
> >
> >
> > No CSC, smart-dma, excl-rect, CDP, etc.
>
> Sorry I am missing something here.
>
> It has one Vig and one DMA
>
> https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi#L68
Correct
>
> If Vig is present, CSC is supported.
This actually puzzled me. Usually the dtsi has qcom,sde-sspp-csc-off
and qcom,sde-csc-type properties. But not in this case.
>
> Even for smart DMA I can see it supported
> https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi#L76
> on the DMA SSPP.
>
> Same for excl rectangle too
> https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi#L74
Ack, my mistake. Maybe I was looking at the wrong dtsi then (or just
mixed something). I'll add them back. And I see that CDP is also
there.
So, this leaves us only with the question regarding CSC. Could you
please doublecheck it?
I also don't see the UBWC (qcom,sde-ubwc-version) and regdma
(qcom,sde-reg-dma-off) properties. Are corresponding features present
on the QCM2290?
> >
> >> QCM2290 should be using the same MDP version as 6115 from the HW version.
> >
> > It is 6.3 vs 6.5 if I remember correctly.
> >
> >>
> >>
> >>> Cc: Loic Poulain
> >>> Fixes: 5334087ee743 ("drm/msm: add support for QCM2290 MDSS")
> >>> Signed-off-by: Dmitry Baryshkov
> >>> ---
> >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 20 +++
> >>> 1 file changed, 12 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>> index 289fb11f99d1..1c3ffa922794 100644
> >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>> @@ -12,10 +12,14 @@
> >>> #include "dpu_hw_catalog.h"
> >>> #include "dpu_kms.h"
> >>> -#define VIG_MASK \
> >>> +#define VIG_BASE_MASK \
> >>> (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |\
> >>> +BIT(DPU_SSPP_TS_PREFILL))
> >>> +
> >>> +#define VIG_MASK \
> >>> +(VIG_BASE_MASK | \
> >>> BIT(DPU_SSPP_CSC_10BIT) | BIT(DPU_SSPP_CDP) |\
> >>> -BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_EXCL_RECT))
> >>> +BIT(DPU_SSPP_EXCL_RECT))
> >>> #define VIG_MSM8998_MASK \
> >>> (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3))
> >>> @@ -29,7 +33,7 @@
> >>> #define VIG_SM8250_MASK \
> >>> (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>> BIT(DPU_SSPP_SCALER_QSEED3LITE))
> >>> -#define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
> >>> +#define VIG_QCM2290_MASK (VIG_BASE_MASK | BIT(DPU_SSPP_QOS_8LVL))
> >>> #define DMA_MSM8998_MASK \
> >>> (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |\
> >>> @@ -50,6 +54,10 @@
> >>> #define DMA_CURSOR_MSM8998_MASK \
> >>> (DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR))
> >>> +#define DMA_QCM2290_MASK \
> >>> +(BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\
> >>> +BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1))
> >>> +
> >>> #define MIXER_MSM8998_MASK \
> >>> (BIT(DPU_MIXER_SOURCESPLIT) | BIT(DPU_DIM_LAYER))
> >>> @@ -316,8 +324,6 @@ static const struct dpu_caps msm8998_dpu_caps = {
> >>> static const struct dpu_caps qcm2290_dpu_caps = {
> >>> .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> >>> .max_mixer_blendstages = 0x4,
> >>> -.smart_dma_rev = DPU_SSPP_SMART_DMA_V2,
> >>> -.ubwc_version = DPU_HW_UBWC_VER_20,
> >>> .has_dim_layer = true,
> >>> .has_idle_pc = true,
> >>> .max_linewidth = 2160,
> >>> @@ -1384,7 +1390,7 @@ static const struct dpu_sspp_sub_blks
> >>> qcm2290_dma_sblk_0 = _DMA_SBLK("8", 1);
> >>> static const struct dpu_sspp_cfg qcm2290_sspp[] = {
> >>> SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_QCM2290_MASK,
> >>>qcm2290_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> >>> -SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> >>> +SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_QCM2290_MASK,
> >>>qcm2290_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> >>> };
> >>> @@ -2836,8 +2842,6 @@ static const struct dpu_mdss_cfg
> >>> qcm2290_dpu_cfg = {
> >>> .intf = qcm2290_intf,
> >>> .vbif_count = ARRAY_SIZE(sdm845_vbif),
> >>> .vbif = sdm845_vbif,
> >>> -.reg_dma_count = 1,
> >>> -.dma_cfg = _regdma,
> >>> .perf = _perf_data,
>
[PATCH v2 5/6] drm: lcdif: Add multiple encoders and first bridges support
The single LCDIF embedded in i.MX93 SoC may drive multiple displays
simultaneously. Look at LCDIF output port's remote port parents to
find all enabled first bridges. Add an encoder for each found bridge
and attach the bridge to the encoder. This is a preparation for
adding i.MX93 LCDIF support.
Signed-off-by: Liu Ying
---
v1->v2:
* Split from patch 2/2 in v1. (Marek, Alexander)
* Drop '!remote ||' from lcdif_attach_bridge(). (Lothar)
* Drop unneeded 'bridges' member from lcdif_drm_private structure.
drivers/gpu/drm/mxsfb/lcdif_drv.c | 68 +++
drivers/gpu/drm/mxsfb/lcdif_drv.h | 4 +-
drivers/gpu/drm/mxsfb/lcdif_kms.c | 21 ++
3 files changed, 66 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index b5b9a8e273c6..eb6c265fa2fe 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -9,13 +9,16 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -38,19 +41,68 @@ static const struct drm_mode_config_helper_funcs
lcdif_mode_config_helpers = {
.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
};
+static const struct drm_encoder_funcs lcdif_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
{
- struct drm_device *drm = lcdif->drm;
+ struct device *dev = lcdif->drm->dev;
+ struct device_node *ep;
struct drm_bridge *bridge;
int ret;
- bridge = devm_drm_of_get_bridge(drm->dev, drm->dev->of_node, 0, 0);
- if (IS_ERR(bridge))
- return PTR_ERR(bridge);
-
- ret = drm_bridge_attach(>encoder, bridge, NULL, 0);
- if (ret)
- return dev_err_probe(drm->dev, ret, "Failed to attach
bridge\n");
+ for_each_endpoint_of_node(dev->of_node, ep) {
+ struct device_node *remote;
+ struct of_endpoint of_ep;
+ struct drm_encoder *encoder;
+
+ remote = of_graph_get_remote_port_parent(ep);
+ if (!of_device_is_available(remote)) {
+ of_node_put(remote);
+ continue;
+ }
+ of_node_put(remote);
+
+ ret = of_graph_parse_endpoint(ep, _ep);
+ if (ret < 0) {
+ dev_err(dev, "Failed to parse endpoint %pOF\n", ep);
+ of_node_put(ep);
+ return ret;
+ }
+
+ if (of_ep.id >= MAX_DISPLAYS) {
+ dev_warn(dev, "invalid endpoint id %u\n", of_ep.id);
+ continue;
+ }
+
+ bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, of_ep.id);
+ if (IS_ERR(bridge)) {
+ of_node_put(ep);
+ return dev_err_probe(dev, PTR_ERR(bridge),
+"Failed to get bridge for
endpoint%u\n",
+of_ep.id);
+ }
+
+ encoder = >encoders[of_ep.id];
+ encoder->possible_crtcs = drm_crtc_mask(>crtc);
+ ret = drm_encoder_init(lcdif->drm, encoder,
_encoder_funcs,
+ DRM_MODE_ENCODER_NONE, NULL);
+ if (ret) {
+ dev_err(dev, "Failed to initialize encoder for
endpoint%u: %d\n",
+ of_ep.id, ret);
+ of_node_put(ep);
+ return ret;
+ }
+
+ ret = drm_bridge_attach(encoder, bridge, NULL, 0);
+ if (ret) {
+ of_node_put(ep);
+ return dev_err_probe(dev, ret,
+"Failed to attach bridge for
endpoint%u\n",
+of_ep.id);
+ }
+ }
return 0;
}
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.h
b/drivers/gpu/drm/mxsfb/lcdif_drv.h
index aa6d099a1897..c7400bd9bbd9 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.h
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.h
@@ -14,6 +14,8 @@
#include
#include
+#define MAX_DISPLAYS 3
+
struct clk;
struct lcdif_drm_private {
@@ -30,7 +32,7 @@ struct lcdif_drm_private {
/* i.MXRT does support overlay planes, add them here. */
} planes;
struct drm_crtc crtc;
- struct drm_encoder encoder;
+ struct drm_encoder encoders[MAX_DISPLAYS];
};
static inline struct lcdif_drm_private *
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index 4ea3d2b2cf61..5f34d01e133e 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@
[PATCH v2 6/6] drm: lcdif: Add i.MX93 LCDIF compatible string
With all previous preparations done to make it possible for the
single LCDIF embedded in i.MX93 SoC to drive multipledisplays
simultaneously, add i.MX93 LCDIF compatible string as the last
step of adding i.MX93 LCDIF support.
Signed-off-by: Liu Ying
---
drivers/gpu/drm/mxsfb/lcdif_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index eb6c265fa2fe..48c43b273a4a 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -249,6 +249,7 @@ static const struct drm_driver lcdif_driver = {
static const struct of_device_id lcdif_dt_ids[] = {
{ .compatible = "fsl,imx8mp-lcdif" },
+ { .compatible = "fsl,imx93-lcdif" },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, lcdif_dt_ids);
--
2.37.1
[PATCH v2 4/6] drm: lcdif: Check consistent bus format and flags across first bridges
The single LCDIF embedded in i.MX93 SoC may drive multiple displays
simultaneously. Check bus format and flags across first bridges in
->atomic_check() to ensure they are consistent. This is a preparation
for adding i.MX93 LCDIF support.
Signed-off-by: Liu Ying
---
v1->v2:
* Split from patch 2/2 in v1. (Marek, Alexander)
* Drop a comment about bridge input bus format from lcdif_crtc_atomic_check().
drivers/gpu/drm/mxsfb/lcdif_drv.c | 2 -
drivers/gpu/drm/mxsfb/lcdif_drv.h | 1 -
drivers/gpu/drm/mxsfb/lcdif_kms.c | 76 ++-
3 files changed, 55 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index cc2ceb301b96..b5b9a8e273c6 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -52,8 +52,6 @@ static int lcdif_attach_bridge(struct lcdif_drm_private
*lcdif)
if (ret)
return dev_err_probe(drm->dev, ret, "Failed to attach
bridge\n");
- lcdif->bridge = bridge;
-
return 0;
}
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.h
b/drivers/gpu/drm/mxsfb/lcdif_drv.h
index 6cdba6e20c02..aa6d099a1897 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.h
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.h
@@ -31,7 +31,6 @@ struct lcdif_drm_private {
} planes;
struct drm_crtc crtc;
struct drm_encoder encoder;
- struct drm_bridge *bridge;
};
static inline struct lcdif_drm_private *
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index 294cecdf5439..4ea3d2b2cf61 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -424,15 +425,19 @@ static int lcdif_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
struct drm_device *drm = crtc->dev;
- struct lcdif_drm_private *lcdif = to_lcdif_drm_private(drm);
struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
struct lcdif_crtc_state *lcdif_crtc_state =
to_lcdif_crtc_state(crtc_state);
bool has_primary = crtc_state->plane_mask &
drm_plane_mask(crtc->primary);
+ struct drm_connector_state *connector_state;
+ struct drm_connector *connector;
+ struct drm_encoder *encoder;
struct drm_bridge_state *bridge_state;
- struct drm_bridge *bridge = lcdif->bridge;
- int ret;
+ struct drm_bridge *bridge;
+ u32 bus_format, bus_flags;
+ bool format_set = false, flags_set = false;
+ int ret, i;
/* The primary plane has to be enabled when the CRTC is active. */
if (crtc_state->active && !has_primary)
@@ -442,26 +447,55 @@ static int lcdif_crtc_atomic_check(struct drm_crtc *crtc,
if (ret)
return ret;
- bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
- if (!bridge_state)
- lcdif_crtc_state->bus_format = MEDIA_BUS_FMT_FIXED;
- else
- lcdif_crtc_state->bus_format =
bridge_state->input_bus_cfg.format;
-
- if (lcdif_crtc_state->bus_format == MEDIA_BUS_FMT_FIXED) {
- dev_warn_once(drm->dev,
- "Bridge does not provide bus format, assuming
MEDIA_BUS_FMT_RGB888_1X24.\n"
- "Please fix bridge driver by handling
atomic_get_input_bus_fmts.\n");
- lcdif_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ /* Try to find consistent bus format and flags across first bridges. */
+ for_each_new_connector_in_state(state, connector, connector_state, i) {
+ if (!connector_state->crtc)
+ continue;
+
+ encoder = connector_state->best_encoder;
+
+ bridge = drm_bridge_chain_get_first_bridge(encoder);
+ if (!bridge)
+ continue;
+
+ bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
+ if (!bridge_state)
+ bus_format = MEDIA_BUS_FMT_FIXED;
+ else
+ bus_format = bridge_state->input_bus_cfg.format;
+
+ if (bus_format == MEDIA_BUS_FMT_FIXED) {
+ dev_warn(drm->dev,
+"[ENCODER:%d:%s]'s bridge does not provide bus
format, assuming MEDIA_BUS_FMT_RGB888_1X24.\n"
+"Please fix bridge driver by handling
atomic_get_input_bus_fmts.\n",
+encoder->base.id, encoder->name);
+ bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ }
+
+ if (!format_set) {
+
[PATCH v2 3/6] drm: lcdif: Determine bus format and flags in ->atomic_check()
Instead of determining LCDIF output bus format and bus flags in
->atomic_enable(), do that in ->atomic_check(). This is a
preparation for the upcoming patch to check consistent bus format
and bus flags across all first downstream bridges in ->atomic_check().
New lcdif_crtc_state structure is introduced to cache bus format
and bus flags states in ->atomic_check() so that they can be read
in ->atomic_enable().
Signed-off-by: Liu Ying
---
v1->v2:
* Split from patch 2/2 in v1. (Marek, Alexander)
* Add comment on the 'base' member of lcdif_crtc_state structure to
note it should always be the first member. (Lothar)
drivers/gpu/drm/mxsfb/lcdif_kms.c | 138 ++
1 file changed, 100 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index e54200a9fcb9..294cecdf5439 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -30,6 +30,18 @@
#include "lcdif_drv.h"
#include "lcdif_regs.h"
+struct lcdif_crtc_state {
+ struct drm_crtc_state base; /* always be the first member */
+ u32 bus_format;
+ u32 bus_flags;
+};
+
+static inline struct lcdif_crtc_state *
+to_lcdif_crtc_state(struct drm_crtc_state *s)
+{
+ return container_of(s, struct lcdif_crtc_state, base);
+}
+
/*
-
* CRTC
*/
@@ -385,48 +397,72 @@ static void lcdif_reset_block(struct lcdif_drm_private
*lcdif)
readl(lcdif->base + LCDC_V8_CTRL);
}
-static void lcdif_crtc_mode_set_nofb(struct lcdif_drm_private *lcdif,
-struct drm_plane_state *plane_state,
-struct drm_bridge_state *bridge_state,
-const u32 bus_format)
+static void lcdif_crtc_mode_set_nofb(struct drm_crtc_state *crtc_state,
+struct drm_plane_state *plane_state)
{
- struct drm_device *drm = lcdif->crtc.dev;
- struct drm_display_mode *m = >crtc.state->adjusted_mode;
- u32 bus_flags = 0;
-
- if (lcdif->bridge->timings)
- bus_flags = lcdif->bridge->timings->input_bus_flags;
- else if (bridge_state)
- bus_flags = bridge_state->input_bus_cfg.flags;
+ struct lcdif_crtc_state *lcdif_crtc_state =
to_lcdif_crtc_state(crtc_state);
+ struct drm_device *drm = crtc_state->crtc->dev;
+ struct lcdif_drm_private *lcdif = to_lcdif_drm_private(drm);
+ struct drm_display_mode *m = _state->adjusted_mode;
DRM_DEV_DEBUG_DRIVER(drm->dev, "Pixel clock: %dkHz (actual: %dkHz)\n",
m->crtc_clock,
(int)(clk_get_rate(lcdif->clk) / 1000));
DRM_DEV_DEBUG_DRIVER(drm->dev, "Bridge bus_flags: 0x%08X\n",
-bus_flags);
+lcdif_crtc_state->bus_flags);
DRM_DEV_DEBUG_DRIVER(drm->dev, "Mode flags: 0x%08X\n", m->flags);
/* Mandatory eLCDIF reset as per the Reference Manual */
lcdif_reset_block(lcdif);
- lcdif_set_formats(lcdif, plane_state, bus_format);
+ lcdif_set_formats(lcdif, plane_state, lcdif_crtc_state->bus_format);
- lcdif_set_mode(lcdif, bus_flags);
+ lcdif_set_mode(lcdif, lcdif_crtc_state->bus_flags);
}
static int lcdif_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
+ struct drm_device *drm = crtc->dev;
+ struct lcdif_drm_private *lcdif = to_lcdif_drm_private(drm);
struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
+ struct lcdif_crtc_state *lcdif_crtc_state =
to_lcdif_crtc_state(crtc_state);
bool has_primary = crtc_state->plane_mask &
drm_plane_mask(crtc->primary);
+ struct drm_bridge_state *bridge_state;
+ struct drm_bridge *bridge = lcdif->bridge;
+ int ret;
/* The primary plane has to be enabled when the CRTC is active. */
if (crtc_state->active && !has_primary)
return -EINVAL;
- return drm_atomic_add_affected_planes(state, crtc);
+ ret = drm_atomic_add_affected_planes(state, crtc);
+ if (ret)
+ return ret;
+
+ bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
+ if (!bridge_state)
+ lcdif_crtc_state->bus_format = MEDIA_BUS_FMT_FIXED;
+ else
+ lcdif_crtc_state->bus_format =
bridge_state->input_bus_cfg.format;
+
+ if (lcdif_crtc_state->bus_format == MEDIA_BUS_FMT_FIXED) {
+ dev_warn_once(drm->dev,
+ "Bridge does not provide bus format, assuming
MEDIA_BUS_FMT_RGB888_1X24.\n"
+ "Please fix
[PATCH v2 2/6] drm: lcdif: Drop unnecessary NULL pointer check on lcdif->bridge
A valid bridge is already found in lcdif_attach_bridge() and set
to lcdif->bridge, so lcdif->bridge cannot be a NULL pointer. Drop
the unnecessary NULL pointer check in KMS stage.
Signed-off-by: Liu Ying
---
v1->v2:
* Split from patch 2/2 in v1. (Marek, Alexander)
drivers/gpu/drm/mxsfb/lcdif_kms.c | 33 +++
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index 262bc43b1079..e54200a9fcb9 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -394,7 +394,7 @@ static void lcdif_crtc_mode_set_nofb(struct
lcdif_drm_private *lcdif,
struct drm_display_mode *m = >crtc.state->adjusted_mode;
u32 bus_flags = 0;
- if (lcdif->bridge && lcdif->bridge->timings)
+ if (lcdif->bridge->timings)
bus_flags = lcdif->bridge->timings->input_bus_flags;
else if (bridge_state)
bus_flags = bridge_state->input_bus_cfg.flags;
@@ -463,30 +463,21 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc
*crtc,
struct drm_display_mode *m = >crtc.state->adjusted_mode;
struct drm_bridge_state *bridge_state = NULL;
struct drm_device *drm = lcdif->drm;
- u32 bus_format = 0;
+ u32 bus_format;
dma_addr_t paddr;
- /* If there is a bridge attached to the LCDIF, use its bus format */
- if (lcdif->bridge) {
- bridge_state =
- drm_atomic_get_new_bridge_state(state,
- lcdif->bridge);
- if (!bridge_state)
- bus_format = MEDIA_BUS_FMT_FIXED;
- else
- bus_format = bridge_state->input_bus_cfg.format;
-
- if (bus_format == MEDIA_BUS_FMT_FIXED) {
- dev_warn_once(drm->dev,
- "Bridge does not provide bus format,
assuming MEDIA_BUS_FMT_RGB888_1X24.\n"
- "Please fix bridge driver by handling
atomic_get_input_bus_fmts.\n");
- bus_format = MEDIA_BUS_FMT_RGB888_1X24;
- }
- }
+ bridge_state = drm_atomic_get_new_bridge_state(state, lcdif->bridge);
+ if (!bridge_state)
+ bus_format = MEDIA_BUS_FMT_FIXED;
+ else
+ bus_format = bridge_state->input_bus_cfg.format;
- /* If all else fails, default to RGB888_1X24 */
- if (!bus_format)
+ if (bus_format == MEDIA_BUS_FMT_FIXED) {
+ dev_warn_once(drm->dev,
+ "Bridge does not provide bus format, assuming
MEDIA_BUS_FMT_RGB888_1X24.\n"
+ "Please fix bridge driver by handling
atomic_get_input_bus_fmts.\n");
bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ }
clk_set_rate(lcdif->clk, m->crtc_clock * 1000);
--
2.37.1
[PATCH v2 1/6] dt-bindings: lcdif: Add i.MX93 LCDIF support
There is one LCDIF embedded in i.MX93 SoC to connect with MIPI DSI controller through LCDIF cross line pattern(controlled by mediamix blk-ctrl) or connect with LVDS display bridge(LDB) directly or connect with a parallel display through parallel display format(also controlled by mediamix blk-ctrl). i.MX93 LCDIF IP is essentially the same to i.MX8MP LCDIF IP. Add device tree binding for i.MX93 LCDIF. Acked-by: Krzysztof Kozlowski Reviewed-by: Marek Vasut Signed-off-by: Liu Ying --- v1->v2: * Add Krzysztof's A-b and Marek's R-b tags on patch 1/6. Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index 75b4efd70ba8..fc11ab5fc465 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -21,6 +21,7 @@ properties: - fsl,imx28-lcdif - fsl,imx6sx-lcdif - fsl,imx8mp-lcdif + - fsl,imx93-lcdif - items: - enum: - fsl,imx6sl-lcdif @@ -88,7 +89,9 @@ allOf: properties: compatible: contains: -const: fsl,imx8mp-lcdif +enum: + - fsl,imx8mp-lcdif + - fsl,imx93-lcdif then: properties: clocks: @@ -107,6 +110,7 @@ allOf: enum: - fsl,imx6sx-lcdif - fsl,imx8mp-lcdif +- fsl,imx93-lcdif then: properties: clocks: @@ -123,6 +127,7 @@ allOf: - fsl,imx8mm-lcdif - fsl,imx8mn-lcdif - fsl,imx8mp-lcdif + - fsl,imx93-lcdif then: required: - power-domains -- 2.37.1
[PATCH v2 0/6] drm: lcdif: Add i.MX93 LCDIF support
Hi, This patch set aims to add i.MX93 LCDIF display controller support in the existing LCDIF DRM driver. The LCDIF embedded in i.MX93 SoC is essentially the same to those embedded in i.MX8mp SoC. Through internal bridges, i.MX93 LCDIF may drive a MIPI DSI display or a LVDS display or a parallel display. Patch 1/6 adds device tree binding support for i.MX93 LCDIF in the existing fsl,lcdif.yaml. Patch 2/6 drops lcdif->bridge NULL pointer check as a cleanup patch. Patch 3/6~5/6 prepare for adding i.MX93 LCDIF support step by step. Patch 6/6 adds i.MX93 LCDIF compatible string as the last step of adding i.MX93 LCDIF support. v1->v2: * Add Krzysztof's A-b and Marek's R-b tags on patch 1/6. * Split patch 2/2 in v1 into patch 2/6~6/6 in v2. (Marek, Alexander) * Drop '!remote ||' from lcdif_attach_bridge(). (Lothar) * Add comment on the 'base' member of lcdif_crtc_state structure to note it should always be the first member. (Lothar) * Drop unneeded 'bridges' member from lcdif_drm_private structure. * Drop a comment about bridge input bus format from lcdif_crtc_atomic_check(). Liu Ying (6): dt-bindings: lcdif: Add i.MX93 LCDIF support drm: lcdif: Drop unnecessary NULL pointer check on lcdif->bridge drm: lcdif: Determine bus format and flags in ->atomic_check() drm: lcdif: Check consistent bus format and flags across first bridges drm: lcdif: Add multiple encoders and first bridges support drm: lcdif: Add i.MX93 LCDIF compatible string .../bindings/display/fsl,lcdif.yaml | 7 +- drivers/gpu/drm/mxsfb/lcdif_drv.c | 71 +- drivers/gpu/drm/mxsfb/lcdif_drv.h | 5 +- drivers/gpu/drm/mxsfb/lcdif_kms.c | 202 -- 4 files changed, 207 insertions(+), 78 deletions(-) -- 2.37.1
Re: [PATCH 02/16] drm/i915/dp: Avoid forcing DSC BPC for MST case
On 1/24/2023 9:53 PM, Jani Nikula wrote:
On Fri, 20 Jan 2023, Ankit Nautiyal wrote:
For MST the bpc is hardcoded to 8, and pipe bpp to 24.
So avoid forcing DSC bpc for MST case.
It's likely better to warn and ignore the debug flag than to bail out.
Hmm..but then the test using this flag will not come to know that bpc it
wanted to force, was indeed used.
Currently there is no way to read the bpc that was selected by the
driver, and so the test can try to force DSC, and the bpc and rely on
whether the ioctl passes or fails.
Regards,
Ankit
Signed-off-by: Ankit Nautiyal
---
drivers/gpu/drm/i915/display/intel_dp.c | 11 +--
drivers/gpu/drm/i915/display/intel_dp_mst.c | 8
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 93aebd3683a4..3d828ea0894d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1500,14 +1500,13 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
if (!intel_dp_supports_dsc(intel_dp, pipe_config))
return -EINVAL;
- if (compute_pipe_bpp)
+ if (intel_dp->force_dsc_bpc && compute_pipe_bpp) {
+ pipe_bpp = intel_dp->force_dsc_bpc * 3;
+ drm_dbg_kms(_priv->drm, "Input DSC BPP forced to %d\n",
pipe_bpp);
+ } else if (compute_pipe_bpp) {
pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
- else
+ } else {
pipe_bpp = pipe_config->pipe_bpp;
-
- if (intel_dp->force_dsc_bpc) {
- pipe_bpp = intel_dp->force_dsc_bpc * 3;
- drm_dbg_kms(_priv->drm, "Input DSC BPP forced to %d",
pipe_bpp);
}
/* Min Input BPC for ICL+ is 8 */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 8b0e4defa3f1..9be04c60cced 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -325,6 +325,14 @@ static int intel_dp_mst_compute_config(struct
intel_encoder *encoder,
/* enable compression if the mode doesn't fit available BW */
drm_dbg_kms(_priv->drm, "Force DSC en = %d\n",
intel_dp->force_dsc_en);
if (ret || intel_dp->force_dsc_en) {
+ if (intel_dp->force_dsc_bpc) {
+ /*
+* FIXME: As bpc is hardcoed to 8 bpc as mentioned
above,
+* Avoid force BPC for now.
+*/
+ drm_dbg_kms(_priv->drm, "Cannot Force BPC for
MST\n");
+ return -EINVAL;
+ }
/*
* Try to get at least some timeslots and then see, if
* we can fit there with DSC.
Re: [PATCH] dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
On Tue, Jan 24, 2023 at 05:02:28PM -0600, Rob Herring wrote: > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present. > > Add unevaluatedProperties or additionalProperties as appropriate, and > then add any missing properties flagged by the addition. > > Signed-off-by: Rob Herring > --- > To: Krzysztof Kozlowski > To: David Airlie > To: Daniel Vetter > To: Bartosz Golaszewski > To: Jean Delvare > To: Guenter Roeck > To: Thomas Gleixner > To: Marc Zyngier > To: Jassi Brar > To: Mauro Carvalho Chehab > To: Lee Jones > To: Ulf Hansson > To: Richard Weinberger > To: Vignesh Raghavendra > To: Sebastian Reichel > To: Mark Brown > To: "Rafael J. Wysocki" > To: Daniel Lezcano > To: Greg Kroah-Hartman > Cc: linux-arm-ker...@lists.infradead.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-g...@vger.kernel.org > Cc: linux-hw...@vger.kernel.org For hwmon: Acked-by: Guenter Roeck > Cc: linux-me...@vger.kernel.org > Cc: open...@lists.ozlabs.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@lists.infradead.org > Cc: linux...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@vger.kernel.org > --- > .../devicetree/bindings/arm/arm,vexpress-juno.yaml | 1 + > .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml | 5 +++-- > .../bindings/arm/tegra/nvidia,tegra20-pmc.yaml | 4 > .../bindings/bus/allwinner,sun50i-a64-de2.yaml | 1 + > .../bindings/bus/allwinner,sun8i-a23-rsb.yaml | 1 + > .../bus/intel,ixp4xx-expansion-bus-controller.yaml | 6 ++ > Documentation/devicetree/bindings/bus/palmbus.yaml | 1 + > .../devicetree/bindings/display/msm/qcom,mdss.yaml | 5 + > Documentation/devicetree/bindings/example-schema.yaml | 2 ++ > .../devicetree/bindings/gpio/x-powers,axp209-gpio.yaml | 1 + > .../devicetree/bindings/hwmon/adi,ltc2992.yaml | 1 + > .../bindings/interrupt-controller/arm,gic-v3.yaml | 2 ++ > .../bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml | 1 + > .../devicetree/bindings/media/i2c/maxim,max9286.yaml | 7 +++ > .../bindings/memory-controllers/arm,pl35x-smc.yaml | 1 + > .../bindings/memory-controllers/exynos-srom.yaml | 1 + > .../memory-controllers/nvidia,tegra124-emc.yaml| 1 + > .../bindings/memory-controllers/st,stm32-fmc2-ebi.yaml | 1 + > .../devicetree/bindings/mfd/mediatek,mt6370.yaml | 2 ++ > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 1 + > Documentation/devicetree/bindings/mtd/mtd.yaml | 1 + > .../devicetree/bindings/power/supply/ti,lp8727.yaml| 1 + > .../devicetree/bindings/soc/imx/fsl,imx93-src.yaml | 3 ++- > .../bindings/soc/microchip/atmel,at91rm9200-tcb.yaml | 1 + > Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 1 + > .../devicetree/bindings/sound/marvell,mmp-sspa.yaml| 1 + > .../devicetree/bindings/sound/qcom,wcd934x.yaml| 1 + > .../devicetree/bindings/sound/samsung,odroid.yaml | 2 ++ > .../devicetree/bindings/soundwire/qcom,soundwire.yaml | 1 + > .../bindings/spi/allwinner,sun4i-a10-spi.yaml | 1 + > .../bindings/spi/allwinner,sun6i-a31-spi.yaml | 1 + > .../devicetree/bindings/spi/spi-controller.yaml| 1 + > .../sram/allwinner,sun4i-a10-system-control.yaml | 10 +- > Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 1 + > .../devicetree/bindings/thermal/thermal-zones.yaml | 1 + > Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 + > 36 files changed, 65 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml > b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml > index eec190a96225..09c319f803ba 100644 > --- a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml > +++ b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml > @@ -144,6 +144,7 @@ patternProperties: >it is stricter and always has two compatibles. > type: object > $ref: '/schemas/simple-bus.yaml' > +unevaluatedProperties: false > > properties: >compatible: > diff --git > a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml > b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml > index b369b374fc4a..39e3c248f5b7 100644 > --- > a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml > +++ > b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml > @@ -30,6 +30,7 @@ properties: > >clocks: > type: object > +additionalProperties: false > > properties: >compatible: > @@ -47,6 +48,7 @@ properties: > >reset: > type: object > +additionalProperties: false > >
Re: [Intel-gfx] [PATCH v2 06/10] drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function
Hi Thomas,
I love your patch! Yet something to improve:
[auto build test ERROR on 7d3e7f64a42d66ba8da6e7b66a8d85457ef84570]
url:
https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-client-Test-for-connectors-before-sending-hotplug-event/20230124-214220
base: 7d3e7f64a42d66ba8da6e7b66a8d85457ef84570
patch link:
https://lore.kernel.org/r/20230124134010.30263-7-tzimmermann%40suse.de
patch subject: [Intel-gfx] [PATCH v2 06/10] drm/fb-helper: Initialize
fb-helper's preferred BPP in prepare function
config: riscv-randconfig-r042-20230123
(https://download.01.org/0day-ci/archive/20230125/202301251250.eyin4zjn-...@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project
4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
#
https://github.com/intel-lab-lkp/linux/commit/70e38534e74e4d12bb02b3b352bba2aed417f541
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Thomas-Zimmermann/drm-client-Test-for-connectors-before-sending-hotplug-event/20230124-214220
git checkout 70e38534e74e4d12bb02b3b352bba2aed417f541
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/radeon/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/radeon/radeon_fb.c:352:10: error: too many arguments to
>> function call, expected 3, have 4
_fb_helper_funcs);
^~~
include/drm/drm_fb_helper.h:295:20: note: 'drm_fb_helper_prepare' declared
here
static inline void drm_fb_helper_prepare(struct drm_device *dev,
^
>> drivers/gpu/drm/radeon/radeon_fb.c:361:52: error: too few arguments to
>> function call, expected 2, have 1
ret = drm_fb_helper_initial_config(>helper);
^
include/drm/drm_fb_helper.h:459:19: note: 'drm_fb_helper_initial_config'
declared here
static inline int drm_fb_helper_initial_config(struct drm_fb_helper
*fb_helper,
^
2 errors generated.
vim +352 drivers/gpu/drm/radeon/radeon_fb.c
386516744ba45d Dave Airlie 2010-03-30 326
386516744ba45d Dave Airlie 2010-03-30 327 int radeon_fbdev_init(struct
radeon_device *rdev)
386516744ba45d Dave Airlie 2010-03-30 328 {
8be48d924c307e Dave Airlie 2010-03-30 329struct radeon_fbdev
*rfbdev;
4abe35204af82a Dave Airlie 2010-03-30 330int bpp_sel = 32;
5a79395b2791cc Chris Wilson 2010-06-06 331int ret;
4abe35204af82a Dave Airlie 2010-03-30 332
e5f243bd2edd95 Alex Deucher 2016-03-10 333/* don't enable fbdev
if no connectors */
e5f243bd2edd95 Alex Deucher 2016-03-10 334if
(list_empty(>ddev->mode_config.connector_list))
e5f243bd2edd95 Alex Deucher 2016-03-10 335return 0;
e5f243bd2edd95 Alex Deucher 2016-03-10 336
7b8bd6bb4298ac Egbert Eich 2017-07-18 337/* select 8 bpp console
on 8MB cards, or 16 bpp on RN50 or 32MB */
7b8bd6bb4298ac Egbert Eich 2017-07-18 338if
(rdev->mc.real_vram_size <= (8*1024*1024))
4abe35204af82a Dave Airlie 2010-03-30 339bpp_sel = 8;
7b8bd6bb4298ac Egbert Eich 2017-07-18 340else if
(ASIC_IS_RN50(rdev) ||
7b8bd6bb4298ac Egbert Eich 2017-07-18 341
rdev->mc.real_vram_size <= (32*1024*1024))
7b8bd6bb4298ac Egbert Eich 2017-07-18 342bpp_sel = 16;
8be48d924c307e Dave Airlie 2010-03-30 343
8be48d924c307e Dave Airlie 2010-03-30 344rfbdev =
kzalloc(sizeof(struct radeon_fbdev), GFP_KERNEL);
8be48d924c307e Dave Airlie 2010-03-30 345if (!rfbdev)
8be48d924c307e Dave Airlie 2010-03-30 346return -ENOMEM;
771fe6b912fca5 Jerome Glisse 2009-06-05 347
8be48d924c307e Dave Airlie 2010-03-30 348rfbdev->rdev = rdev;
8be48d924c307e Dave Airlie 2010-03-30 349rdev->mode_info.rfbdev
= rfbdev;
10a231026574f9 Thierry Reding2014-06-27 350
70e38534e74e4d Thomas Zimmermann 2023-01-24 351
drm_fb_helper_prepare(rdev->ddev, >helper, bpp_sel,
10a231026574f9 Thierry Reding2014-06-
Re: [PATCH] PM / devfreq: Fix build issues with devfreq disabled
On Tue, Jan 24, 2023 at 8:04 PM MyungJoo Ham wrote:
>
> >Sender : Rob Clark
> >Date : 2023-01-24 00:37 (GMT+9)
> >Title : [PATCH] PM / devfreq: Fix build issues with devfreq disabled
> >
> >From: Rob Clark
> >
> >The existing no-op shims for when PM_DEVFREQ (or an individual governor)
> >only do half the job. The governor specific config/tuning structs need
> >to be available to avoid compile errors in drivers using devfreq.
> >
> >Fixes: 6563f60f14cb ("drm/msm/gpu: Add devfreq tuning debugfs")
> >Signed-off-by: Rob Clark
>
> Doesn't this imply that DRM_MSM should depend on PM_DEVFREQ ?
>
> It appears that gpu/drm/msm/DRM_MSM uses PM_DEVFREQ without actually
> declaring the dependency on PM_DEVFREQ.
> You cannot use SIMPLE_ONDEMAND without DEVFREQ.
Possibly that would resolve some issues, and that might have been my
mistake in assuming that depending on SIMPLE_ONDEMAND implied a
dependency on DEFREQ (which seems like a reasonable assumption, IMHO)
But AFAICT some kernel configs that could otherwise use DRM_MSM don't
support PM_DEVFREQ.. either way, lets solve this properly and remove
needless dependencies on devfreq.
BR,
-R
> Cheers,
> MyungJoo
>
> >---
> >Assuming this doesn't conflict with anything else landing via another
> >tree, an a-b to land this via drm/msm-next would let us un-break builds.
> >(And also start removing "select DEVFREQ_GOV_SIMPLE_ONDEMAND"s added in
> >various places to try to work around this issue.)
> >
> > include/linux/devfreq.h | 7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> >diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> >index 4dc7cda4fd46..7fd704bb8f3d 100644
> >--- a/include/linux/devfreq.h
> >+++ b/include/linux/devfreq.h
> >@@ -273,8 +273,8 @@ void devm_devfreq_unregister_notifier(struct device *dev,
> > struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node);
> > struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev,
> > const char *phandle_name, int index);
> >+#endif /* CONFIG_PM_DEVFREQ */
> >
> >-#if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
> > /**
> > * struct devfreq_simple_ondemand_data - ``void *data`` fed to struct
> > devfreq
> > *and devfreq_add_device
> >@@ -292,9 +292,7 @@ struct devfreq_simple_ondemand_data {
> > unsigned int upthreshold;
> > unsigned int downdifferential;
> > };
> >-#endif
> >
> >-#if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
> > enum devfreq_parent_dev_type {
> > DEVFREQ_PARENT_DEV,
> > CPUFREQ_PARENT_DEV,
> >@@ -337,9 +335,8 @@ struct devfreq_passive_data {
> > struct notifier_block nb;
> > struct list_head cpu_data_list;
> > };
> >-#endif
> >
> >-#else /* !CONFIG_PM_DEVFREQ */
> >+#if !defined(CONFIG_PM_DEVFREQ)
> > static inline struct devfreq *devfreq_add_device(struct device *dev,
> > struct devfreq_dev_profile *profile,
> > const char *governor_name,
> >--
> >2.38.1
> >
>
[PATCH] xe: use Makefile/include_next magic to avoid need for symlinks.
From: Dave Airlie I don't really like the symlinks, my other idea was creating new files which just include the original, but this idea seems to at least build. Cc: Matthew Brost Cc: Maarten Lankhorst Cc: Jani Nikula --- drivers/gpu/drm/i915/display/intel_de.h | 4 .../gpu/drm/i915/display/intel_display_trace.h | 4 drivers/gpu/drm/xe/Makefile | 16 drivers/gpu/drm/xe/display/ext/intel_dram.c | 2 +- drivers/gpu/drm/xe/display/i915_reg.h| 1 - drivers/gpu/drm/xe/display/icl_dsi.c | 1 - drivers/gpu/drm/xe/display/icl_dsi.h | 1 - drivers/gpu/drm/xe/display/icl_dsi_regs.h| 1 - drivers/gpu/drm/xe/display/intel_acpi.c | 1 - drivers/gpu/drm/xe/display/intel_acpi.h | 1 - drivers/gpu/drm/xe/display/intel_atomic.c| 1 - drivers/gpu/drm/xe/display/intel_atomic.h| 1 - drivers/gpu/drm/xe/display/intel_atomic_plane.c | 1 - drivers/gpu/drm/xe/display/intel_atomic_plane.h | 1 - drivers/gpu/drm/xe/display/intel_audio.c | 1 - drivers/gpu/drm/xe/display/intel_audio.h | 1 - drivers/gpu/drm/xe/display/intel_audio_regs.h| 1 - drivers/gpu/drm/xe/display/intel_backlight.c | 1 - drivers/gpu/drm/xe/display/intel_backlight.h | 1 - .../gpu/drm/xe/display/intel_backlight_regs.h| 1 - drivers/gpu/drm/xe/display/intel_bios.c | 1 - drivers/gpu/drm/xe/display/intel_bios.h | 1 - drivers/gpu/drm/xe/display/intel_bw.c| 1 - drivers/gpu/drm/xe/display/intel_bw.h| 1 - drivers/gpu/drm/xe/display/intel_cdclk.c | 1 - drivers/gpu/drm/xe/display/intel_cdclk.h | 1 - drivers/gpu/drm/xe/display/intel_color.c | 1 - drivers/gpu/drm/xe/display/intel_color.h | 1 - drivers/gpu/drm/xe/display/intel_combo_phy.c | 1 - drivers/gpu/drm/xe/display/intel_combo_phy.h | 1 - .../gpu/drm/xe/display/intel_combo_phy_regs.h| 1 - drivers/gpu/drm/xe/display/intel_connector.c | 1 - drivers/gpu/drm/xe/display/intel_connector.h | 1 - drivers/gpu/drm/xe/display/intel_crtc.c | 1 - drivers/gpu/drm/xe/display/intel_crtc.h | 1 - .../gpu/drm/xe/display/intel_crtc_state_dump.c | 1 - .../gpu/drm/xe/display/intel_crtc_state_dump.h | 1 - drivers/gpu/drm/xe/display/intel_cursor.c| 1 - drivers/gpu/drm/xe/display/intel_cursor.h| 1 - drivers/gpu/drm/xe/display/intel_ddi.c | 1 - drivers/gpu/drm/xe/display/intel_ddi.h | 1 - drivers/gpu/drm/xe/display/intel_ddi_buf_trans.c | 1 - drivers/gpu/drm/xe/display/intel_ddi_buf_trans.h | 1 - drivers/gpu/drm/xe/display/intel_display.c | 1 - drivers/gpu/drm/xe/display/intel_display.h | 1 - drivers/gpu/drm/xe/display/intel_display_core.h | 1 - .../gpu/drm/xe/display/intel_display_debugfs.c | 1 - .../gpu/drm/xe/display/intel_display_debugfs.h | 1 - drivers/gpu/drm/xe/display/intel_display_power.c | 1 - drivers/gpu/drm/xe/display/intel_display_power.h | 1 - .../gpu/drm/xe/display/intel_display_power_map.c | 1 - .../gpu/drm/xe/display/intel_display_power_map.h | 1 - .../drm/xe/display/intel_display_power_well.c| 1 - .../drm/xe/display/intel_display_power_well.h| 1 - drivers/gpu/drm/xe/display/intel_display_trace.c | 1 - drivers/gpu/drm/xe/display/intel_display_trace.h | 1 - drivers/gpu/drm/xe/display/intel_display_types.h | 1 - drivers/gpu/drm/xe/display/intel_dmc.c | 1 - drivers/gpu/drm/xe/display/intel_dmc.h | 1 - drivers/gpu/drm/xe/display/intel_dmc_regs.h | 1 - drivers/gpu/drm/xe/display/intel_dp.c| 1 - drivers/gpu/drm/xe/display/intel_dp.h| 1 - drivers/gpu/drm/xe/display/intel_dp_aux.c| 1 - drivers/gpu/drm/xe/display/intel_dp_aux.h| 1 - .../gpu/drm/xe/display/intel_dp_aux_backlight.c | 1 - .../gpu/drm/xe/display/intel_dp_aux_backlight.h | 1 - drivers/gpu/drm/xe/display/intel_dp_hdcp.c | 1 - drivers/gpu/drm/xe/display/intel_dp_hdcp.h | 1 - .../gpu/drm/xe/display/intel_dp_link_training.c | 1 - .../gpu/drm/xe/display/intel_dp_link_training.h | 1 - drivers/gpu/drm/xe/display/intel_dp_mst.c| 1 - drivers/gpu/drm/xe/display/intel_dp_mst.h| 1 - drivers/gpu/drm/xe/display/intel_dpio_phy.h | 1 - drivers/gpu/drm/xe/display/intel_dpll.c | 1 - drivers/gpu/drm/xe/display/intel_dpll.h | 1 - drivers/gpu/drm/xe/display/intel_dpll_mgr.c | 1 - drivers/gpu/drm/xe/display/intel_dpll_mgr.h | 1 - drivers/gpu/drm/xe/display/intel_dpt.h | 1 - drivers/gpu/drm/xe/display/intel_drrs.c | 1 - drivers/gpu/drm/xe/display/intel_drrs.h | 1 - drivers/gpu/drm/xe/display/intel_dsb.c | 1 - drivers/gpu/drm/xe/display/intel_dsb.h | 1 - drivers/gpu/drm/xe/display/intel_dsi.c | 1 -
Re: [PATCH v2 6/9] drm/i915/pxp: Add ARB session creation with new PXP API Ver4.3
Thanks Daniele - agreed on all counts - will fix accordingly - already done and tested internally - will re-rev shortly. ..alan On Wed, 2023-01-18 at 17:50 -0800, Ceraolo Spurio, Daniele wrote: > > > On 1/11/2023 1:42 PM, Alan Previn wrote: > > Add MTL's function for ARB session creation using PXP firmware > > version 4.3 ABI structure format. > > > > Before checking the return status, look at the GSC-CS-Mem-Header's > > pending-bit which means the GSC firmware is busy and we should > > resubmit. > > > > Signed-off-by: Alan Previn > > --- > > .../drm/i915/pxp/intel_pxp_cmd_interface_43.h | 21 +++ > > drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c | 56 ++- > > 2 files changed, 74 insertions(+), 3 deletions(-) alan:snip..
Re: [PATCH v6 2/6] drm/i915/pxp: add device link between i915 and mei_pxp
Thanks Rodrigo - will do.
On Tue, 2023-01-24 at 10:10 -0500, Vivi, Rodrigo wrote:
> On Mon, Jan 23, 2023 at 09:31:46PM -0800, Alan Previn wrote:
> > From: Alexander Usyskin
> >
> > Add device link with i915 as consumer and mei_pxp as supplier
> > to ensure proper ordering of power flows.
> >
> > V2: condition on absence of heci_pxp to filter out DG
> >
> > Signed-off-by: Alexander Usyskin
> > ---
> > drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 11 +++
> > drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 6 ++
> > 2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > index 73aa8015f828..cd5b86216506 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > @@ -127,6 +127,12 @@ static int i915_pxp_tee_component_bind(struct device
> > *i915_kdev,
> > intel_wakeref_t wakeref;
> > int ret = 0;
> >
> > + if (!HAS_HECI_PXP(i915)) {
> > + pxp->component_dev_link = device_link_add(i915_kdev,
> > tee_kdev, DL_FLAG_STATELESS);
> > + if (drm_WARN_ON(>drm, !pxp->component_dev_link))
> > + return -ENODEV;
> > + }
> > +
> > mutex_lock(>tee_mutex);
> > pxp->pxp_component = data;
> > pxp->pxp_component->tee_dev = tee_kdev;
> > @@ -169,6 +175,11 @@ static void i915_pxp_tee_component_unbind(struct
> > device *i915_kdev,
> > mutex_lock(>tee_mutex);
> > pxp->pxp_component = NULL;
> > mutex_unlock(>tee_mutex);
> > +
> > + if (pxp->component_dev_link) {
> > + device_link_remove(i915_kdev, tee_kdev);
> > + pxp->component_dev_link = NULL;
> > + }
>
> use the 'del' version instead of the 'remove' one.
>
> if (pxp->dev_link)
> device_link_del(pxp->dev_link);
>
> > }
> >
> > static const struct component_ops i915_pxp_tee_component_ops = {
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > index 7dc5f08d1583..efd2f3915abe 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > @@ -32,6 +32,12 @@ struct intel_pxp {
> > * which are protected by _mutex.
> > */
> > struct i915_pxp_component *pxp_component;
> > +
> > + /**
> > + * @component_dev_link: device link of the pxp-component enforcing
> > i915 as the
> > + * consumer. This is needed for legacy platform (TGL/ADL)
> > full-feature usage.
>
> No need to add platform information here.
>
> What about something shorter:
>
> /* @dev_link: Enforce module relationship for power management ordering. */
>
> > + */
> > + struct device_link *component_dev_link;
>
> What about
> struct device_link *dev_link;
>
> 'component' is already part of the pxp struct.
>
> > /**
> > * @pxp_component_added: track if the pxp component has been added.
> > * Set and cleared in tee init and fini functions respectively.
> > --
> > 2.39.0
> >
Re: [Intel-gfx] [PATCH v6 5/6] drm/i915/pxp: Trigger the global teardown for before suspending
On Tue, 2023-01-24 at 13:43 -0500, Vivi, Rodrigo wrote: > On Tue, Jan 24, 2023 at 10:08:52AM -0800, Juston Li wrote: > > On Tue, 2023-01-24 at 10:17 -0500, Rodrigo Vivi wrote: > > > On Mon, Jan 23, 2023 at 09:31:49PM -0800, Alan Previn wrote: > > > > A driver bug was recently discovered where the security firmware > > > > was > > > > receiving internal HW signals indicating that session key > > > > expirations > > > > had occurred. Architecturally, the firmware was expecting a > > > > response > > > > from the GuC to acknowledge the event with the firmware side. > > > > However the OS was in a suspended state and GuC had been reset. > > > > > > > > Internal specifications actually required the driver to ensure > > > > that all active sessions be properly cleaned up in such cases where > > > > the system is suspended and the GuC potentially unable to respond. > > > > > > > > This patch adds the global teardown code in i915's suspend_prepare > > > > code path. > > > > > > > > Signed-off-by: Alan Previn > > > > Reviewed-by: Juston Li > > > > > > it should probably contain some revision history above so the > > > reviewer can know what changed from the time he reviewed and now. > > > > > > it looks we have another function that I don't remember... > > > Juston, could you please confirm your review stands in this version? > > > if so feel free to add my ack with that. > > > > Sure, v6 is: > > Reviewed-by: Juston Li > > > Acked-by: Rodrigo Vivi > > > Thanks guys - will respin the final one (i hope) with tweaks on patch #2. ...alan
Re: [PATCH v3 3/6] ARM: dts: freescale: Use new media bus type macros
On Thu, Jun 16, 2022 at 01:14:07AM +0300, Laurent Pinchart wrote: > Now that a header exists with macros for the media interface bus-type > values, replace hardcoding numerical constants with the corresponding > macros in the DT sources. > > Signed-off-by: Laurent Pinchart Applied, thanks!
Re: [PATCH v6 4/4] drm/msm: stop storing the array of CRTCs in struct msm_drm_private
On 6/17/2022 4:33 PM, Dmitry Baryshkov wrote:
The array of CRTC in the struct msm_drm_private duplicates a list of
CRTCs in the drm_device. Drop it and use the existing list for CRTC
enumeration.
Reported-by: kernel test robot
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
drivers/gpu/drm/msm/msm_drv.c| 44 +---
drivers/gpu/drm/msm/msm_drv.h| 3 +-
5 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e23e2552e802..e79f0a8817ac 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -806,7 +806,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
ret = PTR_ERR(crtc);
return ret;
}
- priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->num_crtcs++;
}
/* All CRTCs are compatible with all encoders */
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index fb48c8c19ec3..7449c1693e45 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -337,7 +337,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
goto fail;
}
- priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->num_crtcs++;
}
/*
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 3d5621a68f85..36808990f840 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -497,7 +497,7 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
DRM_DEV_ERROR(dev->dev, "failed to construct crtc %d
(%d)\n", i, ret);
goto fail;
}
- priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->num_crtcs++;
}
/*
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1aab6bf86278..567e77dae43b 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -149,7 +149,7 @@ static void msm_irq_uninstall(struct drm_device *dev)
struct msm_vblank_work {
struct work_struct work;
- int crtc_id;
+ struct drm_crtc *crtc;
bool enable;
struct msm_drm_private *priv;
};
@@ -162,15 +162,15 @@ static void vblank_ctrl_worker(struct work_struct *work)
struct msm_kms *kms = priv->kms;
Is there any chance of vbl_work->crtc becoming NULL before this gets
executed?
So do we need to protect this like
if (vbl_work->enable && vbl_work->crtc)
Because the layers below this dont seem to have NULL protection.
if (vbl_work->enable)
- kms->funcs->enable_vblank(kms, priv->crtcs[vbl_work->crtc_id]);
+ kms->funcs->enable_vblank(kms, vbl_work->crtc);
else
- kms->funcs->disable_vblank(kms,
priv->crtcs[vbl_work->crtc_id]);
+ kms->funcs->disable_vblank(kms, vbl_work->crtc);
kfree(vbl_work);
}
static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
- int crtc_id, bool enable)
+ struct drm_crtc *crtc, bool enable)
{
struct msm_vblank_work *vbl_work;
@@ -180,7 +180,7 @@ static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
INIT_WORK(_work->work, vblank_ctrl_worker);
- vbl_work->crtc_id = crtc_id;
+ vbl_work->crtc = crtc;
vbl_work->enable = enable;
vbl_work->priv = priv;
@@ -354,7 +354,8 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
struct msm_drm_private *priv = dev_get_drvdata(dev);
struct drm_device *ddev;
struct msm_kms *kms;
- int ret, i;
+ struct drm_crtc *crtc;
+ int ret;
if (drm_firmware_drivers_only())
return -ENODEV;
@@ -427,20 +428,23 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
ddev->mode_config.funcs = _config_funcs;
ddev->mode_config.helper_private = _config_helper_funcs;
- for (i = 0; i < priv->num_crtcs; i++) {
+ drm_for_each_crtc(crtc, ddev) {
+ struct msm_drm_thread *ev_thread;
+
/* initialize event thread */
- priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id;
- priv->event_thread[i].dev = ddev;
- priv->event_thread[i].worker = kthread_create_worker(0,
- "crtc_event:%d", priv->event_thread[i].crtc_id);
- if (IS_ERR(priv->event_thread[i].worker)) {
- ret =
RE: [PATCHv2 2/2] i915/display/dp: SDP CRC16 for 128b132b link layer
Any comments? Thanks and Regards, Arun R Murthy > -Original Message- > From: Murthy, Arun R > Sent: Thursday, January 19, 2023 5:17 PM > To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Nikula, > Jani > Cc: Murthy, Arun R > Subject: [PATCHv2 2/2] i915/display/dp: SDP CRC16 for 128b132b link layer > > Enable SDP error detection configuration, this will set CRC16 in 128b/132b > link layer. > For Display version 13 a hardware bit31 in register VIDEO_DIP_CTL is added > to enable/disable SDP CRC applicable for DP2.0 only, but the default value of > this bit will enable CRC16 in 128b/132b hence skipping this write. > Corrective actions on SDP corruption is yet to be defined. > > v2: Moved the CRC enable to link training init(Jani N) > > Signed-off-by: Arun R Murthy > --- > .../gpu/drm/i915/display/intel_dp_link_training.c| 12 > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > index 3d3efcf02011..7064e465423b 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -1453,4 +1453,16 @@ void intel_dp_start_link_train(struct intel_dp > *intel_dp, > > if (!passed) > intel_dp_schedule_fallback_link_training(intel_dp, > crtc_state); > + > + /* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */ > + if (intel_dp_is_uhbr(crtc_state) && passed) > + drm_dp_dpcd_writeb(_dp->aux, > + > DP_SDP_ERROR_DETECTION_CONFIGURATION, > +DP_SDP_CRC16_128B132B_EN); > + /* > + * VIDEO_DIP_CTL register bit 31 should be set to '0' to not > + * disable SDP CRC. This is applicable for Display version 13. > + * Default value of bit 31 is '0' hence discarding the write > + */ > + /* TODO: Corrective actions on SDP corruption yet to be > defined */ > } > -- > 2.25.1
[PATCH 2/2] drm/i915/xehp: Annotate a couple more workaround registers as MCR
GAMSTLB_CTRL and GAMCNTRL_CTRL became multicast/replicated registers on
Xe_HP. They should be defined accordingly and use MCR-aware operations.
These registers have only been used for some dg2/xehpsdv workarounds, so
this fix is mostly just for consistency/future-proofing; even lacking
the MCR annotation, workarounds will always be properly applied in a
multicast manner on these platforms.
Cc: Gustavo Sousa
Fixes: 58bc2453ab8a ("drm/i915: Define multicast registers as a new type")
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 ++--
drivers/gpu/drm/i915/gt/intel_workarounds.c | 16
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 4a37d048b512..a0ebf3fa63ca 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1105,12 +1105,12 @@
#define VEBX_MOD_CTRL MCR_REG(0xcf38)
#define FORCE_MISS_FTLB REG_BIT(3)
-#define GEN12_GAMSTLB_CTRL _MMIO(0xcf4c)
+#define XEHP_GAMSTLB_CTRL MCR_REG(0xcf4c)
#define CONTROL_BLOCK_CLKGATE_DISREG_BIT(12)
#define EGRESS_BLOCK_CLKGATE_DIS REG_BIT(11)
#define TAG_BLOCK_CLKGATE_DISREG_BIT(7)
-#define GEN12_GAMCNTRL_CTRL_MMIO(0xcf54)
+#define XEHP_GAMCNTRL_CTRL MCR_REG(0xcf54)
#define INVALIDATION_BROADCAST_MODE_DIS REG_BIT(12)
#define GLOBAL_INVALIDATION_MODE REG_BIT(2)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 0e7f64bb2860..94eb498f3c2c 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1570,8 +1570,8 @@ xehpsdv_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
wa_mcr_write_or(wal, XEHP_MERT_MOD_CTRL, FORCE_MISS_FTLB);
/* Wa_14014368820:xehpsdv */
- wa_write_or(wal, GEN12_GAMCNTRL_CTRL,
- INVALIDATION_BROADCAST_MODE_DIS | GLOBAL_INVALIDATION_MODE);
+ wa_mcr_write_or(wal, XEHP_GAMCNTRL_CTRL,
+ INVALIDATION_BROADCAST_MODE_DIS |
GLOBAL_INVALIDATION_MODE);
}
static void
@@ -1665,10 +1665,10 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
wa_mcr_write_or(wal, SSMCGCTL9530, RTFUNIT_CLKGATE_DIS);
/* Wa_14010680813:dg2_g10 */
- wa_write_or(wal, GEN12_GAMSTLB_CTRL,
- CONTROL_BLOCK_CLKGATE_DIS |
- EGRESS_BLOCK_CLKGATE_DIS |
- TAG_BLOCK_CLKGATE_DIS);
+ wa_mcr_write_or(wal, XEHP_GAMSTLB_CTRL,
+ CONTROL_BLOCK_CLKGATE_DIS |
+ EGRESS_BLOCK_CLKGATE_DIS |
+ TAG_BLOCK_CLKGATE_DIS);
}
/* Wa_14014830051:dg2 */
@@ -1691,8 +1691,8 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
wa_mcr_write_or(wal, VEBX_MOD_CTRL, FORCE_MISS_FTLB);
/* Wa_1509235366:dg2 */
- wa_write_or(wal, GEN12_GAMCNTRL_CTRL,
- INVALIDATION_BROADCAST_MODE_DIS | GLOBAL_INVALIDATION_MODE);
+ wa_mcr_write_or(wal, XEHP_GAMCNTRL_CTRL,
+ INVALIDATION_BROADCAST_MODE_DIS |
GLOBAL_INVALIDATION_MODE);
}
static void
--
2.39.0
[PATCH 1/2] drm/i915/xehp: GAM registers don't need to be re-applied on engine resets
Register reset characteristics (i.e., whether the register maintains or
loses its value on engine reset) is an important factor that determines
which wa_list we want to add workarounds to. We recently found out that
the bspec documentation for the Xe_HP's "GAM" registers in the 0xC800 -
0xCFFF range was misleading; these registers do not actually lose their
value on engine resets as the documentation implied. This means there's
no need to re-apply workarounds touching these registers after a reset,
and the corresponding workarounds should be moved from the 'engine'
lists back to the 'gt' list.
While moving these GAM-related workarounds to the various platforms' GT
workaround functions, we should also take care to handle Wa_18018781329
properly for MTL's two GTs --- the render/compute setting should be set
on the primary GT where those engines reside, and the vd/ve/gsc setting
should be set on the media GT. Previously the VD/VE/GSC setting was not
being properly applied.
Cc: Gustavo Sousa
Fixes: edf176f48d87 ("drm/i915/dg2: Move misplaced 'ctx' & 'gt' wa's to engine
wa list")
Fixes: b2006061ae28 ("drm/i915/xehpsdv: Move render/compute engine reset
domains related workarounds")
Fixes: 41bb543f5598 ("drm/i915/mtl: Add initial gt workarounds")
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/gt/intel_workarounds.c | 88 +
drivers/gpu/drm/i915/i915_drv.h | 4 +
3 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 2727645864db..4a37d048b512 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1100,6 +1100,7 @@
#define XEHP_MERT_MOD_CTRL MCR_REG(0xcf28)
#define RENDER_MOD_CTRLMCR_REG(0xcf2c)
#define COMP_MOD_CTRL MCR_REG(0xcf30)
+#define GSC_MOD_CTRL MCR_REG(0xcf30) /* media GT
only */
#define VDBX_MOD_CTRL MCR_REG(0xcf34)
#define VEBX_MOD_CTRL MCR_REG(0xcf38)
#define FORCE_MISS_FTLB REG_BIT(3)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 4efc1a532982..0e7f64bb2860 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1559,6 +1559,19 @@ xehpsdv_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
/* Wa_14011060649:xehpsdv */
wa_14011060649(gt, wal);
+
+ /* Wa_18018781329 */
+ wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
+ wa_mcr_write_or(wal, COMP_MOD_CTRL, FORCE_MISS_FTLB);
+ wa_mcr_write_or(wal, VDBX_MOD_CTRL, FORCE_MISS_FTLB);
+ wa_mcr_write_or(wal, VEBX_MOD_CTRL, FORCE_MISS_FTLB);
+
+ /* Wa_14012362059:xehpsdv */
+ wa_mcr_write_or(wal, XEHP_MERT_MOD_CTRL, FORCE_MISS_FTLB);
+
+ /* Wa_14014368820:xehpsdv */
+ wa_write_or(wal, GEN12_GAMCNTRL_CTRL,
+ INVALIDATION_BROADCAST_MODE_DIS | GLOBAL_INVALIDATION_MODE);
}
static void
@@ -1599,6 +1612,12 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
DSS_ROUTER_CLKGATE_DIS);
}
+ if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_B0) ||
+ IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)) {
+ /* Wa_14012362059:dg2 */
+ wa_mcr_write_or(wal, XEHP_MERT_MOD_CTRL, FORCE_MISS_FTLB);
+ }
+
if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_B0)) {
/* Wa_14010948348:dg2_g10 */
wa_write_or(wal, UNSLCGCTL9430, MSQDUNIT_CLKGATE_DIS);
@@ -1644,6 +1663,12 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
/* Wa_14011028019:dg2_g10 */
wa_mcr_write_or(wal, SSMCGCTL9530, RTFUNIT_CLKGATE_DIS);
+
+ /* Wa_14010680813:dg2_g10 */
+ wa_write_or(wal, GEN12_GAMSTLB_CTRL,
+ CONTROL_BLOCK_CLKGATE_DIS |
+ EGRESS_BLOCK_CLKGATE_DIS |
+ TAG_BLOCK_CLKGATE_DIS);
}
/* Wa_14014830051:dg2 */
@@ -1658,6 +1683,16 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
/* Wa_14015795083 */
wa_mcr_write_clr(wal, GEN8_MISCCPCTL,
GEN12_DOP_CLOCK_GATE_RENDER_ENABLE);
+
+ /* Wa_18018781329 */
+ wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
+ wa_mcr_write_or(wal, COMP_MOD_CTRL, FORCE_MISS_FTLB);
+ wa_mcr_write_or(wal, VDBX_MOD_CTRL, FORCE_MISS_FTLB);
+ wa_mcr_write_or(wal, VEBX_MOD_CTRL, FORCE_MISS_FTLB);
+
+ /* Wa_1509235366:dg2 */
+ wa_write_or(wal, GEN12_GAMCNTRL_CTRL,
+ INVALIDATION_BROADCAST_MODE_DIS |
Re: [PATCH 2/2] drm/i915/guc: Fix missing return code checks in submission init
On 1/11/2023 5:54 PM, john.c.harri...@intel.com wrote:
From: John Harrison
The CI results for the 'fast request' patch set (enables error return
codes for fire-and-forget H2G messages) hit an issue with the KMD
sending context submission requests on an invalid context. That was
caused by a fault injection probe failing the context creation of a
kernel context. However, there was no return code checking on any of
the kernel context registration paths. So the driver kept going and
tried to use the kernel context for the record defaults process.
This would not cause any actual problems. The invalid requests would
be rejected by GuC and ultimately the start up sequence would
correctly wedge due to the context creation failure. But fixing the
issue correctly rather ignoring it means we won't get CI complaining
when the fast request patch lands and enables the extra error checking.
So fix it by checking for errors and aborting as appropriate when
creating kernel contexts. While at it, clean up some other submission
init related failure cleanup paths. Also, rename guc_init_lrc_mapping
to guc_init_submission as the former name hasn't been valid in a long
time.
Signed-off-by: John Harrison
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 91 ++-
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 7 +-
3 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 982364777d0c6..dd856fd92945b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1431,7 +1431,7 @@ static int guc_action_enable_usage_stats(struct intel_guc
*guc)
return intel_guc_send(guc, action, ARRAY_SIZE(action));
}
-static void guc_init_engine_stats(struct intel_guc *guc)
+static int guc_init_engine_stats(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
intel_wakeref_t wakeref;
@@ -1447,6 +1447,8 @@ static void guc_init_engine_stats(struct intel_guc *guc)
cancel_delayed_work_sync(>timestamp.work);
drm_err(>i915->drm, "Failed to enable usage stats: %d!\n",
ret);
}
+
+ return ret;
}
static void guc_park_engine_stats(struct intel_guc *guc)
@@ -4108,9 +4110,11 @@ static void guc_set_default_submission(struct
intel_engine_cs *engine)
engine->submit_request = guc_submit_request;
}
-static inline void guc_kernel_context_pin(struct intel_guc *guc,
- struct intel_context *ce)
+static inline int guc_kernel_context_pin(struct intel_guc *guc,
+struct intel_context *ce)
{
+ int ret;
+
/*
* Note: we purposefully do not check the returns below because
* the registration can only fail if a reset is just starting.
@@ -4118,16 +4122,24 @@ static inline void guc_kernel_context_pin(struct
intel_guc *guc,
* isn't happening and even it did this code would be run again.
*/
- if (context_guc_id_invalid(ce))
- pin_guc_id(guc, ce);
+ if (context_guc_id_invalid(ce)) {
+ int ret = pin_guc_id(guc, ce);
Why do you need a local ret variable inside this if statement, when you
already have a function-level one? or is it just a cut & paste error?
+
+ if (ret < 0)
+ return ret;
+ }
if (!test_bit(CONTEXT_GUC_INIT, >flags))
guc_context_init(ce);
- try_context_registration(ce, true);
+ ret = try_context_registration(ce, true);
+ if (ret)
+ unpin_guc_id(guc, ce);
+
+ return ret;
}
-static inline void guc_init_lrc_mapping(struct intel_guc *guc)
+static inline int guc_init_submission(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
struct intel_engine_cs *engine;
@@ -4154,9 +4166,17 @@ static inline void guc_init_lrc_mapping(struct intel_guc
*guc)
struct intel_context *ce;
list_for_each_entry(ce, >pinned_contexts_list,
- pinned_contexts_link)
- guc_kernel_context_pin(guc, ce);
+ pinned_contexts_link) {
+ int ret = guc_kernel_context_pin(guc, ce);
+
+ if (ret) {
+ /* No point in trying to clean up as i915 will
wedge on failure */
+ return ret;
+ }
+ }
}
+
+ return 0;
}
static void guc_release(struct intel_engine_cs *engine)
@@ -4400,30 +4420,57 @@ static int guc_init_global_schedule_policy(struct
intel_guc *guc)
return ret;
}
-void intel_guc_submission_enable(struct intel_guc *guc)
+static void
Re: [Intel-gfx] [PATCH v4 3/7] drm/i915: Allow error capture without a request
On 1/24/2023 16:39, Ceraolo Spurio, Daniele wrote:
On 1/20/2023 3:28 PM, john.c.harri...@intel.com wrote:
From: John Harrison
There was a report of error captures occurring without any hung
context being indicated despite the capture being initiated by a 'hung
context notification' from GuC. The problem was not reproducible.
However, it is possible to happen if the context in question has no
active requests. For example, if the hang was in the context switch
itself then the breadcrumb write would have occurred and the KMD would
see an idle context.
In the interests of attempting to provide as much information as
possible about a hang, it seems wise to include the engine info
regardless of whether a request was found or not. As opposed to just
prentending there was no hang at all.
So update the error capture code to always record engine information
if a context is given. Which means updating record_context() to take a
context instead of a request (which it only ever used to find the
context anyway). And split the request agnostic parts of
intel_engine_coredump_add_request() out into a seaprate function.
v2: Remove a duplicate 'if' statement (Umesh) and fix a put of a null
pointer.
v3: Tidy up request locking code flow (Tvrtko)
v4: Pull in improved info message from next patch and fix up potential
leak of GuC register state (Daniele)
In the very unlikely case that the capture fails, we're leaving the
data inside the GuC buffer. This is not new with this patch and not a
bug (that buffer is a ring and the stale data will be overwritten if
it gets full), but maybe something that can be improved as a follow-up.
Correct and correct.
John.
Reviewed-by: Daniele Ceraolo Spurio
Daniele
Signed-off-by: John Harrison
Reviewed-by: Umesh Nerlige Ramappa
Acked-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gpu_error.c | 74 ++-
1 file changed, 50 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index b20bd6365615b..225f1b11a6b93 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1370,14 +1370,14 @@ static void engine_record_execlists(struct
intel_engine_coredump *ee)
}
static bool record_context(struct i915_gem_context_coredump *e,
- const struct i915_request *rq)
+ struct intel_context *ce)
{
struct i915_gem_context *ctx;
struct task_struct *task;
bool simulated;
rcu_read_lock();
- ctx = rcu_dereference(rq->context->gem_context);
+ ctx = rcu_dereference(ce->gem_context);
if (ctx && !kref_get_unless_zero(>ref))
ctx = NULL;
rcu_read_unlock();
@@ -1396,8 +1396,8 @@ static bool record_context(struct
i915_gem_context_coredump *e,
e->guilty = atomic_read(>guilty_count);
e->active = atomic_read(>active_count);
- e->total_runtime =
intel_context_get_total_runtime_ns(rq->context);
- e->avg_runtime = intel_context_get_avg_runtime_ns(rq->context);
+ e->total_runtime = intel_context_get_total_runtime_ns(ce);
+ e->avg_runtime = intel_context_get_avg_runtime_ns(ce);
simulated = i915_gem_context_no_error_capture(ctx);
@@ -1532,15 +1532,37 @@ intel_engine_coredump_alloc(struct
intel_engine_cs *engine, gfp_t gfp, u32 dump_
return ee;
}
+static struct intel_engine_capture_vma *
+engine_coredump_add_context(struct intel_engine_coredump *ee,
+ struct intel_context *ce,
+ gfp_t gfp)
+{
+ struct intel_engine_capture_vma *vma = NULL;
+
+ ee->simulated |= record_context(>context, ce);
+ if (ee->simulated)
+ return NULL;
+
+ /*
+ * We need to copy these to an anonymous buffer
+ * as the simplest method to avoid being overwritten
+ * by userspace.
+ */
+ vma = capture_vma(vma, ce->ring->vma, "ring", gfp);
+ vma = capture_vma(vma, ce->state, "HW context", gfp);
+
+ return vma;
+}
+
struct intel_engine_capture_vma *
intel_engine_coredump_add_request(struct intel_engine_coredump *ee,
struct i915_request *rq,
gfp_t gfp)
{
- struct intel_engine_capture_vma *vma = NULL;
+ struct intel_engine_capture_vma *vma;
- ee->simulated |= record_context(>context, rq);
- if (ee->simulated)
+ vma = engine_coredump_add_context(ee, rq->context, gfp);
+ if (!vma)
return NULL;
/*
@@ -1550,8 +1572,6 @@ intel_engine_coredump_add_request(struct
intel_engine_coredump *ee,
*/
vma = capture_vma_snapshot(vma, rq->batch_res, gfp, "batch");
vma = capture_user(vma, rq, gfp);
- vma = capture_vma(vma, rq->ring->vma, "ring", gfp);
- vma = capture_vma(vma, rq->context->state, "HW context", gfp);
ee->rq_head = rq->head;
ee->rq_post = rq->postfix;
@@ -1604,25 +1624,31 @@ capture_engine(struct intel_engine_cs *engine,
return NULL;
Re: [Intel-gfx] [PATCH 1/2] drm/i915/guc: Improve clean up of busyness stats worker
On 1/11/2023 5:54 PM, john.c.harri...@intel.com wrote:
From: John Harrison
The stats worker thread management was mis-matched between
enable/disable call sites. Fix those up. Also, abstract the cancel
code into a helper function rather than replicating in multiple places.
Signed-off-by: John Harrison
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 ---
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index b436dd7f12e42..982364777d0c6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1435,19 +1435,25 @@ static void guc_init_engine_stats(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
intel_wakeref_t wakeref;
+ int ret;
mod_delayed_work(system_highpri_wq, >timestamp.work,
guc->timestamp.ping_delay);
- with_intel_runtime_pm(>i915->runtime_pm, wakeref) {
- int ret = guc_action_enable_usage_stats(guc);
+ with_intel_runtime_pm(>i915->runtime_pm, wakeref)
+ ret = guc_action_enable_usage_stats(guc);
- if (ret)
- drm_err(>i915->drm,
- "Failed to enable usage stats: %d!\n", ret);
+ if (ret) {
+ cancel_delayed_work_sync(>timestamp.work);
Wouldn't it be easier to just call mod_delayed_work after the H2G if
ret==0, instead of having it before and cancelling if we get a failure?
+ drm_err(>i915->drm, "Failed to enable usage stats: %d!\n",
ret);
}
}
+static void guc_park_engine_stats(struct intel_guc *guc)
+{
+ cancel_delayed_work_sync(>timestamp.work);
+}
+
Now you're asymmetric with the park/unpark, because on the park side you
have this wrapper, while on the unpark side you directly call
mod_delayed_work.
Daniele
void intel_guc_busyness_park(struct intel_gt *gt)
{
struct intel_guc *guc = >uc.guc;
@@ -1460,7 +1466,7 @@ void intel_guc_busyness_park(struct intel_gt *gt)
* and causes an unclaimed register access warning. Cancel the worker
* synchronously here.
*/
- cancel_delayed_work_sync(>timestamp.work);
+ guc_park_engine_stats(guc);
/*
* Before parking, we should sample engine busyness stats if we need to.
@@ -4409,11 +4415,11 @@ void intel_guc_submission_enable(struct intel_guc *guc)
guc_init_global_schedule_policy(guc);
}
+/* Note: By the time we're here, GuC may have already been reset */
void intel_guc_submission_disable(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
-
- /* Note: By the time we're here, GuC may have already been reset */
+ guc_park_engine_stats(guc);
/* Disable and route to host */
if (GRAPHICS_VER(gt->i915) >= 12)
[PATCH] drm/i915/guc: Fix missing ecodes
From: John Harrison
Error captures are tagged with an 'ecode'. This is a pseduo-unique magic
number that is meant to distinguish similar seeming bugs with
different underlying signatures. It is a combination of two ring state
registers. Unfortunately, the register state being used is only valid
in execlist mode. In GuC mode, the register state exists in a separate
list of arbitrary register address/value pairs rather than the named
entry structure. So, search through that list to find the two exciting
registers and copy them over to the structure's named members.
Signed-off-by: John Harrison
Fixes: a6f0f9cf330a ("drm/i915/guc: Plumb GuC-capture into gpu_coredump")
Cc: Alan Previn
Cc: Umesh Nerlige Ramappa
Cc: Lucas De Marchi
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: Matt Roper
Cc: Aravind Iddamsetty
Cc: Michael Cheng
Cc: Matthew Brost
Cc: Bruce Chang
Cc: Daniele Ceraolo Spurio
Cc: Matthew Auld
---
.../gpu/drm/i915/gt/uc/intel_guc_capture.c| 22 +++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 1c1b85073b4bd..4e0b06ceed96d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -1571,6 +1571,27 @@ int intel_guc_capture_print_engine_node(struct
drm_i915_error_state_buf *ebuf,
#endif //CONFIG_DRM_I915_CAPTURE_ERROR
+static void guc_capture_find_ecode(struct intel_engine_coredump *ee)
+{
+ struct gcap_reg_list_info *reginfo;
+ struct guc_mmio_reg *regs;
+ i915_reg_t reg_ipehr = RING_IPEHR(0);
+ i915_reg_t reg_instdone = RING_INSTDONE(0);
+ int i;
+
+ if (!ee->guc_capture_node)
+ return;
+
+ reginfo = ee->guc_capture_node->reginfo +
GUC_CAPTURE_LIST_TYPE_ENGINE_INSTANCE;
+ regs = reginfo->regs;
+ for (i = 0; i < reginfo->num_regs; i++) {
+ if (regs[i].offset == reg_ipehr.reg)
+ ee->ipehr = regs[i].value;
+ if (regs[i].offset == reg_instdone.reg)
+ ee->instdone.instdone = regs[i].value;
+ }
+}
+
void intel_guc_capture_free_node(struct intel_engine_coredump *ee)
{
if (!ee || !ee->guc_capture_node)
@@ -1612,6 +1633,7 @@ void intel_guc_capture_get_matching_node(struct intel_gt
*gt,
list_del(>link);
ee->guc_capture_node = n;
ee->capture = guc->capture;
+ guc_capture_find_ecode(ee);
return;
}
}
--
2.39.1
Re: [PATCH v4] drm/i915/uncore: Use GT message helpers in uncore
This is actually v1 not v4. Not sure how that crept in!
John.
On 1/24/2023 12:54, john.c.harri...@intel.com wrote:
From: John Harrison
Uncore is really part of the GT. So use the GT specific debug/error
message helpers so as to get the GT index in the prints.
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/intel_uncore.c | 133 +---
1 file changed, 63 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 8dee9e62a73ee..4e357477c6592 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -25,6 +25,7 @@
#include
#include "gt/intel_engine_regs.h"
+#include "gt/intel_gt_print.h"
#include "gt/intel_gt_regs.h"
#include "i915_drv.h"
@@ -83,8 +84,7 @@ static void mmio_debug_resume(struct intel_uncore *uncore)
uncore->debug->unclaimed_mmio_check =
uncore->debug->saved_mmio_check;
if (check_for_unclaimed_mmio(uncore))
- drm_info(>i915->drm,
-"Invalid mmio detected during user access\n");
+ gt_info(uncore->gt, "Invalid mmio detected during user
access\n");
spin_unlock(>debug->lock);
}
@@ -179,9 +179,9 @@ static inline void
fw_domain_wait_ack_clear(const struct intel_uncore_forcewake_domain *d)
{
if (wait_ack_clear(d, FORCEWAKE_KERNEL)) {
- drm_err(>uncore->i915->drm,
- "%s: timed out waiting for forcewake ack to clear.\n",
- intel_uncore_forcewake_domain_to_str(d->id));
+ gt_err(d->uncore->gt,
+ "%s: timed out waiting for forcewake ack to clear.\n",
+ intel_uncore_forcewake_domain_to_str(d->id));
add_taint_for_CI(d->uncore->i915, TAINT_WARN); /* CI now
unreliable */
}
}
@@ -228,12 +228,11 @@ fw_domain_wait_ack_with_fallback(const struct
intel_uncore_forcewake_domain *d,
fw_clear(d, FORCEWAKE_KERNEL_FALLBACK);
} while (!ack_detected && pass++ < 10);
- drm_dbg(>uncore->i915->drm,
- "%s had to use fallback to %s ack, 0x%x (passes %u)\n",
- intel_uncore_forcewake_domain_to_str(d->id),
- type == ACK_SET ? "set" : "clear",
- fw_ack(d),
- pass);
+ gt_dbg(d->uncore->gt, "%s had to use fallback to %s ack, 0x%x (passes
%u)\n",
+ intel_uncore_forcewake_domain_to_str(d->id),
+ type == ACK_SET ? "set" : "clear",
+ fw_ack(d),
+ pass);
return ack_detected ? 0 : -ETIMEDOUT;
}
@@ -258,9 +257,8 @@ static inline void
fw_domain_wait_ack_set(const struct intel_uncore_forcewake_domain *d)
{
if (wait_ack_set(d, FORCEWAKE_KERNEL)) {
- drm_err(>uncore->i915->drm,
- "%s: timed out waiting for forcewake ack request.\n",
- intel_uncore_forcewake_domain_to_str(d->id));
+ gt_err(d->uncore->gt, "%s: timed out waiting for forcewake ack
request.\n",
+ intel_uncore_forcewake_domain_to_str(d->id));
add_taint_for_CI(d->uncore->i915, TAINT_WARN); /* CI now
unreliable */
}
}
@@ -366,9 +364,9 @@ static void __gen6_gt_wait_for_thread_c0(struct
intel_uncore *uncore)
* w/a for a sporadic read returning 0 by waiting for the GT
* thread to wake up.
*/
- drm_WARN_ONCE(>i915->drm,
- wait_for_atomic_us(gt_thread_status(uncore) == 0, 5000),
- "GT thread status wait timed out\n");
+ gt_WARN_ONCE(uncore->gt,
+wait_for_atomic_us(gt_thread_status(uncore) == 0, 5000),
+"GT thread status wait timed out\n");
}
static void fw_domains_get_with_thread_status(struct intel_uncore *uncore,
@@ -402,8 +400,7 @@ static void __gen6_gt_wait_for_fifo(struct intel_uncore
*uncore)
if (wait_for_atomic((n = fifo_free_entries(uncore)) >
GT_FIFO_NUM_RESERVED_ENTRIES,
GT_FIFO_TIMEOUT_MS)) {
- drm_dbg(>i915->drm,
- "GT_FIFO timeout, entries: %u\n", n);
+ gt_dbg(uncore->gt, "GT_FIFO timeout, entries: %u\n", n);
return;
}
}
@@ -476,7 +473,7 @@ intel_uncore_forcewake_reset(struct intel_uncore *uncore)
break;
if (--retry_count == 0) {
- drm_err(>i915->drm, "Timed out waiting for forcewake
timers to finish\n");
+ gt_err(uncore->gt, "Timed out waiting for forcewake timers
to finish\n");
break;
}
@@ -484,7 +481,7 @@ intel_uncore_forcewake_reset(struct intel_uncore *uncore)
cond_resched();
}
- drm_WARN_ON(>i915->drm,
Re: [Intel-gfx] [PATCH v4 7/7] drm/i915/guc: Rename GuC register state capture node to be more obvious
On 1/20/2023 3:28 PM, john.c.harri...@intel.com wrote:
From: John Harrison
The GuC specific register state entry in the error capture object was
just called 'capture'. Although the companion 'node' entry was called
'guc_capture_node'. Rename the base entry to be 'guc_capture' instead
so that it is a) more consistent and b) more obvious what it is.
Signed-off-by: John Harrison
Reviewed-by: Daniele Ceraolo Spurio
Daniele
---
drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 8
drivers/gpu/drm/i915/i915_gpu_error.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 1c1b85073b4bd..fc3b994626a4f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -1506,7 +1506,7 @@ int intel_guc_capture_print_engine_node(struct
drm_i915_error_state_buf *ebuf,
if (!ebuf || !ee)
return -EINVAL;
- cap = ee->capture;
+ cap = ee->guc_capture;
if (!cap || !ee->engine)
return -ENODEV;
@@ -1576,8 +1576,8 @@ void intel_guc_capture_free_node(struct intel_engine_coredump *ee)
if (!ee || !ee->guc_capture_node)
return;
- guc_capture_add_node_to_cachelist(ee->capture, ee->guc_capture_node);
- ee->capture = NULL;
+ guc_capture_add_node_to_cachelist(ee->guc_capture,
ee->guc_capture_node);
+ ee->guc_capture = NULL;
ee->guc_capture_node = NULL;
}
@@ -1611,7 +1611,7 @@ void intel_guc_capture_get_matching_node(struct intel_gt *gt,
(ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
list_del(>link);
ee->guc_capture_node = n;
- ee->capture = guc->capture;
+ ee->guc_capture = guc->capture;
return;
}
}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h
b/drivers/gpu/drm/i915/i915_gpu_error.h
index efc75cc2ffdb9..56027ffbce51f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -94,7 +94,7 @@ struct intel_engine_coredump {
struct intel_instdone instdone;
/* GuC matched capture-lists info */
- struct intel_guc_state_capture *capture;
+ struct intel_guc_state_capture *guc_capture;
struct __guc_capture_parsed_output *guc_capture_node;
struct i915_gem_context_coredump {
Re: [Intel-gfx] [PATCH v4 3/7] drm/i915: Allow error capture without a request
On 1/20/2023 3:28 PM, john.c.harri...@intel.com wrote:
From: John Harrison
There was a report of error captures occurring without any hung
context being indicated despite the capture being initiated by a 'hung
context notification' from GuC. The problem was not reproducible.
However, it is possible to happen if the context in question has no
active requests. For example, if the hang was in the context switch
itself then the breadcrumb write would have occurred and the KMD would
see an idle context.
In the interests of attempting to provide as much information as
possible about a hang, it seems wise to include the engine info
regardless of whether a request was found or not. As opposed to just
prentending there was no hang at all.
So update the error capture code to always record engine information
if a context is given. Which means updating record_context() to take a
context instead of a request (which it only ever used to find the
context anyway). And split the request agnostic parts of
intel_engine_coredump_add_request() out into a seaprate function.
v2: Remove a duplicate 'if' statement (Umesh) and fix a put of a null
pointer.
v3: Tidy up request locking code flow (Tvrtko)
v4: Pull in improved info message from next patch and fix up potential
leak of GuC register state (Daniele)
In the very unlikely case that the capture fails, we're leaving the data
inside the GuC buffer. This is not new with this patch and not a bug
(that buffer is a ring and the stale data will be overwritten if it gets
full), but maybe something that can be improved as a follow-up.
Reviewed-by: Daniele Ceraolo Spurio
Daniele
Signed-off-by: John Harrison
Reviewed-by: Umesh Nerlige Ramappa
Acked-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gpu_error.c | 74 ++-
1 file changed, 50 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index b20bd6365615b..225f1b11a6b93 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1370,14 +1370,14 @@ static void engine_record_execlists(struct
intel_engine_coredump *ee)
}
static bool record_context(struct i915_gem_context_coredump *e,
- const struct i915_request *rq)
+ struct intel_context *ce)
{
struct i915_gem_context *ctx;
struct task_struct *task;
bool simulated;
rcu_read_lock();
- ctx = rcu_dereference(rq->context->gem_context);
+ ctx = rcu_dereference(ce->gem_context);
if (ctx && !kref_get_unless_zero(>ref))
ctx = NULL;
rcu_read_unlock();
@@ -1396,8 +1396,8 @@ static bool record_context(struct
i915_gem_context_coredump *e,
e->guilty = atomic_read(>guilty_count);
e->active = atomic_read(>active_count);
- e->total_runtime = intel_context_get_total_runtime_ns(rq->context);
- e->avg_runtime = intel_context_get_avg_runtime_ns(rq->context);
+ e->total_runtime = intel_context_get_total_runtime_ns(ce);
+ e->avg_runtime = intel_context_get_avg_runtime_ns(ce);
simulated = i915_gem_context_no_error_capture(ctx);
@@ -1532,15 +1532,37 @@ intel_engine_coredump_alloc(struct intel_engine_cs *engine, gfp_t gfp, u32 dump_
return ee;
}
+static struct intel_engine_capture_vma *
+engine_coredump_add_context(struct intel_engine_coredump *ee,
+ struct intel_context *ce,
+ gfp_t gfp)
+{
+ struct intel_engine_capture_vma *vma = NULL;
+
+ ee->simulated |= record_context(>context, ce);
+ if (ee->simulated)
+ return NULL;
+
+ /*
+* We need to copy these to an anonymous buffer
+* as the simplest method to avoid being overwritten
+* by userspace.
+*/
+ vma = capture_vma(vma, ce->ring->vma, "ring", gfp);
+ vma = capture_vma(vma, ce->state, "HW context", gfp);
+
+ return vma;
+}
+
struct intel_engine_capture_vma *
intel_engine_coredump_add_request(struct intel_engine_coredump *ee,
struct i915_request *rq,
gfp_t gfp)
{
- struct intel_engine_capture_vma *vma = NULL;
+ struct intel_engine_capture_vma *vma;
- ee->simulated |= record_context(>context, rq);
- if (ee->simulated)
+ vma = engine_coredump_add_context(ee, rq->context, gfp);
+ if (!vma)
return NULL;
/*
@@ -1550,8 +1572,6 @@ intel_engine_coredump_add_request(struct
intel_engine_coredump *ee,
*/
vma = capture_vma_snapshot(vma, rq->batch_res, gfp, "batch");
vma = capture_user(vma, rq, gfp);
- vma = capture_vma(vma, rq->ring->vma, "ring", gfp);
- vma = capture_vma(vma, rq->context->state, "HW context", gfp);
ee->rq_head = rq->head;
ee->rq_post = rq->postfix;
@@ -1604,25 +1624,31 @@
Re: [Intel-gfx] [PATCH v4 2/7] drm/i915: Fix up locking around dumping requests lists
On 1/20/2023 3:28 PM, john.c.harri...@intel.com wrote:
From: John Harrison
The debugfs dump of requests was confused about what state requires
the execlist lock versus the GuC lock. There was also a bunch of
duplicated messy code between it and the error capture code.
So refactor the hung request search into a re-usable function. And
reduce the span of the execlist state lock to only the execlist
specific code paths. In order to do that, also move the report of hold
count (which is an execlist only concept) from the top level dump
function to the lower level execlist specific function. Also, move the
execlist specific code into the execlist source file.
v2: Rename some functions and move to more appropriate files (Daniele).
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/gt/intel_engine.h| 4 +-
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 74 +--
.../drm/i915/gt/intel_execlists_submission.c | 27 +++
.../drm/i915/gt/intel_execlists_submission.h | 4 +
drivers/gpu/drm/i915/i915_gpu_error.c | 26 +--
5 files changed, 73 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
b/drivers/gpu/drm/i915/gt/intel_engine.h
index 0e24af5efee9c..b58c30ac8ef02 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -250,8 +250,8 @@ void intel_engine_dump_active_requests(struct list_head
*requests,
ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine,
ktime_t *now);
-struct i915_request *
-intel_engine_execlist_find_hung_request(struct intel_engine_cs *engine);
+void intel_engine_get_hung_entity(struct intel_engine_cs *engine,
+ struct intel_context **ce, struct
i915_request **rq);
u32 intel_engine_context_size(struct intel_gt *gt, u8 class);
struct intel_context *
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index fbc0a81617e89..1d77e27801bce 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -2114,17 +2114,6 @@ static void print_request_ring(struct drm_printer *m,
struct i915_request *rq)
}
}
-static unsigned long list_count(struct list_head *list)
-{
- struct list_head *pos;
- unsigned long count = 0;
-
- list_for_each(pos, list)
- count++;
-
- return count;
-}
-
static unsigned long read_ul(void *p, size_t x)
{
return *(unsigned long *)(p + x);
@@ -2216,11 +2205,11 @@ void intel_engine_dump_active_requests(struct list_head
*requests,
}
}
-static void engine_dump_active_requests(struct intel_engine_cs *engine, struct drm_printer *m)
+static void engine_dump_active_requests(struct intel_engine_cs *engine,
+ struct drm_printer *m)
{
+ struct intel_context *hung_ce = NULL;
struct i915_request *hung_rq = NULL;
- struct intel_context *ce;
- bool guc;
/*
* No need for an engine->irq_seqno_barrier() before the seqno reads.
@@ -2229,29 +2218,20 @@ static void engine_dump_active_requests(struct
intel_engine_cs *engine, struct d
* But the intention here is just to report an instantaneous snapshot
* so that's fine.
*/
- lockdep_assert_held(>sched_engine->lock);
+ intel_engine_get_hung_entity(engine, _ce, _rq);
drm_printf(m, "\tRequests:\n");
- guc = intel_uc_uses_guc_submission(>gt->uc);
- if (guc) {
- ce = intel_engine_get_hung_context(engine);
- if (ce)
- hung_rq = intel_context_find_active_request_get(ce);
- } else {
- hung_rq = intel_engine_execlist_find_hung_request(engine);
- if (hung_rq)
- hung_rq = i915_request_get_rcu(hung_rq);
- }
-
if (hung_rq)
engine_dump_request(hung_rq, m, "\t\thung");
+ else if (hung_ce)
+ drm_printf(m, "\t\tGot hung ce but no hung rq!\n");
- if (guc)
+ if (intel_uc_uses_guc_submission(>gt->uc))
intel_guc_dump_active_requests(engine, hung_rq, m);
else
-
intel_engine_dump_active_requests(>sched_engine->requests,
- hung_rq, m);
+ intel_execlist_dump_active_requests(engine, hung_rq, m);
+
if (hung_rq)
i915_request_put(hung_rq);
}
@@ -2263,7 +2243,6 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct i915_gpu_error * const error = >i915->gpu_error;
struct i915_request *rq;
intel_wakeref_t wakeref;
- unsigned long flags;
ktime_t dummy;
if (header) {
@@ -2300,13 +2279,8 @@ void intel_engine_dump(struct intel_engine_cs *engine,
i915_reset_count(error));
Re: [PATCH v1 00/14] add display port DSC feature
Hi Dmitry / Marijn I have seen your review comments and agree there is much work to be done to get this in a better shape. We will post a better V2 to address the concerns. Would appreciate some patience till then. Thanks for your support in reviews as always Abhinav On 1/23/2023 10:24 AM, Kuogee Hsieh wrote: This patch add DSC related supporting functions into to both dp controller and dpu enccoder Kuogee Hsieh (14): drm/msm/dp: add dpcd read of both dsc and fec capability drm/msm/dp: add dsc factor into calculation of supported bpp drm/msm/dp: add configure mainlink_levels base on lane number drm/msm/dp: correct configure Colorimetry Indicator Field at MISC0 drm/msm/dp: upgrade tu calculation base on newest algorithm drm/msm/dp: add display compression related struct drm/msm/dp: add dsc helper functions drm/msm/dp: add dsc supporting functions to DP controller drm/msm/dsi: export struct msm_compression_info to dpu encoder drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine drm/msm/disp/dpu1: add supports of new flush mechanism drm/msm/disp/dpu1: revise timing engine programming to work for DSC drm/msm/disp/dpu1: add dsc supporting functions to dpu encoder drm/msm/disp/dpu1: add sc7280 dsc block and sub block drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c | 537 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h | 25 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 341 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 74 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 371 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 132 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h| 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 +- drivers/gpu/drm/msm/dp/dp_catalog.c| 176 - drivers/gpu/drm/msm/dp/dp_catalog.h| 97 ++- drivers/gpu/drm/msm/dp/dp_ctrl.c | 839 ++--- drivers/gpu/drm/msm/dp/dp_display.c| 61 +- drivers/gpu/drm/msm/dp/dp_link.c | 29 +- drivers/gpu/drm/msm/dp/dp_panel.c | 749 +- drivers/gpu/drm/msm/dp/dp_panel.h | 67 +- drivers/gpu/drm/msm/dp/dp_reg.h| 40 +- drivers/gpu/drm/msm/dsi/dsi.c | 3 +- drivers/gpu/drm/msm/dsi/dsi.h | 3 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 14 +- drivers/gpu/drm/msm/msm_drv.h | 113 ++- 32 files changed, 3429 insertions(+), 497 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
Re: [PATCH] drm/msm/dpu: disable features unsupported by QCM2290
On 1/24/2023 12:22 AM, Dmitry Baryshkov wrote:
On 24/01/2023 03:32, Abhinav Kumar wrote:
On 1/22/2023 11:11 PM, Dmitry Baryshkov wrote:
QCM2290 doesn't seem to support reg-dma, smart-dma, UBWC, CDP, exclusion
rectangles and CSC. Drop corresponding features being incorrectly
enabled for qcm2290.
Can you please point me to which vendor DT you are referring to for this?
CSC is supported on the VIG SSPPs from what I can see.
https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi
No CSC, smart-dma, excl-rect, CDP, etc.
Sorry I am missing something here.
It has one Vig and one DMA
https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi#L68
If Vig is present, CSC is supported.
Even for smart DMA I can see it supported
https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi#L76
on the DMA SSPP.
Same for excl rectangle too
https://github.com/MiCode/kernel_devicetree/blob/psyche-r-oss/qcom/scuba-sde.dtsi#L74
QCM2290 should be using the same MDP version as 6115 from the HW version.
It is 6.3 vs 6.5 if I remember correctly.
Cc: Loic Poulain
Fixes: 5334087ee743 ("drm/msm: add support for QCM2290 MDSS")
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 20 +++
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 289fb11f99d1..1c3ffa922794 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -12,10 +12,14 @@
#include "dpu_hw_catalog.h"
#include "dpu_kms.h"
-#define VIG_MASK \
+#define VIG_BASE_MASK \
(BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |\
+ BIT(DPU_SSPP_TS_PREFILL))
+
+#define VIG_MASK \
+ (VIG_BASE_MASK | \
BIT(DPU_SSPP_CSC_10BIT) | BIT(DPU_SSPP_CDP) |\
- BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_EXCL_RECT))
+ BIT(DPU_SSPP_EXCL_RECT))
#define VIG_MSM8998_MASK \
(VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3))
@@ -29,7 +33,7 @@
#define VIG_SM8250_MASK \
(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
BIT(DPU_SSPP_SCALER_QSEED3LITE))
-#define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
+#define VIG_QCM2290_MASK (VIG_BASE_MASK | BIT(DPU_SSPP_QOS_8LVL))
#define DMA_MSM8998_MASK \
(BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |\
@@ -50,6 +54,10 @@
#define DMA_CURSOR_MSM8998_MASK \
(DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR))
+#define DMA_QCM2290_MASK \
+ (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\
+ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1))
+
#define MIXER_MSM8998_MASK \
(BIT(DPU_MIXER_SOURCESPLIT) | BIT(DPU_DIM_LAYER))
@@ -316,8 +324,6 @@ static const struct dpu_caps msm8998_dpu_caps = {
static const struct dpu_caps qcm2290_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0x4,
- .smart_dma_rev = DPU_SSPP_SMART_DMA_V2,
- .ubwc_version = DPU_HW_UBWC_VER_20,
.has_dim_layer = true,
.has_idle_pc = true,
.max_linewidth = 2160,
@@ -1384,7 +1390,7 @@ static const struct dpu_sspp_sub_blks
qcm2290_dma_sblk_0 = _DMA_SBLK("8", 1);
static const struct dpu_sspp_cfg qcm2290_sspp[] = {
SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_QCM2290_MASK,
qcm2290_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
- SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
+ SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_QCM2290_MASK,
qcm2290_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
};
@@ -2836,8 +2842,6 @@ static const struct dpu_mdss_cfg
qcm2290_dpu_cfg = {
.intf = qcm2290_intf,
.vbif_count = ARRAY_SIZE(sdm845_vbif),
.vbif = sdm845_vbif,
- .reg_dma_count = 1,
- .dma_cfg = _regdma,
.perf = _perf_data,
.mdss_irqs = IRQ_SC7180_MASK,
};
Re: [PATCH v1 10/14] drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine
On 1/23/2023 12:11 PM, Marijn Suijten wrote:
add support for*
drm/msm/dpu*
On 2023-01-23 10:24:30, Kuogee Hsieh wrote:
DSC V1.2 encoder engine is newly added hardware module. This patch
add support functions to configure and enable DSC V1.2 encoder engine.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 60 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 23 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 23 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 371 +
6 files changed, 463 insertions(+), 17 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 28cf52b..271c29a15 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_dsc_helper.o \
disp/dpu1/dpu_hw_interrupts.o \
disp/dpu1/dpu_hw_intf.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7f4a439..901e317 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1821,7 +1821,7 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc
*hw_dsc,
u32 initial_lines)
{
if (hw_dsc->ops.dsc_config)
- hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode, initial_lines);
+ hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode, initial_lines,
false);
As usual, an enum is better: readers have no idea what a free-floating
bool means.
if (hw_dsc->ops.dsc_config_thresh)
hw_dsc->ops.dsc_config_thresh(hw_dsc, dsc);
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 978e3bd..7b0b092 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.
*/
@@ -11,6 +11,7 @@
#include
#include
#include
+#include "dpu_hw_mdss.h"
Unused if you remove the unused DECLARE_BITMAP(dsc_pair_mask, DSC_MAX).
/**
* Max hardware block count: For ex: max 12 SSPP pipes or
@@ -182,6 +183,7 @@ enum {
* @DPU_PINGPONG_TE2Additional tear check block for split pipes
* @DPU_PINGPONG_SPLIT PP block supports split fifo
* @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
+ * @DPU_PINGPONG_DSC, Display stream compression blocks
* @DPU_PINGPONG_DITHER,Dither blocks
* @DPU_PINGPONG_MAX
*/
@@ -190,10 +192,32 @@ enum {
DPU_PINGPONG_TE2,
DPU_PINGPONG_SPLIT,
DPU_PINGPONG_SLAVE,
+ DPU_PINGPONG_DSC,
This is not used.
DPU_PINGPONG_DITHER,
DPU_PINGPONG_MAX
};
+
+/** DSC sub-blocks/features
Newline between /** and the text.
+ * @DPU_DSC_OUTPUT_CTRL Supports the control of the pp id which gets
+ * the pixel output from this DSC.
The original comment is much more concise, can we keep it?
+ * @DPU_DSC_HW_REV_1_1 dsc block supports dsc 1.1 only
+ * @DPU_DSC_HW_REV_1_2 dsc block supports dsc 1.1 and 1.2
Capitalize DSC just like elsewhere.
+ * @DPU_DSC_NATIVE_422_EN, Supports native422 and native420 encoding
+ * @DPU_DSC_ENC,DSC encoder sub block
+ * @DPU_DSC_CTL,DSC ctl sub block
No need for trailing commas in doc comments; if anything replace them
with colons?
+ * @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_ENC,
+ DPU_DSC_CTL,
These two enum values only have a meaning within the dpu_hw_dsc_1_2.c
file, and have nothing to do with the other feature flags/block
description. Please move them there (and give _dsc_subblk_offset a
proper enum type).
+ DPU_DSC_MAX
+};
+
/**
* CTL sub-blocks
* @DPU_CTL_SPLIT_DISPLAY:CTL supports video mode split display
@@ -276,15 +300,6 @@ enum {
};
/**
- * DSC features
- * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
- *the pixel output from this DSC.
- */
-enum {
- DPU_DSC_OUTPUT_CTRL = 0x1,
Did this have to move?
-};
-
-/**
* MACRO DPU_HW_BLK_INFO - information of HW
linux-next: manual merge of the drm-intel tree with the drm tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in:
drivers/gpu/drm/i915/i915_drv.h
between commit:
f67986b0119c ("drm/i915/pxp: Promote pxp subsystem to top-level of i915")
from the drm tree and commit:
a3f839762ceb ("drm/i915: move pch_ssc_use to display sub-struct under dpll")
from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/gpu/drm/i915/i915_drv.h
index 48fd82722f12,ac4c3c6f5541..
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@@ -65,26 -65,40 +65,41 @@@
#include "intel_uncore.h"
struct drm_i915_clock_gating_funcs;
- struct drm_i915_gem_object;
- struct drm_i915_private;
- struct intel_connector;
- struct intel_dp;
- struct intel_encoder;
- struct intel_limit;
- struct intel_overlay_error_state;
struct vlv_s0ix_state;
+struct intel_pxp;
- #define I915_GEM_GPU_DOMAINS \
- (I915_GEM_DOMAIN_RENDER | \
-I915_GEM_DOMAIN_SAMPLER | \
-I915_GEM_DOMAIN_COMMAND | \
-I915_GEM_DOMAIN_INSTRUCTION | \
-I915_GEM_DOMAIN_VERTEX)
+ #define GEM_QUIRK_PIN_SWIZZLED_PAGES BIT(0)
- #define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
+ /* Data Stolen Memory (DSM) aka "i915 stolen memory" */
+ struct i915_dsm {
+ /*
+* The start and end of DSM which we can optionally use to create GEM
+* objects backed by stolen memory.
+*
+* Note that usable_size tells us exactly how much of this we are
+* actually allowed to use, given that some portion of it is in fact
+* reserved for use by hardware functions.
+*/
+ struct resource stolen;
- #define GEM_QUIRK_PIN_SWIZZLED_PAGES BIT(0)
+ /*
+* Reserved portion of DSM.
+*/
+ struct resource reserved;
+
+ /*
+* Total size minus reserved ranges.
+*
+* DSM is segmented in hardware with different portions offlimits to
+* certain functions.
+*
+* The drm_mm is initialised to the total accessible range, as found
+* from the PCI config. On Broadwell+, this is further restricted to
+* avoid the first page! The upper end of DSM is reserved for hardware
+* functions and similarly removed from the accessible range.
+*/
+ resource_size_t usable_size;
+ };
struct i915_suspend_saved_registers {
u32 saveDSPARB;
@@@ -366,10 -337,6 +338,8 @@@ struct drm_i915_private
struct file *mmap_singleton;
} gem;
+ struct intel_pxp *pxp;
+
- u8 pch_ssc_use;
-
/* For i915gm/i945gm vblank irq workaround */
u8 vblank_enabled;
pgptsz1xvy1cE.pgp
Description: OpenPGP digital signature
Re: [PATCH v1 12/14] drm/msm/disp/dpu1: revise timing engine programming to work for DSC
On 2023-01-24 09:55:24, Kuogee Hsieh wrote: > This timing engine code is derived from our downstream code directly and > it has been used at many mobile devices by many vendors for many years > already. > > On the other words, it had been tested very thorough and works on > dsi/dp/hdmi/dsc/widebus applications. And the code already in mainline has seen 12 rounds of review, with a focus on inter-SoC compatibility. Regardless of that, we have processes to make changes on mainline: formatting changes (when actually making an improvement) go separate from semantic changes. Bugfixes are clearly described in individual patches with Fixes: tags. If you really think the code has to be as proposed in this patch, follow Dmitry's advice and split this accordingly. > When i brought dsc v1.2 over, I just merged it over and did not consider > too much. And that is exactly what is wrong with this *entire* series: copying over downstream code without "considering too much", stomping over previous review and even reverting bugfixes [1] [2] without giving it ANY ATTENTION in your patch description. That's unacceptable and insulting to contributors and reviewers. Full stop. Or did you expect us to turn a blind eye? This is mainline, not some techpack playground. [1]: https://lore.kernel.org/linux-arm-msm/20230123201133.zzt2zbyaw3pfk...@somainline.org/ [2]: https://lore.kernel.org/linux-arm-msm/20221026182824.876933-10-marijn.suij...@somainline.org/ > Can we adapt this code so that both upstream and down stream shared same > timing engine programming so that easier to maintain? Easy, I've said this before in IRC and will state it again: stop this techpack nonsense and focus on upstream-first. When something passes mainline review (and please don't bother maintainers and reviewers with series like this) it is inevitably good enough to be copied to techpack... at which point techpack becomes worthless as you can just backport a mainline patch or use a recent-enough kernel. tl;dr: it seems like you nor anyone involved in pre-reviewing/vetting this series is familiar with upstream guidelines. Follow the global advice from Dmitry [3] to reach a more efficient v2, and please don't let this run to v10 (or beyond) again. One suggestion to improve efficiency: split off the DPU v1.2 hardware block addition (and related changes) into a separate series. A smaller series (and properly split patches!) will give everyone less moving parts to worry about, and paves the way for DSI support without blocking on DP. [3]: https://lore.kernel.org/linux-arm-msm/47c83e8c-09f1-d1dd-ca79-574122638...@linaro.org/ - Marijn
[PATCH] drm/bridge: Kconfig: fix a spelling mistake
Correct a spelling mistake (reported by codespell). Signed-off-by: Randy Dunlap Cc: Jagan Teki Cc: Andrzej Hajda Cc: Neil Armstrong Cc: Robert Foss Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org -- drivers/gpu/drm/bridge/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -- a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -326,7 +326,7 @@ config DRM_TI_DLPC3433 input that produces a DMD output in RGB565, RGB666, RGB888 formats. - It supports upto 720p resolution with 60 and 120 Hz refresh + It supports up to 720p resolution with 60 and 120 Hz refresh rates. config DRM_TI_TFP410
Re: [PATCH 3/3] drm/msm/dpu: add missing ubwc_swizzle setting to catalog
On 12/7/2022 6:28 AM, Dmitry Baryshkov wrote:
Use the values from the vendor DTs to set ubwc_swizzle in the catalog.
Signed-off-by: Dmitry Baryshkov
I have already acked https://patchwork.freedesktop.org/patch/519662/
So this can be dropped from this series.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index a1b52b9b16f1..b80cc11a9a83 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -516,6 +516,7 @@ static const struct dpu_mdp_cfg sm6115_mdp[] = {
.base = 0x0, .len = 0x494,
.features = 0,
.highest_bank_bit = 0x1,
+ .ubwc_swizzle = 0x7,
.clk_ctrls[DPU_CLK_CTRL_VIG0] = {
.reg_off = 0x2ac, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA0] = {
@@ -529,6 +530,7 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
.base = 0x0, .len = 0x494,
.features = 0,
.highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */
+ .ubwc_swizzle = 0x6,
.clk_ctrls[DPU_CLK_CTRL_VIG0] = {
.reg_off = 0x2AC, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_VIG1] = {
@@ -558,6 +560,7 @@ static const struct dpu_mdp_cfg sm8450_mdp[] = {
.base = 0x0, .len = 0x494,
.features = BIT(DPU_MDP_PERIPH_0_REMOVED),
.highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */
+ .ubwc_swizzle = 0x6,
.clk_ctrls[DPU_CLK_CTRL_VIG0] = {
.reg_off = 0x2AC, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_VIG1] = {
Re: [PATCH 2/3] drm/msm/dpu: correct the UBWC version on sm6115
On 12/7/2022 6:28 AM, Dmitry Baryshkov wrote:
According to downstream (bengal-sde.dtsi), the sm6115 uses UBWC 1.0.
Change the catalog entry accordingly.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index b4ca123d8e69..a1b52b9b16f1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -335,7 +335,7 @@ static const struct dpu_caps sm6115_dpu_caps = {
.max_mixer_blendstages = 0x4,
.qseed_type = DPU_SSPP_SCALER_QSEED3LITE,
.smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
- .ubwc_version = DPU_HW_UBWC_VER_20,
+ .ubwc_version = DPU_HW_UBWC_VER_10,
.has_dim_layer = true,
.has_idle_pc = true,
.max_linewidth = 2160,
Re: [PATCH 1/3] drm/msm/dpu: handle UBWC 1.0 in dpu_hw_sspp_setup_format
On 12/7/2022 6:28 AM, Dmitry Baryshkov wrote:
Extend dpu_hw_sspp_setup_format() to also handle the UBWC 1.0 case.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
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 691c471b08c2..4246ab0b3bee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -310,7 +310,11 @@ static void dpu_hw_sspp_setup_format(struct dpu_hw_pipe
*ctx,
ctx->mdp->highest_bank_bit << 18);
switch (ctx->catalog->caps->ubwc_version) {
case DPU_HW_UBWC_VER_10:
- /* TODO: UBWC v1 case */
+ fast_clear = fmt->alpha_enable ? BIT(31) : 0;
+ DPU_REG_WRITE(c, SSPP_UBWC_STATIC_CTRL,
+ fast_clear | (ctx->mdp->ubwc_swizzle &
0x1) |
+ BIT(8) |
+ (ctx->mdp->highest_bank_bit << 4));
break;
case DPU_HW_UBWC_VER_20:
fast_clear = fmt->alpha_enable ? BIT(31) : 0;
Re: DMA-heap driver hints
On Mon, Jan 23, 2023 at 11:49 PM Christian König wrote: > > Am 24.01.23 um 04:56 schrieb James Jones: > > On 1/23/23 08:58, Laurent Pinchart wrote: > >> Hi Christian, > >> > >> On Mon, Jan 23, 2023 at 05:29:18PM +0100, Christian König wrote: > >>> Am 23.01.23 um 14:55 schrieb Laurent Pinchart: > Hi Christian, > > CC'ing James as I think this is related to his work on the unix device > memory allocator ([1]). > > > > Thank you for including me. > > Sorry for not having you in initially. I wasn't aware of your previous > work in this area. > > > > [1] > https://lore.kernel.org/dri-devel/8b555674-1c5b-c791-4547-2ea7c16ae...@nvidia.com/ > > On Mon, Jan 23, 2023 at 01:37:54PM +0100, Christian König wrote: > > Hi guys, > > > > this is just an RFC! The last time we discussed the DMA-buf coherency > > problem [1] we concluded that DMA-heap first needs a better way to > > communicate to userspace which heap to use for a certain device. > > > > As far as I know userspace currently just hard codes that information > > which is certainly not desirable considering that we should have this > > inside the kernel as well. > > > > So what those two patches here do is to first add some > > dma_heap_create_device_link() and dma_heap_remove_device_link() > > function and then demonstrating the functionality with uvcvideo > > driver. > > > > The preferred DMA-heap is represented with a symlink in sysfs between > > the device and the virtual DMA-heap device node. > > I'll start with a few high-level comments/questions: > > - Instead of tying drivers to heaps, have you considered a system > where > a driver would expose constraints, and a heap would then be > selected > based on those constraints ? A tight coupling between heaps and > drivers means downstream patches to drivers in order to use > vendor-specific heaps, that sounds painful. > >>> > >>> I was wondering the same thing as well, but came to the conclusion that > >>> just the other way around is the less painful approach. > >> > >> From a kernel point of view, sure, it's simpler and thus less painful. > >> From the point of view of solving the whole issue, I'm not sure :-) > >> > >>> The problem is that there are so many driver specific constrains that I > >>> don't even know where to start from. > >> > >> That's where I was hoping James would have some feedback for us, based > >> on the work he did on the Unix device memory allocator. If that's not > >> the case, we can brainstorm this from scratch. > > > > Simon Ser's and my presentation from XDC 2020 focused entirely on > > this. The idea was not to try to enumerate every constraint up front, > > but rather to develop an extensible mechanism that would be flexible > > enough to encapsulate many disparate types of constraints and perform > > set operations on them (merging sets was the only operation we tried > > to solve). Simon implemented a prototype header-only library to > > implement the mechanism: > > > > https://gitlab.freedesktop.org/emersion/drm-constraints > > > > The links to the presentation and talk are below, along with notes > > from the follow-up workshop. > > > > https://lpc.events/event/9/contributions/615/attachments/704/1301/XDC_2020__Allocation_Constraints.pdf > > > > https://www.youtube.com/watch?v=HZEClOP5TIk > > https://paste.sr.ht/~emersion/c43b30be08bab1882f1b107402074462bba3b64a > > > > Note one of the hard parts of this was figuring out how to express a > > device or heap within the constraint structs. One of the better ideas > > proposed back then was something like heap IDs, where dma heaps would > > each have one, > > We already have that. Each dma_heap has it's own unique name. > > > and devices could register their own heaps (or even just themselves?) > > with the heap subsystem and be assigned a locally-unique ID that > > userspace could pass around. > > I was more considering that we expose some kind of flag noting that a > certain device needs its buffer allocated from that device to utilize > all use cases. > > > This sounds similar to what you're proposing. Perhaps a reasonable > > identifier is a device (major, minor) pair. Such a constraint could be > > expressed as a symlink for easy visualization/discoverability from > > userspace, but might be easier to serialize over the wire as the > > (major, minor) pair. I'm not clear which direction is better to > > express this either: As a link from heap->device, or device->heap. > > > A constraint-based system would also, I think, be easier to extend > with additional constraints in the future. > > - I assume some drivers will be able to support multiple heaps. How do > you envision this being implemented ? > >>> > >>> I don't really see an use case for this. > > > > One use case I know of here is same-vendor GPU local
Re: [PATCH v2 0/8] Docs: Add missing SPDX license identifiers of
On Tue, 24 Jan 2023 15:31:04 -0700 Jonathan Corbet wrote: > SeongJae Park writes: > > > Some subsystem documents are missing SPDX license identifiers on index > > files. This patchset adds those. > > > > Changes from v1 > > (https://lore.kernel.org/lkml/20230114194741.115855-2...@kernel.org/) > > - Separate from index file content changes > > - Separate patch for each subsystem doc (Alex Deucher) > > - Use MIT license for gpu (Alex Deucher) > > > > SeongJae Park (8): > > Docs/crypto/index: Add missing SPDX License Identifier > > Docs/driver-api/index: Add missing SPDX License Identifier > > Docs/gpu/index: Add missing SPDX License Identifier > > Docs/hwmon/index: Add missing SPDX License Identifier > > Docs/input/index: Add missing SPDX License Identifier > > Docs/mm/index: Add missing SPDX License Identifier > > Docs/scheduler/index: Add missing SPDX License Identifier > > Docs/sound/index: Add missing SPDX License Identifier > > So I've applied patches 2 (since I wrote the initial file) and 8 (with > Takashi's ack). The others are also fine, I think, but I hesitate to > apply license texts to files without knowing that they match the > author's intent. I hate to say it, but I think the best approach is to > send each of the remaining patches to the appropriate maintainers for > the subsystem involved. That makes 100% sense, thank you, Jon! Thanks, SJ > > Thanks, > > jon
[PATCH] dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
Just as unevaluatedProperties or additionalProperties are required at the top level of schemas, they should (and will) also be required for child node schemas. That ensures only documented properties are present. Add unevaluatedProperties or additionalProperties as appropriate, and then add any missing properties flagged by the addition. Signed-off-by: Rob Herring --- To: Krzysztof Kozlowski To: David Airlie To: Daniel Vetter To: Bartosz Golaszewski To: Jean Delvare To: Guenter Roeck To: Thomas Gleixner To: Marc Zyngier To: Jassi Brar To: Mauro Carvalho Chehab To: Lee Jones To: Ulf Hansson To: Richard Weinberger To: Vignesh Raghavendra To: Sebastian Reichel To: Mark Brown To: "Rafael J. Wysocki" To: Daniel Lezcano To: Greg Kroah-Hartman Cc: linux-arm-ker...@lists.infradead.org Cc: dri-devel@lists.freedesktop.org Cc: linux-g...@vger.kernel.org Cc: linux-hw...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: open...@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-...@lists.infradead.org Cc: linux...@vger.kernel.org Cc: alsa-de...@alsa-project.org Cc: linux-...@vger.kernel.org Cc: linux-...@vger.kernel.org --- .../devicetree/bindings/arm/arm,vexpress-juno.yaml | 1 + .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml | 5 +++-- .../bindings/arm/tegra/nvidia,tegra20-pmc.yaml | 4 .../bindings/bus/allwinner,sun50i-a64-de2.yaml | 1 + .../bindings/bus/allwinner,sun8i-a23-rsb.yaml | 1 + .../bus/intel,ixp4xx-expansion-bus-controller.yaml | 6 ++ Documentation/devicetree/bindings/bus/palmbus.yaml | 1 + .../devicetree/bindings/display/msm/qcom,mdss.yaml | 5 + Documentation/devicetree/bindings/example-schema.yaml | 2 ++ .../devicetree/bindings/gpio/x-powers,axp209-gpio.yaml | 1 + .../devicetree/bindings/hwmon/adi,ltc2992.yaml | 1 + .../bindings/interrupt-controller/arm,gic-v3.yaml | 2 ++ .../bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml | 1 + .../devicetree/bindings/media/i2c/maxim,max9286.yaml | 7 +++ .../bindings/memory-controllers/arm,pl35x-smc.yaml | 1 + .../bindings/memory-controllers/exynos-srom.yaml | 1 + .../memory-controllers/nvidia,tegra124-emc.yaml| 1 + .../bindings/memory-controllers/st,stm32-fmc2-ebi.yaml | 1 + .../devicetree/bindings/mfd/mediatek,mt6370.yaml | 2 ++ .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 1 + Documentation/devicetree/bindings/mtd/mtd.yaml | 1 + .../devicetree/bindings/power/supply/ti,lp8727.yaml| 1 + .../devicetree/bindings/soc/imx/fsl,imx93-src.yaml | 3 ++- .../bindings/soc/microchip/atmel,at91rm9200-tcb.yaml | 1 + Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 1 + .../devicetree/bindings/sound/marvell,mmp-sspa.yaml| 1 + .../devicetree/bindings/sound/qcom,wcd934x.yaml| 1 + .../devicetree/bindings/sound/samsung,odroid.yaml | 2 ++ .../devicetree/bindings/soundwire/qcom,soundwire.yaml | 1 + .../bindings/spi/allwinner,sun4i-a10-spi.yaml | 1 + .../bindings/spi/allwinner,sun6i-a31-spi.yaml | 1 + .../devicetree/bindings/spi/spi-controller.yaml| 1 + .../sram/allwinner,sun4i-a10-system-control.yaml | 10 +- Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 1 + .../devicetree/bindings/thermal/thermal-zones.yaml | 1 + Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 + 36 files changed, 65 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml index eec190a96225..09c319f803ba 100644 --- a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml +++ b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml @@ -144,6 +144,7 @@ patternProperties: it is stricter and always has two compatibles. type: object $ref: '/schemas/simple-bus.yaml' +unevaluatedProperties: false properties: compatible: diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml index b369b374fc4a..39e3c248f5b7 100644 --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml @@ -30,6 +30,7 @@ properties: clocks: type: object +additionalProperties: false properties: compatible: @@ -47,6 +48,7 @@ properties: reset: type: object +additionalProperties: false properties: compatible: @@ -63,6 +65,7 @@ properties: pwm: type: object +additionalProperties: false properties: compatible: @@ -76,8 +79,6 @@ properties: - compatible - "#pwm-cells" -additionalProperties: false - required: - compatible - mboxes diff --git
[PATCH] dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
Just as unevaluatedProperties or additionalProperties are required at the top level of schemas, they should (and will) also be required for child node schemas. That ensures only documented properties are present. Add unevaluatedProperties or additionalProperties as appropriate, and then add any missing properties flagged by the addition. Signed-off-by: Rob Herring --- To: Krzysztof Kozlowski To: David Airlie To: Daniel Vetter To: Bartosz Golaszewski To: Jean Delvare To: Guenter Roeck To: Thomas Gleixner To: Marc Zyngier To: Jassi Brar To: Mauro Carvalho Chehab To: Lee Jones To: Ulf Hansson To: Richard Weinberger To: Vignesh Raghavendra To: Sebastian Reichel To: Mark Brown To: "Rafael J. Wysocki" To: Daniel Lezcano To: Greg Kroah-Hartman Cc: linux-arm-ker...@lists.infradead.org Cc: dri-devel@lists.freedesktop.org Cc: linux-g...@vger.kernel.org Cc: linux-hw...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: open...@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-...@lists.infradead.org Cc: linux...@vger.kernel.org Cc: alsa-de...@alsa-project.org Cc: linux-...@vger.kernel.org Cc: linux-...@vger.kernel.org --- .../devicetree/bindings/arm/arm,vexpress-juno.yaml | 1 + .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml | 5 +++-- .../bindings/arm/tegra/nvidia,tegra20-pmc.yaml | 4 .../bindings/bus/allwinner,sun50i-a64-de2.yaml | 1 + .../bindings/bus/allwinner,sun8i-a23-rsb.yaml | 1 + .../bus/intel,ixp4xx-expansion-bus-controller.yaml | 6 ++ Documentation/devicetree/bindings/bus/palmbus.yaml | 1 + .../devicetree/bindings/display/msm/qcom,mdss.yaml | 5 + Documentation/devicetree/bindings/example-schema.yaml | 2 ++ .../devicetree/bindings/gpio/x-powers,axp209-gpio.yaml | 1 + .../devicetree/bindings/hwmon/adi,ltc2992.yaml | 1 + .../bindings/interrupt-controller/arm,gic-v3.yaml | 2 ++ .../bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml | 1 + .../devicetree/bindings/media/i2c/maxim,max9286.yaml | 7 +++ .../bindings/memory-controllers/arm,pl35x-smc.yaml | 1 + .../bindings/memory-controllers/exynos-srom.yaml | 1 + .../memory-controllers/nvidia,tegra124-emc.yaml| 1 + .../bindings/memory-controllers/st,stm32-fmc2-ebi.yaml | 1 + .../devicetree/bindings/mfd/mediatek,mt6370.yaml | 2 ++ .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 1 + Documentation/devicetree/bindings/mtd/mtd.yaml | 1 + .../devicetree/bindings/power/supply/ti,lp8727.yaml| 1 + .../devicetree/bindings/soc/imx/fsl,imx93-src.yaml | 3 ++- .../bindings/soc/microchip/atmel,at91rm9200-tcb.yaml | 1 + Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 1 + .../devicetree/bindings/sound/marvell,mmp-sspa.yaml| 1 + .../devicetree/bindings/sound/qcom,wcd934x.yaml| 1 + .../devicetree/bindings/sound/samsung,odroid.yaml | 2 ++ .../devicetree/bindings/soundwire/qcom,soundwire.yaml | 1 + .../bindings/spi/allwinner,sun4i-a10-spi.yaml | 1 + .../bindings/spi/allwinner,sun6i-a31-spi.yaml | 1 + .../devicetree/bindings/spi/spi-controller.yaml| 1 + .../sram/allwinner,sun4i-a10-system-control.yaml | 10 +- Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 1 + .../devicetree/bindings/thermal/thermal-zones.yaml | 1 + Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 + 36 files changed, 65 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml index eec190a96225..09c319f803ba 100644 --- a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml +++ b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml @@ -144,6 +144,7 @@ patternProperties: it is stricter and always has two compatibles. type: object $ref: '/schemas/simple-bus.yaml' +unevaluatedProperties: false properties: compatible: diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml index b369b374fc4a..39e3c248f5b7 100644 --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml @@ -30,6 +30,7 @@ properties: clocks: type: object +additionalProperties: false properties: compatible: @@ -47,6 +48,7 @@ properties: reset: type: object +additionalProperties: false properties: compatible: @@ -63,6 +65,7 @@ properties: pwm: type: object +additionalProperties: false properties: compatible: @@ -76,8 +79,6 @@ properties: - compatible - "#pwm-cells" -additionalProperties: false - required: - compatible - mboxes diff --git
Re: [PATCH v2 2/2] dt-bindings: display: simple-framebuffer: Document physical width and height properties
> Date: Tue, 24 Jan 2023 22:19:09 + > From: Rayyan Ansari > > On 23/01/2023 17:53, Rob Herring wrote: > > On Sun, Jan 22, 2023 at 05:25:38PM +, Rayyan Ansari wrote: > >> On 22/01/2023 15:36, Rob Herring wrote: > >>> On Sat, Jan 21, 2023 at 9:36 AM Rayyan Ansari wrote: > > >>> > >>> Why do you need this change? > >>> > >>> The 'simple-framebuffer' contains data on how the bootloader > >>> configured the display. The bootloader doesn't configure the display > >>> size, so this information doesn't belong here. The information should > >>> already be in the panel node, so also no point in duplicating it here. > >>> > Signed-off-by: Rayyan Ansari > --- > .../devicetree/bindings/display/simple-framebuffer.yaml | 8 > 1 file changed, 8 insertions(+) > >> > >> Hi Rob, > >> > >> There is the usecase that Hans has mentioned, but I have also mentioned > >> another usecase previously. > >> > >> Adding the width-mm and height-mm properties allows user interfaces such as > >> Phosh (https://puri.sm/posts/phosh-overview/) to scale correctly to the > >> screen. In my case, a panel node is not available and the aforementioned > >> interface is in fact running on the SimpleDRM driver (which binds to the > >> simple-framebuffer device). > > > > Why is the panel node not available? Why not add it? Presumably it is > > not there because you aren't (yet) using the simple-panel driver (and > > others that would need). But presumably you will eventually as I'd > > imagine turning the screen off and back on might be a desired feature. > > It requires more than using the simple-panel driver: first the SoC side > display hardware needs to be brought up, then a panel driver that > implements the proper DCS initialisation sequence needs to be written > (which is currently not fully known). You don't really need a driver. You can just lookup the panel node from your simple-framebuffer driver and get the values of the properties there.
[PATCH v3 1/2] drm/simpledrm: Allow physical width and height configuration via DT
Add optional width-mm and height-mm devicetree properties and use this
to set the DRM Display Mode instead of calculating it based on an
average DPI.
Signed-off-by: Rayyan Ansari
---
drivers/gpu/drm/tiny/simpledrm.c | 60 +++-
1 file changed, 51 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 162eb44dcba8..7aab7fa572f0 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -128,6 +128,23 @@ simplefb_read_u32_of(struct drm_device *dev, struct
device_node *of_node,
return ret;
}
+static int
+simplefb_read_u32_of_opt(struct drm_device *dev, struct device_node *of_node,
+const char *name, u32 *value)
+{
+ int ret = of_property_read_u32(of_node, name, value);
+
+ if (ret == -EINVAL) {
+ *value = 0;
+ ret = 0;
+ } else if (ret) {
+ drm_err(dev, "simplefb: cannot parse framebuffer %s: error
%d\n",
+ name, ret);
+ }
+
+ return ret;
+}
+
static int
simplefb_read_string_of(struct drm_device *dev, struct device_node *of_node,
const char *name, const char **value)
@@ -184,6 +201,19 @@ simplefb_get_format_of(struct drm_device *dev, struct
device_node *of_node)
return simplefb_get_validated_format(dev, format);
}
+static int
+simplefb_get_mm_of(struct drm_device *dev, struct device_node *of_node,
+ const char *name)
+{
+ int mm;
+ int ret = simplefb_read_u32_of_opt(dev, of_node, name, );
+
+ if (ret)
+ return ret;
+ return simplefb_get_validated_int(dev, name, mm);
+}
+
+
/*
* Simple Framebuffer device
*/
@@ -599,16 +629,12 @@ static const struct drm_mode_config_funcs
simpledrm_mode_config_funcs = {
*/
static struct drm_display_mode simpledrm_mode(unsigned int width,
- unsigned int height)
+ unsigned int height,
+ unsigned int width_mm,
+ unsigned int height_mm)
{
- /*
-* Assume a monitor resolution of 96 dpi to
-* get a somewhat reasonable screen size.
-*/
const struct drm_display_mode mode = {
- DRM_MODE_INIT(60, width, height,
- DRM_MODE_RES_MM(width, 96ul),
- DRM_MODE_RES_MM(height, 96ul))
+ DRM_MODE_INIT(60, width, height, width_mm, height_mm)
};
return mode;
@@ -622,6 +648,7 @@ static struct simpledrm_device
*simpledrm_device_create(struct drm_driver *drv,
struct simpledrm_device *sdev;
struct drm_device *dev;
int width, height, stride;
+ int width_mm = 0, height_mm = 0;
const struct drm_format_info *format;
struct resource *res, *mem;
void __iomem *screen_base;
@@ -676,6 +703,12 @@ static struct simpledrm_device
*simpledrm_device_create(struct drm_driver *drv,
format = simplefb_get_format_of(dev, of_node);
if (IS_ERR(format))
return ERR_CAST(format);
+ width_mm = simplefb_get_mm_of(dev, of_node, "width-mm");
+ if (width_mm < 0)
+ return ERR_PTR(width_mm);
+ height_mm = simplefb_get_mm_of(dev, of_node, "height-mm");
+ if (height_mm < 0)
+ return ERR_PTR(height_mm);
} else {
drm_err(dev, "no simplefb configuration found\n");
return ERR_PTR(-ENODEV);
@@ -686,7 +719,16 @@ static struct simpledrm_device
*simpledrm_device_create(struct drm_driver *drv,
return ERR_PTR(-EINVAL);
}
- sdev->mode = simpledrm_mode(width, height);
+ /*
+* Assume a monitor resolution of 96 dpi if physical dimensions
+* are not specified to get a somewhat reasonable screen size.
+*/
+ if (!width_mm)
+ width_mm = DRM_MODE_RES_MM(width, 96ul);
+ if (!height_mm)
+ height_mm = DRM_MODE_RES_MM(height, 96ul);
+
+ sdev->mode = simpledrm_mode(width, height, width_mm, height_mm);
sdev->format = format;
sdev->pitch = stride;
--
2.39.1
[PATCH v3 2/2] dt-bindings: display: simple-framebuffer: Document physical width and height properties
Document the optional width-mm and height-mm simple-framebuffer properties. Signed-off-by: Rayyan Ansari --- .../devicetree/bindings/display/simple-framebuffer.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml index dd64f70b5014..4ae33a4d2da9 100644 --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml @@ -106,6 +106,12 @@ properties: - x2r10g10b10 - x8r8g8b8 + width-mm: +description: Physical width of the display in millimetres + + height-mm: +description: Physical height of the display in millimetres + display: $ref: /schemas/types.yaml#/definitions/phandle description: Primary display hardware node -- 2.39.1
[PATCH v3 0/2] SimpleDRM: allow configuring physical width and height
Hello, The following patches: - Add support for configuring the width-mm and height-mm DRM mode properties in the SimpleDRM driver via Device Tree - Document these two new Device Tree properties This is useful for allowing interfaces such as Phosh to calculate proper scaling values and for early boot code knowing if hi-dpi rendering is necessary. Changes since v2: - Remove $ref property (because it is an SI unit) - Extend commit messages Rayyan Ansari (2): drm/simpledrm: Allow physical width and height configuration via DT dt-bindings: display: simple-framebuffer: Document physical width and height properties .../bindings/display/simple-framebuffer.yaml | 6 ++ drivers/gpu/drm/tiny/simpledrm.c | 60 --- 2 files changed, 57 insertions(+), 9 deletions(-) -- 2.39.1
Re: [PATCH v2 0/8] Docs: Add missing SPDX license identifiers of
SeongJae Park writes: > Some subsystem documents are missing SPDX license identifiers on index > files. This patchset adds those. > > Changes from v1 > (https://lore.kernel.org/lkml/20230114194741.115855-2...@kernel.org/) > - Separate from index file content changes > - Separate patch for each subsystem doc (Alex Deucher) > - Use MIT license for gpu (Alex Deucher) > > SeongJae Park (8): > Docs/crypto/index: Add missing SPDX License Identifier > Docs/driver-api/index: Add missing SPDX License Identifier > Docs/gpu/index: Add missing SPDX License Identifier > Docs/hwmon/index: Add missing SPDX License Identifier > Docs/input/index: Add missing SPDX License Identifier > Docs/mm/index: Add missing SPDX License Identifier > Docs/scheduler/index: Add missing SPDX License Identifier > Docs/sound/index: Add missing SPDX License Identifier So I've applied patches 2 (since I wrote the initial file) and 8 (with Takashi's ack). The others are also fine, I think, but I hesitate to apply license texts to files without knowing that they match the author's intent. I hate to say it, but I think the best approach is to send each of the remaining patches to the appropriate maintainers for the subsystem involved. Thanks, jon
Re: [PATCH v2 1/1] Docs/subsystem-apis: Remove '[The ]Linux' prefixes from titles of listed documents
SeongJae Park writes: > Some documents that listed on subsystem-apis have 'Linux' or 'The Linux' > title prefixes. It's duplicated information, and makes finding the > document of interest with human eyes not easy. Remove the prefixes from > the titles. > > Signed-off-by: SeongJae Park > --- > Changes from v1 > (https://lore.kernel.org/lkml/20230114194741.115855-1...@kernel.org/) > - Drop second patch (will post later for each subsystem) > > Documentation/PCI/index.rst| 6 +++--- > Documentation/cpu-freq/index.rst | 6 +++--- > Documentation/crypto/index.rst | 6 +++--- > Documentation/driver-api/index.rst | 6 +++--- > Documentation/gpu/index.rst| 6 +++--- > Documentation/hwmon/index.rst | 6 +++--- > Documentation/input/index.rst | 6 +++--- > Documentation/mm/index.rst | 6 +++--- > Documentation/peci/index.rst | 6 +++--- > Documentation/scheduler/index.rst | 6 +++--- > Documentation/scsi/index.rst | 6 +++--- > Documentation/sound/index.rst | 6 +++--- > Documentation/virt/index.rst | 6 +++--- > Documentation/watchdog/index.rst | 6 +++--- > 14 files changed, 42 insertions(+), 42 deletions(-) Applied, thanks. jon
Re: [PATCH] docs: accel: Fix debugfs path
Jeffrey Hugo writes:
> The device specific directory in debugfs does not have "accel". For
> example, the documentation says device 0 should have a debugfs entry as
> /sys/kernel/debug/accel/accel0/ but in reality the entry is
> /sys/kernel/debug/accel/0/
>
> Fix the documentation to match the implementation.
>
> Fixes: 8c5577a5ccc6 ("doc: add documentation for accel subsystem")
> Signed-off-by: Jeffrey Hugo
> ---
> Documentation/accel/introduction.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/accel/introduction.rst
> b/Documentation/accel/introduction.rst
> index 6f31af1..11cade5 100644
> --- a/Documentation/accel/introduction.rst
> +++ b/Documentation/accel/introduction.rst
> @@ -69,7 +69,7 @@ The accelerator devices will be exposed to the user space
> with the dedicated
>
> - device char files - /dev/accel/accel*
> - sysfs - /sys/class/accel/accel*/
> -- debugfs - /sys/kernel/debug/accel/accel*/
> +- debugfs - /sys/kernel/debug/accel/*/
>
Applied, thanks.
jon
Re: [PATCH v2 2/2] dt-bindings: display: simple-framebuffer: Document physical width and height properties
On 23/01/2023 17:53, Rob Herring wrote: On Sun, Jan 22, 2023 at 05:25:38PM +, Rayyan Ansari wrote: On 22/01/2023 15:36, Rob Herring wrote: On Sat, Jan 21, 2023 at 9:36 AM Rayyan Ansari wrote: Why do you need this change? The 'simple-framebuffer' contains data on how the bootloader configured the display. The bootloader doesn't configure the display size, so this information doesn't belong here. The information should already be in the panel node, so also no point in duplicating it here. Signed-off-by: Rayyan Ansari --- .../devicetree/bindings/display/simple-framebuffer.yaml | 8 1 file changed, 8 insertions(+) Hi Rob, There is the usecase that Hans has mentioned, but I have also mentioned another usecase previously. Adding the width-mm and height-mm properties allows user interfaces such as Phosh (https://puri.sm/posts/phosh-overview/) to scale correctly to the screen. In my case, a panel node is not available and the aforementioned interface is in fact running on the SimpleDRM driver (which binds to the simple-framebuffer device). Why is the panel node not available? Why not add it? Presumably it is not there because you aren't (yet) using the simple-panel driver (and others that would need). But presumably you will eventually as I'd imagine turning the screen off and back on might be a desired feature. It requires more than using the simple-panel driver: first the SoC side display hardware needs to be brought up, then a panel driver that implements the proper DCS initialisation sequence needs to be written (which is currently not fully known). So why add a temporary DT property that's tied to your *current* kernel? > The DT should not be tightly coupled to the kernel. I'm not sure what you mean by it being "tightly coupled" to the kernel. Rob -- Rayyan Ansari https://ansari.sh
Re: [PATCH v3 01/21] dt-bindings: display: tegra: add Tegra20 VIP
On 12/29/22 16:31, Luca Ceresoli wrote: > VIP is the parallel video capture component within the video input > subsystem of Tegra20 (and other Tegra chips, apparently). > > Signed-off-by: Luca Ceresoli > > --- > > Changed in v3: > - remove channel@0 node (Krzysztof, Rob, Dmitry) > - add myself as a maintainer of the whole Tegra video driver (Dmitry) > > Changed in v2 (suggested by Krzysztof Kozlowski): > - remove redundant "bindings" from subject line > - remove $nodename > - add channel@0 description > - add reg: const: 0 > --- > .../display/tegra/nvidia,tegra20-vip.yaml | 49 +++ > MAINTAINERS | 2 + > 2 files changed, 51 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > > diff --git > a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > new file mode 100644 > index ..c23fd02dd18e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > @@ -0,0 +1,49 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NVIDIA Tegra VIP (parallel video capture) controller > + > +maintainers: > + - Luca Ceresoli > + > +properties: > + compatible: > +enum: > + - nvidia,tegra20-vip > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > + ports: > +$ref: /schemas/graph.yaml#/properties/ports > + > +properties: > + port@0: > +$ref: /schemas/graph.yaml#/properties/port > +description: > + Port receiving the video stream from the sensor > + > + port@1: > +$ref: /schemas/graph.yaml#/properties/port > +description: > + Port sending the video stream to the VI > + > +required: > + - port@0 > + - port@1 > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + - ports > + > +# see nvidia,tegra20-vi.yaml for an example > diff --git a/MAINTAINERS b/MAINTAINERS > index f61eb221415b..4feb2e1258ce 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20563,10 +20563,12 @@ TEGRA VIDEO DRIVER > M: Thierry Reding > M: Jonathan Hunter > M: Sowjanya Komatineni > +M: Luca Ceresoli > L: linux-me...@vger.kernel.org > L: linux-te...@vger.kernel.org > S: Maintained > F: > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > +F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > F: drivers/staging/media/tegra-video/ > > TEGRA XUSB PADCTL DRIVER Reviewed-by: Dmitry Osipenko Please feel free to add my r-b to the rest of the patches. I looked though them and haven't noticed anything suspicious. -- Best regards, Dmitry
Re: [PATCH v3 02/21] dt-bindings: display: tegra: vi: add 'vip' property and example
On 1/25/23 00:14, Luca Ceresoli wrote:
> Hi Dmitry,
>
> On Tue, 24 Jan 2023 20:02:39 +0300
> Dmitry Osipenko wrote:
>
>> On 12/29/22 16:31, Luca Ceresoli wrote:
>>> +vip {
>>> +compatible = "nvidia,tegra20-vip";
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +channel@0 {
>>> +reg = <0>;
>>> +ports {
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +port@0 {
>>> +reg = <0>;
>>> +vi_vip_in: endpoint {
>>> +remote-endpoint = <_out>;
>>> +};
>>> +};
>>> +port@1 {
>>> +reg = <1>;
>>> +vi_vip_out: endpoint {
>>> +remote-endpoint = <_in>;
>>> +};
>>> +};
>>> +};
>>> +};
>>
>> In the changelog you said that the channel@0 node is removed
>
> Thanks, you are right, this is a leftover. I must have forgotten about
> updating the example. :-\
>
> It will be fixed in v4 obviously, I was waiting before sending it in
> case of any feedback on the other patches.
The rest of the patches look okay. The patchset should get testing on
T210 and then will be good to apply it, IMO. If nobody will volunteer to
test T210 then should be good to apply it too, we can always fix the
code later on.
--
Best regards,
Dmitry
Re: [PATCH v1 12/14] drm/msm/disp/dpu1: revise timing engine programming to work for DSC
On 24/01/2023 19:55, Kuogee Hsieh wrote:
On 1/24/2023 1:11 AM, Dmitry Baryshkov wrote:
On 23/01/2023 20:24, Kuogee Hsieh wrote:
Current implementation timing engine programming does not consider
compression factors. This patch add consideration of DSC factors
while programming timing engine.
Signed-off-by: Kuogee Hsieh
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 14 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 132
+
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +-
5 files changed, 110 insertions(+), 54 deletions(-)
[skipped]
@@ -113,82 +124,96 @@ static void
dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
/* read interface_cfg */
intf_cfg = DPU_REG_READ(c, INTF_CONFIG);
- if (ctx->cap->type == INTF_DP)
+ if (ctx->cap->type == INTF_EDP || ctx->cap->type == INTF_DP)
dp_intf = true;
hsync_period = p->hsync_pulse_width + p->h_back_porch +
p->width +
- p->h_front_porch;
+ p->h_front_porch;
vsync_period = p->vsync_pulse_width + p->v_back_porch +
p->height +
- p->v_front_porch;
+ p->v_front_porch;
Actually I went on through the history and found the previous
submission, https://patchwork.freedesktop.org/patch/471505/.
Exactly the same piece of code. Did you expect that the comments will
be different this time?
I really hoped that at that time we already went through this. But it
seems I was wrong. That series went through v10 or v12 before being
accepted. And it was just adding wide_bus_en. Back at that time we
lightly discussed that the code will receive compression support. But
I never expected to see the original submission again.
It might sound bad, but could you please find somebody who can do
internal review for you? Good internal review.
That said, I really do not expect to see v2 before the whole series is
reworked, restructured and prepared for the review on your side.
This timing engine code is derived from our downstream code directly and
it has been used at many mobile devices by many vendors for many years
already.
On the other words, it had been tested very thorough and works on
dsi/dp/hdmi/dsc/widebus applications.
As far as I understand, it has been tested on the recent generations of
the hardware. I doubt that anybody retests new techpack drops on
previous hardware generations. Correct?
When was the last time this particular code drop was tested on
INTF_HDMI? I think it was back in the 4.4 era. Newer vendor kernels do
not have hdmi-staging, so at least the claim of testing this codepiece
on HDMI is not correct.
What is the earliest chip that has been driven by this particular code
instance?
When i brought dsc v1.2 over, I just merged it over and did not consider
too much.
Can we adapt this code so that both upstream and down stream shared same
timing engine programming so that easier to maintain?
We have been discussing exactly the same piece of code a year ago. Could
you please recheck the comments that were provided to your patches. And
I actually mean that. There were 12 iterations of wide bus patchset.
Timing engine programming patch had 8. I do not want to start again from
the very beginning.
The basic idea is that you have to evolve the code rather than flushing
us with the 'latest and greatest code dump'. Split this into individual
atomic changes that we can review. Provide justification (= motivation)
for each change. Previously we haven't seen them.
We know that current function works. We must be able to assume that new
instance doesn't break things. Or, if something breaks, understand which
particular change broke it. Consider the case that your patch breaks
msm8998. Or sdm845. How can we cope? Would you be able to spot the place
which did that? I know I wouldn't. The only way would be to revert the
patch completely. And inherently the whole series.
display_v_start = ((p->vsync_pulse_width + p->v_back_porch) *
- hsync_period) + p->hsync_skew;
+ hsync_period) + p->hsync_skew;
display_v_end = ((vsync_period - p->v_front_porch) *
hsync_period) +
- p->hsync_skew - 1;
+ p->hsync_skew - 1;
+
+ hsync_ctl = (hsync_period << 16) | p->hsync_pulse_width;
hsync_start_x = p->h_back_porch + p->hsync_pulse_width;
hsync_end_x = hsync_period - p->h_front_porch - 1;
- if (p->width != p->xres) { /* border fill added */
- active_h_start = hsync_start_x;
- active_h_end = active_h_start + p->xres - 1;
- } else {
- active_h_start = 0;
- active_h_end = 0;
- }
-
- if (p->height != p->yres) { /* border fill added */
- active_v_start = display_v_start;
- active_v_end = active_v_start + (p->yres * hsync_period) - 1;
- } else {
- active_v_start
Re: [PATCH v3 02/21] dt-bindings: display: tegra: vi: add 'vip' property and example
Hi Dmitry,
On Tue, 24 Jan 2023 20:02:39 +0300
Dmitry Osipenko wrote:
> On 12/29/22 16:31, Luca Ceresoli wrote:
> > +vip {
> > +compatible = "nvidia,tegra20-vip";
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +channel@0 {
> > +reg = <0>;
> > +ports {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +port@0 {
> > +reg = <0>;
> > +vi_vip_in: endpoint {
> > +remote-endpoint = <_out>;
> > +};
> > +};
> > +port@1 {
> > +reg = <1>;
> > +vi_vip_out: endpoint {
> > +remote-endpoint = <_in>;
> > +};
> > +};
> > +};
> > +};
>
> In the changelog you said that the channel@0 node is removed
Thanks, you are right, this is a leftover. I must have forgotten about
updating the example. :-\
It will be fixed in v4 obviously, I was waiting before sending it in
case of any feedback on the other patches.
Best regards.
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [RESEND PATCH v11 13/18] drm: exynos: dsi: Add Exynos based host irq hooks
On Wed, Jan 25, 2023 at 2:54 AM Jagan Teki wrote: > > On Wed, Jan 25, 2023 at 2:42 AM Marek Vasut wrote: > > > > On 1/24/23 22:01, Jagan Teki wrote: > > > On Wed, Jan 25, 2023 at 2:18 AM Marek Vasut wrote: > > >> > > >> On 1/23/23 16:12, Jagan Teki wrote: > > >>> Enable and disable of te_gpio's are Exynos platform specific > > >>> irq handling, so add the exynos based irq operations and hook > > >>> them for exynos plat_data. > > >> > > >> If this is just an optional generic GPIO IRQ, why not keep it in the > > >> core code ? TE (tearing enable?) should be available on MX8M too. > > > > > > So far the discussion (since from initial versions) with Marek > > > Szyprowski, seems to be available in Exynos. So, I keep it separate > > > from the DSIM core. > > > > Isn't TE a generic GPIO IRQ ? If so, it is available also on i.MX8M . I will check this. Jagan.
Re: [RESEND PATCH v11 13/18] drm: exynos: dsi: Add Exynos based host irq hooks
On Wed, Jan 25, 2023 at 2:42 AM Marek Vasut wrote: > > On 1/24/23 22:01, Jagan Teki wrote: > > On Wed, Jan 25, 2023 at 2:18 AM Marek Vasut wrote: > >> > >> On 1/23/23 16:12, Jagan Teki wrote: > >>> Enable and disable of te_gpio's are Exynos platform specific > >>> irq handling, so add the exynos based irq operations and hook > >>> them for exynos plat_data. > >> > >> If this is just an optional generic GPIO IRQ, why not keep it in the > >> core code ? TE (tearing enable?) should be available on MX8M too. > > > > So far the discussion (since from initial versions) with Marek > > Szyprowski, seems to be available in Exynos. So, I keep it separate > > from the DSIM core. > > Isn't TE a generic GPIO IRQ ? If so, it is available also on i.MX8M . -- Jagan Teki, Amarula Solutions India Pvt. Ltd. Co-Founder & Embedded Linux Architect 405/E-Block, Sri Lakshmi Shubham Arcade, Chandanagar, Hyderabad - 500050, India M. (+91) 910 009 0959 [`as] http://www.amarulasolutions.com
Re: [RESEND PATCH v11 11/18] drm: exynos: dsi: Add atomic_get_input_bus_fmts
On Wed, Jan 25, 2023 at 2:49 AM Marek Vasut wrote:
>
> On 1/24/23 22:16, Jagan Teki wrote:
> > On Wed, Jan 25, 2023 at 2:15 AM Marek Vasut wrote:
> >>
> >> On 1/23/23 16:12, Jagan Teki wrote:
> >>
> >> [...]
> >>
> >>> +static bool exynos_dsi_pixel_output_fmt_supported(u32 fmt)
> >>> +{
> >>> + int i;
> >>
> >> if (fmt == MEDIA_BUS_FMT_FIXED)
> >>return false;
> >>
> >>> + for (i = 0; i < ARRAY_SIZE(exynos_dsi_pixel_output_fmts); i++) {
> >>> + if (exynos_dsi_pixel_output_fmts[i] == fmt)
> >>> + return true;
> >>> + }
> >>> +
> >>> + return false;
> >>> +}
> >>> +
> >>> +static u32 *
> >>> +exynos_dsi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> >>> + struct drm_bridge_state *bridge_state,
> >>> + struct drm_crtc_state *crtc_state,
> >>> + struct drm_connector_state *conn_state,
> >>> + u32 output_fmt,
> >>> + unsigned int *num_input_fmts)
> >>> +{
> >>> + u32 *input_fmts;
> >>> +
> >>> + if (!exynos_dsi_pixel_output_fmt_supported(output_fmt))
> >>> + /*
> >>> + * Some bridge/display drivers are still not able to pass
> >>> the
> >>> + * correct format, so handle those pipelines by falling back
> >>> + * to the default format till the supported formats
> >>> finalized.
> >>> + */
> >>> + output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
> >>
> >> You can move this ^ past the kmalloc() call, so in unlikely case the
> >> kmalloc() fails, it would fail first.
> >
> > I didn't get this point, what do we need to do if
> > exynos_dsi_pixel_output_fmt_supported returns false?
>
> {
> u32 *input_fmts;
>
> input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
> if (!input_fmts)
> return NULL;
>
> if (!exynos_dsi_pixel_output_fmt_supported(output_fmt))
> /* ... the comment ... */
> output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
>
> input_fmts[0] = output_fmt;
> *num_input_fmts = 1;
>
> return input_fmts;
> }
Got it, thanks!
Re: [RESEND PATCH v11 11/18] drm: exynos: dsi: Add atomic_get_input_bus_fmts
On 1/24/23 22:16, Jagan Teki wrote:
On Wed, Jan 25, 2023 at 2:15 AM Marek Vasut wrote:
On 1/23/23 16:12, Jagan Teki wrote:
[...]
+static bool exynos_dsi_pixel_output_fmt_supported(u32 fmt)
+{
+ int i;
if (fmt == MEDIA_BUS_FMT_FIXED)
return false;
+ for (i = 0; i < ARRAY_SIZE(exynos_dsi_pixel_output_fmts); i++) {
+ if (exynos_dsi_pixel_output_fmts[i] == fmt)
+ return true;
+ }
+
+ return false;
+}
+
+static u32 *
+exynos_dsi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ u32 *input_fmts;
+
+ if (!exynos_dsi_pixel_output_fmt_supported(output_fmt))
+ /*
+ * Some bridge/display drivers are still not able to pass the
+ * correct format, so handle those pipelines by falling back
+ * to the default format till the supported formats finalized.
+ */
+ output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
You can move this ^ past the kmalloc() call, so in unlikely case the
kmalloc() fails, it would fail first.
I didn't get this point, what do we need to do if
exynos_dsi_pixel_output_fmt_supported returns false?
{
u32 *input_fmts;
input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
if (!input_fmts)
return NULL;
if (!exynos_dsi_pixel_output_fmt_supported(output_fmt))
/* ... the comment ... */
output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
input_fmts[0] = output_fmt;
*num_input_fmts = 1;
return input_fmts;
}
Re: [RESEND PATCH v11 11/18] drm: exynos: dsi: Add atomic_get_input_bus_fmts
On Wed, Jan 25, 2023 at 2:15 AM Marek Vasut wrote:
>
> On 1/23/23 16:12, Jagan Teki wrote:
>
> [...]
>
> > +static bool exynos_dsi_pixel_output_fmt_supported(u32 fmt)
> > +{
> > + int i;
>
> if (fmt == MEDIA_BUS_FMT_FIXED)
> return false;
>
> > + for (i = 0; i < ARRAY_SIZE(exynos_dsi_pixel_output_fmts); i++) {
> > + if (exynos_dsi_pixel_output_fmts[i] == fmt)
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static u32 *
> > +exynos_dsi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> > + struct drm_bridge_state *bridge_state,
> > + struct drm_crtc_state *crtc_state,
> > + struct drm_connector_state *conn_state,
> > + u32 output_fmt,
> > + unsigned int *num_input_fmts)
> > +{
> > + u32 *input_fmts;
> > +
> > + if (!exynos_dsi_pixel_output_fmt_supported(output_fmt))
> > + /*
> > + * Some bridge/display drivers are still not able to pass the
> > + * correct format, so handle those pipelines by falling back
> > + * to the default format till the supported formats finalized.
> > + */
> > + output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
>
> You can move this ^ past the kmalloc() call, so in unlikely case the
> kmalloc() fails, it would fail first.
I didn't get this point, what do we need to do if
exynos_dsi_pixel_output_fmt_supported returns false?
Jagan.
Re: [RESEND PATCH v11 00/18] drm: Add Samsung MIPI DSIM bridge
On 1/23/23 16:11, Jagan Teki wrote: This series supports common bridge support for Samsung MIPI DSIM which is used in Exynos and i.MX8MM SoC's. The final bridge supports both the Exynos and i.MX8M Mini/Nano/Plus. Patch 0001 - 0004: adding devm_drm_of_dsi_get_bridge Patch 0005 - 0006: optional PHY, PMS_P offset Patch 0007 : introduce hw_type Patch 0008 : fixing host init Patch 0009 : atomic_check Patch 0010 : input_bus_flags Patch 0011 : atomic_get_input_bus_fmts Patch 0012 - 0013: component vs bridge Patch 0014 : DSIM bridge Patch 0015 - 0016: i.MX8M Mini/Nano Patch 0017 - 0018: i.MX8M Plus Please drop chen.f...@nxp.com, narmstr...@linaro.org, jy0922.s...@samsung.com from CC, they bounce.
Re: [RESEND PATCH v11 13/18] drm: exynos: dsi: Add Exynos based host irq hooks
On 1/24/23 22:01, Jagan Teki wrote: On Wed, Jan 25, 2023 at 2:18 AM Marek Vasut wrote: On 1/23/23 16:12, Jagan Teki wrote: Enable and disable of te_gpio's are Exynos platform specific irq handling, so add the exynos based irq operations and hook them for exynos plat_data. If this is just an optional generic GPIO IRQ, why not keep it in the core code ? TE (tearing enable?) should be available on MX8M too. So far the discussion (since from initial versions) with Marek Szyprowski, seems to be available in Exynos. So, I keep it separate from the DSIM core. Isn't TE a generic GPIO IRQ ? If so, it is available also on i.MX8M .
Re: [RESEND PATCH v11 12/18] drm: exynos: dsi: Consolidate component and bridge
On 1/23/23 16:12, Jagan Teki wrote:
DSI host registration, attach and detach operations are quite
different for the component and bridge-based DRM drivers.
Supporting generic bridge driver to use both component and bridge
based DRM drivers can be tricky and would require additional host
related operation hooks.
Add host operation hooks for registering and unregistering Exynos
and generic drivers, where Exynos hooks are used in existing Exynos
component based DRM drivers and generic hooks are used in i.MX8M
bridge based DRM drivers.
Add host attach and detach operation hooks for Exynos component
DRM drivers and those get invoked while DSI core host attach and
detach gets called.
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
Changes for v11:
- none
Changes for v10:
- split from previous series patch
"drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge"
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 179 ++--
1 file changed, 140 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 7afbbe30d1d3..fc7f00ab01b4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -250,6 +250,8 @@ struct exynos_dsi_transfer {
u16 rx_done;
};
+struct exynos_dsi;
Is this forward declaration really necessary ? Can't the structures
below be reordered to get rid of this ?
#define DSIM_STATE_ENABLEDBIT(0)
#define DSIM_STATE_INITIALIZEDBIT(1)
#define DSIM_STATE_CMD_LPMBIT(2)
@@ -281,12 +283,19 @@ struct exynos_dsi_driver_data {
const unsigned int *reg_values;
};
+struct exynos_dsim_host_ops {
+ int (*register_host)(struct exynos_dsi *dsim);
+ void (*unregister_host)(struct exynos_dsi *dsim);
+ int (*attach)(struct exynos_dsi *dsim, struct mipi_dsi_device *device);
+ int (*detach)(struct exynos_dsi *dsim, struct mipi_dsi_device *device);
+};
+
struct exynos_dsi_plat_data {
enum exynos_dsi_type hw_type;
+ const struct exynos_dsim_host_ops *host_ops;
};
struct exynos_dsi {
- struct drm_encoder encoder;
struct mipi_dsi_host dsi_host;
struct drm_bridge bridge;
struct drm_bridge *out_bridge;
@@ -316,6 +325,12 @@ struct exynos_dsi {
const struct exynos_dsi_driver_data *driver_data;
const struct exynos_dsi_plat_data *plat_data;
+
+ void *priv;
+};
+
+struct exynos_dsi_enc {
+ struct drm_encoder encoder;
};
#define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
@@ -1319,10 +1334,11 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
{
- struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
+ struct exynos_dsi *dsim = (struct exynos_dsi *)dev_id;
Is the rename really needed ?
+ struct exynos_dsi_enc *dsi = dsim->priv;
Call this variable something else , like dsi_enc , and you shouldn't
need the rename above ...
struct drm_encoder *encoder = >encoder;
- if (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE)
+ if (dsim->state & DSIM_STATE_VIDOUT_AVAILABLE)
... and the rename here .
exynos_drm_crtc_te_handler(encoder->crtc);
return IRQ_HANDLED;
[...]
static void exynos_dsi_unbind(struct device *dev, struct device *master,
void *data)
{
- struct exynos_dsi *dsi = dev_get_drvdata(dev);
+ struct exynos_dsi *dsim = dev_get_drvdata(dev);
Please avoid the variable renames globally, that should simplify this
patch and remove unrelated changes.
- exynos_dsi_atomic_disable(>bridge, NULL);
+ dsim->bridge.funcs->atomic_disable(>bridge, NULL);
- mipi_dsi_host_unregister(>dsi_host);
+ mipi_dsi_host_unregister(>dsi_host);
}
[...]
With that fixed:
Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 13/18] drm: exynos: dsi: Add Exynos based host irq hooks
On Wed, Jan 25, 2023 at 2:18 AM Marek Vasut wrote: > > On 1/23/23 16:12, Jagan Teki wrote: > > Enable and disable of te_gpio's are Exynos platform specific > > irq handling, so add the exynos based irq operations and hook > > them for exynos plat_data. > > If this is just an optional generic GPIO IRQ, why not keep it in the > core code ? TE (tearing enable?) should be available on MX8M too. So far the discussion (since from initial versions) with Marek Szyprowski, seems to be available in Exynos. So, I keep it separate from the DSIM core. Jagan.
Re: [RESEND PATCH v11 08/18] drm: exynos: dsi: Handle proper host initialization
On 1/23/23 16:12, Jagan Teki wrote: From: Marek Szyprowski Host transfer() in the DSI master will invoke only when the DSI commands are sent from DSI devices like DSI Panel or DSI bridges and this host the transfer wouldn't invoke for I2C-based-DSI bridge drivers. Handling DSI host initialization in transfer calls misses the controller setup for I2C configured DSI bridges. This patch updates the DSI host initialization by calling host to init from bridge pre_enable as the bridge pre_enable API is invoked by core as it is common across all classes of DSI device drivers. The host init during pre_enable is conditional and not invoked for Exynos as existing downstream drm panels and bridges in Exynos are expecting the host initialization during DSI transfer. Reviewed-by: Frieder Schrempf Signed-off-by: Marek Szyprowski Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut Although this probably needs to be revisited.
Re: [RESEND PATCH v11 18/18] drm: bridge: samsung-dsim: Add i.MX8M Plus support
On 1/23/23 16:12, Jagan Teki wrote: From: Marek Vasut Add extras to support i.MX8M Plus. The main change is the removal of HS/VS/DE signal inversion in the LCDIFv3-DSIM glue logic, otherwise the implementation of this IP in i.MX8M Plus is very much compatible with the i.MX8M Mini/Nano one. Reviewed-by: Frieder Schrempf Acked-by: Robert Foss Signed-off-by: Marek Vasut Signed-off-by: Jagan Teki Self-review for completeness: Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 14/18] drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge
On 1/23/23 16:12, Jagan Teki wrote: [...] @@ -6738,6 +6738,15 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F:Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml F:drivers/gpu/drm/panel/panel-samsung-db7430.c +DRM DRIVER FOR SAMSUNG MIPI DSIM BRIDGE +M: Jagan Teki +M: Marek Szyprowski +M: Inki Dae Keep the list sorted. +S: Maintained +T: git git://anongit.freedesktop.org/drm/drm-misc +F: drivers/gpu/drm/bridge/samsung-dsim.c +F: include/drm/bridge/samsung-dsim.h + DRM DRIVER FOR SAMSUNG S6D27A1 PANELS M:Markuss Broks S:Maintained [...] With that fixed, Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 17/18] dt-bindings: display: exynos: dsim: Add NXP i.MX8M Plus support
On 1/23/23 16:12, Jagan Teki wrote: Samsung MIPI DSIM bridge can also be found in i.MX8M Plus SoC. Add dt-bingings for it. Acked-by: Rob Herring Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 15/18] dt-bindings: display: exynos: dsim: Add NXP i.MX8M Mini/Nano support
On 1/23/23 16:12, Jagan Teki wrote: Samsung MIPI DSIM bridge can also be found in i.MX8M Mini/Nano SoC. Add dt-bingings for it. Acked-by: Rob Herring Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 16/18] drm: bridge: samsung-dsim: Add i.MX8M Mini/Nano support
On 1/23/23 16:12, Jagan Teki wrote: Samsung MIPI DSIM master can also be found in i.MX8M Mini/Nano SoC. Add compatible and associated driver_data for it. Reviewed-by: Frieder Schrempf Acked-by: Robert Foss Reviewed-by: Laurent Pinchart Signed-off-by: Marek Szyprowski Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 10/18] drm: exynos: dsi: Add input_bus_flags
On 1/23/23 16:12, Jagan Teki wrote: LCDIF-DSIM glue logic inverts the HS/VS/DE signals and expecting the i.MX8M Mini/Nano DSI host to add additional Data Enable signal active low (DE_LOW). This makes the valid data transfer on each horizontal line. So, add additional bus flags DE_LOW setting via input_bus_flags for i.MX8M Mini/Nano platforms. Reviewed-by: Frieder Schrempf Suggested-by: Marek Vasut Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut
Re: [RESEND PATCH v11 09/18] drm: exynos: dsi: Add atomic check
On 1/23/23 16:12, Jagan Teki wrote: Look like an explicit fixing up of mode_flags is required for DSIM IP present in i.MX8M Mini/Nano SoCs. At least the LCDIF + DSIM needs active low sync polarities in order to correlate the correct sync flags of the surrounding components in the chain to make sure the whole pipeline can work properly. On the other hand the i.MX 8M Mini Applications Processor Reference Manual, Rev. 3, 11/2020 says. "13.6.3.5.2 RGB interface Vsync, Hsync, and VDEN are active high signals." i.MX 8M Mini Applications Processor Reference Manual Rev. 3, 11/2020 3.6.3.5.2 RGB interface i.MX 8M Nano Applications Processor Reference Manual Rev. 2, 07/2022 13.6.2.7.2 RGB interface both claim "Vsync, Hsync, and VDEN are active high signals.", the LCDIF must generate inverted HS/VS/DE signals, i.e. active LOW. No clear evidence about whether it can be documentation issues or something, so added proper comments on the code. Comments are suggested by Marek Vasut. Reviewed-by: Frieder Schrempf Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut
[PATCH v4] drm/i915/uncore: Use GT message helpers in uncore
From: John Harrison
Uncore is really part of the GT. So use the GT specific debug/error
message helpers so as to get the GT index in the prints.
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/intel_uncore.c | 133 +---
1 file changed, 63 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 8dee9e62a73ee..4e357477c6592 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -25,6 +25,7 @@
#include
#include "gt/intel_engine_regs.h"
+#include "gt/intel_gt_print.h"
#include "gt/intel_gt_regs.h"
#include "i915_drv.h"
@@ -83,8 +84,7 @@ static void mmio_debug_resume(struct intel_uncore *uncore)
uncore->debug->unclaimed_mmio_check =
uncore->debug->saved_mmio_check;
if (check_for_unclaimed_mmio(uncore))
- drm_info(>i915->drm,
-"Invalid mmio detected during user access\n");
+ gt_info(uncore->gt, "Invalid mmio detected during user
access\n");
spin_unlock(>debug->lock);
}
@@ -179,9 +179,9 @@ static inline void
fw_domain_wait_ack_clear(const struct intel_uncore_forcewake_domain *d)
{
if (wait_ack_clear(d, FORCEWAKE_KERNEL)) {
- drm_err(>uncore->i915->drm,
- "%s: timed out waiting for forcewake ack to clear.\n",
- intel_uncore_forcewake_domain_to_str(d->id));
+ gt_err(d->uncore->gt,
+ "%s: timed out waiting for forcewake ack to clear.\n",
+ intel_uncore_forcewake_domain_to_str(d->id));
add_taint_for_CI(d->uncore->i915, TAINT_WARN); /* CI now
unreliable */
}
}
@@ -228,12 +228,11 @@ fw_domain_wait_ack_with_fallback(const struct
intel_uncore_forcewake_domain *d,
fw_clear(d, FORCEWAKE_KERNEL_FALLBACK);
} while (!ack_detected && pass++ < 10);
- drm_dbg(>uncore->i915->drm,
- "%s had to use fallback to %s ack, 0x%x (passes %u)\n",
- intel_uncore_forcewake_domain_to_str(d->id),
- type == ACK_SET ? "set" : "clear",
- fw_ack(d),
- pass);
+ gt_dbg(d->uncore->gt, "%s had to use fallback to %s ack, 0x%x (passes
%u)\n",
+ intel_uncore_forcewake_domain_to_str(d->id),
+ type == ACK_SET ? "set" : "clear",
+ fw_ack(d),
+ pass);
return ack_detected ? 0 : -ETIMEDOUT;
}
@@ -258,9 +257,8 @@ static inline void
fw_domain_wait_ack_set(const struct intel_uncore_forcewake_domain *d)
{
if (wait_ack_set(d, FORCEWAKE_KERNEL)) {
- drm_err(>uncore->i915->drm,
- "%s: timed out waiting for forcewake ack request.\n",
- intel_uncore_forcewake_domain_to_str(d->id));
+ gt_err(d->uncore->gt, "%s: timed out waiting for forcewake ack
request.\n",
+ intel_uncore_forcewake_domain_to_str(d->id));
add_taint_for_CI(d->uncore->i915, TAINT_WARN); /* CI now
unreliable */
}
}
@@ -366,9 +364,9 @@ static void __gen6_gt_wait_for_thread_c0(struct
intel_uncore *uncore)
* w/a for a sporadic read returning 0 by waiting for the GT
* thread to wake up.
*/
- drm_WARN_ONCE(>i915->drm,
- wait_for_atomic_us(gt_thread_status(uncore) == 0, 5000),
- "GT thread status wait timed out\n");
+ gt_WARN_ONCE(uncore->gt,
+wait_for_atomic_us(gt_thread_status(uncore) == 0, 5000),
+"GT thread status wait timed out\n");
}
static void fw_domains_get_with_thread_status(struct intel_uncore *uncore,
@@ -402,8 +400,7 @@ static void __gen6_gt_wait_for_fifo(struct intel_uncore
*uncore)
if (wait_for_atomic((n = fifo_free_entries(uncore)) >
GT_FIFO_NUM_RESERVED_ENTRIES,
GT_FIFO_TIMEOUT_MS)) {
- drm_dbg(>i915->drm,
- "GT_FIFO timeout, entries: %u\n", n);
+ gt_dbg(uncore->gt, "GT_FIFO timeout, entries: %u\n", n);
return;
}
}
@@ -476,7 +473,7 @@ intel_uncore_forcewake_reset(struct intel_uncore *uncore)
break;
if (--retry_count == 0) {
- drm_err(>i915->drm, "Timed out waiting for
forcewake timers to finish\n");
+ gt_err(uncore->gt, "Timed out waiting for forcewake
timers to finish\n");
break;
}
@@ -484,7 +481,7 @@ intel_uncore_forcewake_reset(struct intel_uncore *uncore)
cond_resched();
}
- drm_WARN_ON(>i915->drm, active_domains);
+ gt_WARN_ON(uncore->gt, active_domains);
fw = uncore->fw_domains_active;
if (fw)
Re: [RESEND PATCH v11 07/18] drm: exynos: dsi: Introduce hw_type platform data
On 1/23/23 16:12, Jagan Teki wrote: Samsung MIPI DSIM controller is common DSI IP that can be used in various SoCs like Exynos, i.MX8M Mini/Nano/Plus. Add hw_type enum via platform_data so that accessing the different controller data between various platforms becomes easy and meaningful. Reviewed-by: Frieder Schrempf Suggested-by: Marek Szyprowski Signed-off-by: Jagan Teki Reviewed-by: Marek Vasut
Re: [PATCH v3 01/10] iommu: Add a gfp parameter to iommu_map()
On Mon, Jan 23, 2023 at 04:35:54PM -0400, Jason Gunthorpe wrote:
> The internal mechanisms support this, but instead of exposting the gfp to
> the caller it wrappers it into iommu_map() and iommu_map_atomic()
>
> Fix this instead of adding more variants for GFP_KERNEL_ACCOUNT.
>
> Reviewed-by: Kevin Tian
> Signed-off-by: Jason Gunthorpe
> ---
> arch/arm/mm/dma-mapping.c | 11 ++
> .../drm/nouveau/nvkm/subdev/instmem/gk20a.c | 3 ++-
> drivers/gpu/drm/tegra/drm.c | 2 +-
> drivers/gpu/host1x/cdma.c | 2 +-
> drivers/infiniband/hw/usnic/usnic_uiom.c | 4 ++--
> drivers/iommu/dma-iommu.c | 2 +-
> drivers/iommu/iommu.c | 22 +--
> drivers/iommu/iommufd/pages.c | 6 +++--
> drivers/media/platform/qcom/venus/firmware.c | 2 +-
> drivers/net/ipa/ipa_mem.c | 6 +++--
> drivers/net/wireless/ath/ath10k/snoc.c| 2 +-
> drivers/net/wireless/ath/ath11k/ahb.c | 4 ++--
> drivers/remoteproc/remoteproc_core.c | 5 +++--
Reviewed-by: Mathieu Poirier
> drivers/vfio/vfio_iommu_type1.c | 9
> drivers/vhost/vdpa.c | 2 +-
> include/linux/iommu.h | 4 ++--
> 16 files changed, 48 insertions(+), 38 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index c135f6e37a00ca..8bc01071474ab7 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -984,7 +984,8 @@ __iommu_create_mapping(struct device *dev, struct page
> **pages, size_t size,
>
> len = (j - i) << PAGE_SHIFT;
> ret = iommu_map(mapping->domain, iova, phys, len,
> - __dma_info_to_prot(DMA_BIDIRECTIONAL, attrs));
> + __dma_info_to_prot(DMA_BIDIRECTIONAL, attrs),
> + GFP_KERNEL);
> if (ret < 0)
> goto fail;
> iova += len;
> @@ -1207,7 +1208,8 @@ static int __map_sg_chunk(struct device *dev, struct
> scatterlist *sg,
>
> prot = __dma_info_to_prot(dir, attrs);
>
> - ret = iommu_map(mapping->domain, iova, phys, len, prot);
> + ret = iommu_map(mapping->domain, iova, phys, len, prot,
> + GFP_KERNEL);
> if (ret < 0)
> goto fail;
> count += len >> PAGE_SHIFT;
> @@ -1379,7 +1381,8 @@ static dma_addr_t arm_iommu_map_page(struct device
> *dev, struct page *page,
>
> prot = __dma_info_to_prot(dir, attrs);
>
> - ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len,
> prot);
> + ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len,
> + prot, GFP_KERNEL);
> if (ret < 0)
> goto fail;
>
> @@ -1443,7 +1446,7 @@ static dma_addr_t arm_iommu_map_resource(struct device
> *dev,
>
> prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
>
> - ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
> + ret = iommu_map(mapping->domain, dma_addr, addr, len, prot, GFP_KERNEL);
> if (ret < 0)
> goto fail;
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
> index 648ecf5a8fbc2a..a4ac94a2ab57fc 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
> @@ -475,7 +475,8 @@ gk20a_instobj_ctor_iommu(struct gk20a_instmem *imem, u32
> npages, u32 align,
> u32 offset = (r->offset + i) << imem->iommu_pgshift;
>
> ret = iommu_map(imem->domain, offset, node->dma_addrs[i],
> - PAGE_SIZE, IOMMU_READ | IOMMU_WRITE);
> + PAGE_SIZE, IOMMU_READ | IOMMU_WRITE,
> + GFP_KERNEL);
> if (ret < 0) {
> nvkm_error(subdev, "IOMMU mapping failure: %d\n", ret);
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 7bd2e65c2a16c5..6ca9f396e55be4 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1057,7 +1057,7 @@ void *tegra_drm_alloc(struct tegra_drm *tegra, size_t
> size, dma_addr_t *dma)
>
> *dma = iova_dma_addr(>carveout.domain, alloc);
> err = iommu_map(tegra->domain, *dma, virt_to_phys(virt),
> - size, IOMMU_READ | IOMMU_WRITE);
> + size, IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
> if (err < 0)
> goto free_iova;
>
> diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> index 103fda055394ab..4ddfcd2138c95b 100644
> --- a/drivers/gpu/host1x/cdma.c
> +++ b/drivers/gpu/host1x/cdma.c
> @@ -105,7 +105,7 @@ static int
Re: [PATCH] drm/msm/dsi: properly handle the case of empty OPP table in dsi_mgr_bridge_mode_valid
On 1/24/2023 12:36 PM, Dmitry Baryshkov wrote:
It was left unnoticed during the review that even if there is no OPP
table in device tree, one will be created by a call to the function
devm_pm_opp_set_clkname(). This leads to dsi_mgr_bridge_mode_valid()
rejecting all modes if DT contains no OPP table for the DSI host.
Rework dsi_mgr_bridge_mode_valid() to handle this case by actually
checking that the table is populated with frequency entries before
returning an error.
Fixes: 8328041b8c82 ("drm/msm/dsi: implement opp table based check for
dsi_mgr_bridge_mode_valid()")
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index b20fddb534a7..1bbac72dad35 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -456,18 +456,19 @@ static enum drm_mode_status
dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
byte_clk_rate = dsi_byte_clk_get_rate(host, IS_BONDED_DSI(), mode);
- /*
-* fail all errors except -ENODEV as that could mean that opp
-* table is not yet implemented
-*/
opp = dev_pm_opp_find_freq_ceil(>dev, _clk_rate);
- if (IS_ERR(opp)) {
- if (PTR_ERR(opp) == -ERANGE)
+ if (!IS_ERR(opp)) {
+ dev_pm_opp_put(opp);
+ } else if (PTR_ERR(opp) == -ERANGE) {
+ /*
+* An empty table is created by devm_pm_opp_set_clkname() even
+* if there is none. Thus find_freq_ceil will still return
+* -ERANGE in such case.
+*/
+ if (dev_pm_opp_get_opp_count(>dev) != 0)
return MODE_CLOCK_RANGE;
- else if (PTR_ERR(opp) != -ENODEV)
- return MODE_ERROR;
} else {
- dev_pm_opp_put(opp);
+ return MODE_ERROR;
}
return msm_dsi_host_check_dsc(host, mode);
Re: [RESEND PATCH v11 13/18] drm: exynos: dsi: Add Exynos based host irq hooks
On 1/23/23 16:12, Jagan Teki wrote: Enable and disable of te_gpio's are Exynos platform specific irq handling, so add the exynos based irq operations and hook them for exynos plat_data. If this is just an optional generic GPIO IRQ, why not keep it in the core code ? TE (tearing enable?) should be available on MX8M too.
Re: [RESEND PATCH v11 11/18] drm: exynos: dsi: Add atomic_get_input_bus_fmts
On 1/23/23 16:12, Jagan Teki wrote:
[...]
+static bool exynos_dsi_pixel_output_fmt_supported(u32 fmt)
+{
+ int i;
if (fmt == MEDIA_BUS_FMT_FIXED)
return false;
+ for (i = 0; i < ARRAY_SIZE(exynos_dsi_pixel_output_fmts); i++) {
+ if (exynos_dsi_pixel_output_fmts[i] == fmt)
+ return true;
+ }
+
+ return false;
+}
+
+static u32 *
+exynos_dsi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state,
+u32 output_fmt,
+unsigned int *num_input_fmts)
+{
+ u32 *input_fmts;
+
+ if (!exynos_dsi_pixel_output_fmt_supported(output_fmt))
+ /*
+* Some bridge/display drivers are still not able to pass the
+* correct format, so handle those pipelines by falling back
+* to the default format till the supported formats finalized.
+*/
+ output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
You can move this ^ past the kmalloc() call, so in unlikely case the
kmalloc() fails, it would fail first.
+ input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
Delete from here ...
+ switch (output_fmt) {
+ case MEDIA_BUS_FMT_FIXED:
+ input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+ default:
+ input_fmts[0] = output_fmt;
+ break;
+ }
... until here, and do simple:
input_fmts[0] = output_fmt;
The effect should be the same, the code should be simpler and faster.
+ *num_input_fmts = 1;
[...]
linux-next: duplicate patches in the drm tree
Hi all,
The following commits are also in Linus Torvalds' tree as different
commits (but the same patches):
0c8a6e9ea232 ("drm/i915: re-disable RC6p on Sandy Bridge")
14ec40a88210 ("drm/i915/selftests: Unwind hugepages to drop wakeref on error")
3db9d590557d ("drm/i915/gt: Reset twice")
4f0755c2faf7 ("drm/i915: Reserve enough fence slot for i915_vma_unbind_async")
bed4b455cf53 ("drm/i915: Fix potential context UAFs")
--
Cheers,
Stephen Rothwell
pgpZp2ZSz4iQ1.pgp
Description: OpenPGP digital signature
[PATCH] drm/msm/dsi: properly handle the case of empty OPP table in dsi_mgr_bridge_mode_valid
It was left unnoticed during the review that even if there is no OPP
table in device tree, one will be created by a call to the function
devm_pm_opp_set_clkname(). This leads to dsi_mgr_bridge_mode_valid()
rejecting all modes if DT contains no OPP table for the DSI host.
Rework dsi_mgr_bridge_mode_valid() to handle this case by actually
checking that the table is populated with frequency entries before
returning an error.
Fixes: 8328041b8c82 ("drm/msm/dsi: implement opp table based check for
dsi_mgr_bridge_mode_valid()")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index b20fddb534a7..1bbac72dad35 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -456,18 +456,19 @@ static enum drm_mode_status
dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
byte_clk_rate = dsi_byte_clk_get_rate(host, IS_BONDED_DSI(), mode);
- /*
-* fail all errors except -ENODEV as that could mean that opp
-* table is not yet implemented
-*/
opp = dev_pm_opp_find_freq_ceil(>dev, _clk_rate);
- if (IS_ERR(opp)) {
- if (PTR_ERR(opp) == -ERANGE)
+ if (!IS_ERR(opp)) {
+ dev_pm_opp_put(opp);
+ } else if (PTR_ERR(opp) == -ERANGE) {
+ /*
+* An empty table is created by devm_pm_opp_set_clkname() even
+* if there is none. Thus find_freq_ceil will still return
+* -ERANGE in such case.
+*/
+ if (dev_pm_opp_get_opp_count(>dev) != 0)
return MODE_CLOCK_RANGE;
- else if (PTR_ERR(opp) != -ENODEV)
- return MODE_ERROR;
} else {
- dev_pm_opp_put(opp);
+ return MODE_ERROR;
}
return msm_dsi_host_check_dsc(host, mode);
--
2.39.0
Re: [PATCH v3 2/2] drm/msm/dsi: implement opp table based check for dsi_mgr_bridge_mode_valid()
On 12/01/2023 02:16, Abhinav Kumar wrote:
Currently there is no protection against a user trying to set
an unsupported mode on DSI. Implement a check based on the opp
table whether the byte clock for the mode can be supported by
validating whether an opp table entry exists.
For devices which have not added opp table support yet, skip
this check otherwise it will break bootup on those devices.
changes in v3:
- make the comment shorter
- handle all errors except ENODEV
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/15
Reported-by: Rob Clark
Signed-off-by: Abhinav Kumar
I finally acquired the db410c again. And... this patch breaks support
for the board. We were wrong on the assumption that the code will return
-ENODEV when there is no opp table, my bad. devm_pm_opp_set_clkname(),
called from msm_dsi_host_init(), allocates an empty table if one doesn't
exist in DT.
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 3a1417397283..b20fddb534a7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -450,6 +450,25 @@ static enum drm_mode_status
dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
int id = dsi_mgr_bridge_get_id(bridge);
struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
struct mipi_dsi_host *host = msm_dsi->host;
+ struct platform_device *pdev = msm_dsi->pdev;
+ struct dev_pm_opp *opp;
+ unsigned long byte_clk_rate;
+
+ byte_clk_rate = dsi_byte_clk_get_rate(host, IS_BONDED_DSI(), mode);
+
+ /*
+* fail all errors except -ENODEV as that could mean that opp
+* table is not yet implemented
+*/
+ opp = dev_pm_opp_find_freq_ceil(>dev, _clk_rate);
+ if (IS_ERR(opp)) {
+ if (PTR_ERR(opp) == -ERANGE)
+ return MODE_CLOCK_RANGE;
+ else if (PTR_ERR(opp) != -ENODEV)
+ return MODE_ERROR;
+ } else {
+ dev_pm_opp_put(opp);
+ }
return msm_dsi_host_check_dsc(host, mode);
}
--
With best wishes
Dmitry
Re: [PATCH v2 1/4] memcg: Track exported dma-buffers
On Tue, Jan 24, 2023 at 03:59:58PM +0100, Michal Hocko wrote: > On Mon 23-01-23 19:17:23, T.J. Mercier wrote: > > When a buffer is exported to userspace, use memcg to attribute the > > buffer to the allocating cgroup until all buffer references are > > released. > > Is there any reason why this memory cannot be charged during the > allocation (__GFP_ACCOUNT used)? > Also you do charge and account the memory but underlying pages do not > know about their memcg (this is normally done with commit_charge for > user mapped pages). This would become a problem if the memory is > migrated for example. I don't think this is movable memory. > This also means that you have to maintain memcg > reference outside of the memcg proper which is not really nice either. > This mimicks tcp kmem limit implementation which I really have to say I > am not a great fan of and this pattern shouldn't be coppied. > I think we should keep the discussion on technical merits instead of personal perference. To me using skmem like interface is totally fine but the pros/cons need to be very explicit and the clear reasons to select that option should be included. To me there are two options: 1. Using skmem like interface as this patch series: The main pros of this option is that it is very simple. Let me list down the cons of this approach: a. There is time window between the actual memory allocation/free and the charge and uncharge and [un]charge happen when the whole memory is allocated or freed. I think for the charge path that might not be a big issue but on the uncharge, this can cause issues. The application and the potential shrinkers have freed some of this dmabuf memory but until the whole dmabuf is freed, the memcg uncharge will not happen. This can consequences on reclaim and oom behavior of the application. b. Due to the usage model i.e. a central daemon allocating the dmabuf memory upfront, there is a requirement to have a memcg charge transfer functionality to transfer the charge from the central daemon to the client applications. This does introduce complexity and avenues of weird reclaim and oom behavior. 2. Allocate and charge the memory on page fault by actual user In this approach, the memory is not allocated upfront by the central daemon but rather on the page fault by the client application and the memcg charge happen at the same time. The only cons I can think of is this approach is more involved and may need some clever tricks to track the page on the free patch i.e. we to decrement the dmabuf memcg stat on free path. Maybe a page flag. The pros of this approach is there is no need have a charge transfer functionality and the charge/uncharge being closely tied to the actual memory allocation and free. Personally I would prefer the second approach but I don't want to just block this work if the dmabuf folks are ok with the cons mentioned of the first approach. thanks, Shakeel
Re: [RESEND PATCH v11 00/18] drm: Add Samsung MIPI DSIM bridge
On Mon, Jan 23, 2023 at 8:42 PM Jagan Teki wrote: > > This series supports common bridge support for Samsung MIPI DSIM > which is used in Exynos and i.MX8MM SoC's. > > The final bridge supports both the Exynos and i.MX8M Mini/Nano/Plus. > > Patch 0001 - 0004: adding devm_drm_of_dsi_get_bridge > > Patch 0005 - 0006: optional PHY, PMS_P offset > > Patch 0007 : introduce hw_type > > Patch 0008 : fixing host init > > Patch 0009 : atomic_check > > Patch 0010 : input_bus_flags > > Patch 0011 : atomic_get_input_bus_fmts > > Patch 0012 - 0013: component vs bridge > > Patch 0014 : DSIM bridge > > Patch 0015 - 0016: i.MX8M Mini/Nano > > Patch 0017 - 0018: i.MX8M Plus > > Changes for v11: > - collect RB from Frieder Schrempf > - collect ACK from Rob > - collect ACK from Robert > - fix BIT macro replacements > - fix checkpatch --strict warnings > - fix unneeded commit text > - drop extra lines > > Changes for v10: > - rebase on drm-misc-next > - add drm_of_dsi_find_panel_or_bridge > - add devm_drm_of_dsi_get_bridge > - fix host initialization (Thanks to Marek Szyprowski) > - rearrange the tiny patches for easy to review > - update simple names for enum hw_type > - add is_hw_exynos macro > - rework on commit messages > > Changes for v9: > - rebase on drm-misc-next > - drop drm bridge attach fix for Exynos > - added prepare_prev_first flag > - added pre_enable_prev_first flag > - fix bridge chain order for exynos > - added fix for Exynos host init for first DSI transfer > - added MEDIA_BUS_FMT_FIXED > - return MEDIA_BUS_FMT_RGB888_1X24 output_fmt if supported output_fmt > list is unsupported. > - added MEDIA_BUS_FMT_YUYV10_1X20 > - added MEDIA_BUS_FMT_YUYV12_1X24 > > Changes for v8: > * fixed comment lines > * fixed commit messages > * fixed video mode bits > * collect Marek Ack > * fixed video mode bit names > * update input formats logic > * added imx8mplus support > > Changes for v7: > * fix the drm bridge attach chain for exynos drm dsi driver > * fix the hw_type checking logic > > Changes for v6: > * handle previous bridge for exynos dsi while attaching bridge > > Changes for v5: > * bridge changes to support multi-arch > * updated and clear commit messages > * add hw_type via plat data > * removed unneeded quirk > * rebased on linux-next > > Changes for v4: > * include Inki Dae in MAINTAINERS > * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > * update init handling to ensure host init done on first cmd transfer > > Changes for v3: > * fix the mult-arch build > * fix dsi host init > * updated commit messages > > Changes for v2: > * fix bridge handling > * fix dsi host init > * correct the commit messages > > Tested in Engicam i.Core MX8M Mini SoM. > > Repo: > https://github.com/openedev/kernel/tree/imx8mm-dsi-v11 > > v10: > https://lore.kernel.org/all/20221214125907.376148-1-ja...@amarulasolutions.com/ > > Any inputs? > Jagan. > > Jagan Teki (16): > drm: of: Lookup if child node has DSI panel or bridge > drm: bridge: panel: Add devm_drm_of_dsi_get_bridge helper > drm: exynos: dsi: Drop explicit call to bridge detach > drm: exynos: dsi: Switch to devm_drm_of_dsi_get_bridge > drm: exynos: dsi: Mark PHY as optional > drm: exynos: dsi: Add platform PLL_P (PMS_P) offset > drm: exynos: dsi: Introduce hw_type platform data > drm: exynos: dsi: Add atomic check > drm: exynos: dsi: Add input_bus_flags > drm: exynos: dsi: Add atomic_get_input_bus_fmts > drm: exynos: dsi: Consolidate component and bridge > drm: exynos: dsi: Add Exynos based host irq hooks > drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge > dt-bindings: display: exynos: dsim: Add NXP i.MX8M Mini/Nano support > drm: bridge: samsung-dsim: Add i.MX8M Mini/Nano support > dt-bindings: display: exynos: dsim: Add NXP i.MX8M Plus support > > Marek Szyprowski (1): > drm: exynos: dsi: Handle proper host initialization > > Marek Vasut (1): > drm: bridge: samsung-dsim: Add i.MX8M Plus support Can anyone pick this series? Thanks, Jagan.
Re: [PATCH v2 18/21] drm/amd/display: Fallback to 2020_YCBCR if the pixel encoding is not RGB
On 1/24/23 10:37, Harry Wentland wrote: > > > On 1/23/23 15:30, Sebastian Wick wrote: >> A new property to control YCC and subsampling would be the more >> complete path here. If we actually want to fix this in the short-term >> though, we should handle the YCC and RGB Colorspace values as >> equivalent, everywhere. Technically we're breaking the user space API >> here so it should be documented on the KMS property and other drivers >> must be adjusted accordingly as well. >> > > Could someone point me to a userspace that uses this currently? > To elaborate a bit more... A driver has always had the ability to pick the wire format, whether it'd be RGB or YCbCr (444, or 420). In some cases that selection is required in order to satisfy bandwidth requirements. In others we follow a certain policy to ensure similar behaviors between our Windows and Linux drivers. I don't think it makes sense for userspace to control this. Based on what I see I am not convinced the entirety of the colorspace definition has a corresponding implementation in an upstream, canonical userspace, hence my question. Not even an IGT test existed when I started looking at this. In the absence of a missing userspace implementation I am not convinced we're breaking anything. Even then, this was never implemented in amdgpu so there is no way this regresses any existing behavior. Harry > Harry > >> On Fri, Jan 13, 2023 at 5:26 PM Harry Wentland >> wrote: >>> >>> From: Joshua Ashton >>> >>> Userspace might not aware whether we're sending RGB or YCbCr >>> data to the display. If COLOR_SPACE_2020_RGB_FULLRANGE is >>> requested but the output encoding is YCbCr we should >>> send COLOR_SPACE_2020_YCBCR. >>> >>> Signed-off-by: Joshua Ashton >>> Signed-off-by: Harry Wentland >>> Cc: Pekka Paalanen >>> Cc: Sebastian Wick >>> Cc: vitaly.pros...@amd.com >>> Cc: Joshua Ashton >>> Cc: dri-devel@lists.freedesktop.org >>> Cc: amd-...@lists.freedesktop.org >>> Reviewed-by: Harry Wentland >>> --- >>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 - >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> index f74b125af31f..16940ea61b59 100644 >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> @@ -5184,7 +5184,10 @@ get_output_color_space(const struct dc_crtc_timing >>> *dc_crtc_timing, >>> color_space = COLOR_SPACE_ADOBERGB; >>> break; >>> case DRM_MODE_COLORIMETRY_BT2020_RGB: >>> - color_space = COLOR_SPACE_2020_RGB_FULLRANGE; >>> + if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) >>> + color_space = COLOR_SPACE_2020_RGB_FULLRANGE; >>> + else >>> + color_space = COLOR_SPACE_2020_YCBCR; >>> break; >>> case DRM_MODE_COLORIMETRY_BT2020_YCC: >>> color_space = COLOR_SPACE_2020_YCBCR; >>> -- >>> 2.39.0 >>> >> >
Re: [PATCH v2 1/4] memcg: Track exported dma-buffers
On Tue, Jan 24, 2023 at 7:00 AM Michal Hocko wrote: > > On Mon 23-01-23 19:17:23, T.J. Mercier wrote: > > When a buffer is exported to userspace, use memcg to attribute the > > buffer to the allocating cgroup until all buffer references are > > released. > > Is there any reason why this memory cannot be charged during the > allocation (__GFP_ACCOUNT used)? My main motivation was to keep code changes away from exporters and implement the accounting in one common spot for all of them. This is a bit of a carryover from a previous approach [1] where there was some objection to pushing off this work onto exporters and forcing them to adapt, but __GFP_ACCOUNT does seem like a smaller burden than before at least initially. However in order to support charge transfer between cgroups with __GFP_ACCOUNT we'd need to be able to get at the pages backing dmabuf objects, and the exporters are the ones with that access. Meaning I think we'd have to add some additional dma_buf_ops to achieve that, which was the objection from [1]. [1] https://lore.kernel.org/lkml/5cc27a05-8131-ce9b-dea1-5c75e9942...@amd.com/ > > Also you do charge and account the memory but underlying pages do not > know about their memcg (this is normally done with commit_charge for > user mapped pages). This would become a problem if the memory is > migrated for example. Hmm, what problem do you see in this situation? If the backing pages are to be migrated that requires the cooperation of the exporter, which currently has no influence on how the cgroup charging is done and that seems fine. (Unless you mean migrating the charge across cgroups? In which case that's the next patch.) > This also means that you have to maintain memcg > reference outside of the memcg proper which is not really nice either. > This mimicks tcp kmem limit implementation which I really have to say I > am not a great fan of and this pattern shouldn't be coppied. > Ah, what can I say. This way looked simple to me. I think otherwise we're back to making all exporters do more stuff for the accounting. > Also you are not really saying anything about the oom behavior. With > this implementation the kernel will try to reclaim the memory and even > trigger the memcg oom killer if the request size is <= 8 pages. Is this > a desirable behavior? It will try to reclaim some memory, but not the dmabuf pages right? Not *yet* anyway. This behavior sounds expected to me. I would only expect it to be surprising for cgroups making heavy use of dmabufs (that weren't accounted before) *and* with hard limits already very close to actual usage. I remember Johannes mentioning that what counts under memcg use is already a bit of a moving target. > -- > Michal Hocko > SUSE Labs
Re: [Intel-gfx] [PATCH v6 5/6] drm/i915/pxp: Trigger the global teardown for before suspending
On Tue, Jan 24, 2023 at 10:08:52AM -0800, Juston Li wrote:
> On Tue, 2023-01-24 at 10:17 -0500, Rodrigo Vivi wrote:
> > On Mon, Jan 23, 2023 at 09:31:49PM -0800, Alan Previn wrote:
> > > A driver bug was recently discovered where the security firmware
> > > was
> > > receiving internal HW signals indicating that session key
> > > expirations
> > > had occurred. Architecturally, the firmware was expecting a
> > > response
> > > from the GuC to acknowledge the event with the firmware side.
> > > However the OS was in a suspended state and GuC had been reset.
> > >
> > > Internal specifications actually required the driver to ensure
> > > that all active sessions be properly cleaned up in such cases where
> > > the system is suspended and the GuC potentially unable to respond.
> > >
> > > This patch adds the global teardown code in i915's suspend_prepare
> > > code path.
> > >
> > > Signed-off-by: Alan Previn
> > > Reviewed-by: Juston Li
> >
> > it should probably contain some revision history above so the
> > reviewer can know what changed from the time he reviewed and now.
> >
> > it looks we have another function that I don't remember...
> > Juston, could you please confirm your review stands in this version?
> > if so feel free to add my ack with that.
>
> Sure, v6 is:
> Reviewed-by: Juston Li
Acked-by: Rodrigo Vivi
>
> Appreciate the reviews/revisions Rodrigo and Alan!
>
> >
> > > ---
> > > drivers/gpu/drm/i915/pxp/intel_pxp.c | 65
> > > +---
> > > drivers/gpu/drm/i915/pxp/intel_pxp.h | 1 +
> > > drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 2 +-
> > > drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 6 +-
> > > drivers/gpu/drm/i915/pxp/intel_pxp_session.h | 5 ++
> > > 5 files changed, 66 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > > b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > > index cfc9af8b3d21..9d4c7724e98e 100644
> > > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > > @@ -270,6 +270,60 @@ static bool pxp_component_bound(struct
> > > intel_pxp *pxp)
> > > return bound;
> > > }
> > >
> > > +static int __pxp_global_teardown_final(struct intel_pxp *pxp)
> > > +{
> > > + if (!pxp->arb_is_valid)
> > > + return 0;
> > > + /*
> > > + * To ensure synchronous and coherent session teardown
> > > completion
> > > + * in response to suspend or shutdown triggers, don't use a
> > > worker.
> > > + */
> > > + intel_pxp_mark_termination_in_progress(pxp);
> > > + intel_pxp_terminate(pxp, false);
> > > +
> > > + if (!wait_for_completion_timeout(>termination,
> > > msecs_to_jiffies(250)))
> > > + return -ETIMEDOUT;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int __pxp_global_teardown_restart(struct intel_pxp *pxp)
> > > +{
> > > + if (pxp->arb_is_valid)
> > > + return 0;
> > > + /*
> > > + * The arb-session is currently inactive and we are doing a
> > > reset and restart
> > > + * due to a runtime event. Use the worker that was designed
> > > for this.
> > > + */
> > > + pxp_queue_termination(pxp);
> > > +
> > > + if (!wait_for_completion_timeout(>termination,
> > > msecs_to_jiffies(250)))
> > > + return -ETIMEDOUT;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +void intel_pxp_end(struct intel_pxp *pxp)
> > > +{
> > > + struct drm_i915_private *i915 = pxp->ctrl_gt->i915;
> > > + intel_wakeref_t wakeref;
> > > +
> > > + if (!intel_pxp_is_enabled(pxp))
> > > + return;
> > > +
> > > + wakeref = intel_runtime_pm_get(>runtime_pm);
> > > +
> > > + mutex_lock(>arb_mutex);
> > > +
> > > + if (__pxp_global_teardown_final(pxp))
> > > + drm_dbg(>drm, "PXP end timed out\n");
> > > +
> > > + mutex_unlock(>arb_mutex);
> > > +
> > > + intel_pxp_fini_hw(pxp);
> > > + intel_runtime_pm_put(>runtime_pm, wakeref);
> > > +}
> > > +
> > > /*
> > > * the arb session is restarted from the irq work when we receive
> > > the
> > > * termination completion interrupt
> > > @@ -286,16 +340,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
> > >
> > > mutex_lock(>arb_mutex);
> > >
> > > - if (pxp->arb_is_valid)
> > > - goto unlock;
> > > -
> > > - pxp_queue_termination(pxp);
> > > -
> > > - if (!wait_for_completion_timeout(>termination,
> > > - msecs_to_jiffies(250))) {
> > > - ret = -ETIMEDOUT;
> > > + ret = __pxp_global_teardown_restart(pxp);
> > > + if (ret)
> > > goto unlock;
> > > - }
> > >
> > > /* make sure the compiler doesn't optimize the double
> > > access */
> > > barrier();
> > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> > >
Re: [PATCH v6 5/6] drm/i915/pxp: Trigger the global teardown for before suspending
On Tue, 2023-01-24 at 10:17 -0500, Rodrigo Vivi wrote:
> On Mon, Jan 23, 2023 at 09:31:49PM -0800, Alan Previn wrote:
> > A driver bug was recently discovered where the security firmware
> > was
> > receiving internal HW signals indicating that session key
> > expirations
> > had occurred. Architecturally, the firmware was expecting a
> > response
> > from the GuC to acknowledge the event with the firmware side.
> > However the OS was in a suspended state and GuC had been reset.
> >
> > Internal specifications actually required the driver to ensure
> > that all active sessions be properly cleaned up in such cases where
> > the system is suspended and the GuC potentially unable to respond.
> >
> > This patch adds the global teardown code in i915's suspend_prepare
> > code path.
> >
> > Signed-off-by: Alan Previn
> > Reviewed-by: Juston Li
>
> it should probably contain some revision history above so the
> reviewer can know what changed from the time he reviewed and now.
>
> it looks we have another function that I don't remember...
> Juston, could you please confirm your review stands in this version?
> if so feel free to add my ack with that.
Sure, v6 is:
Reviewed-by: Juston Li
Appreciate the reviews/revisions Rodrigo and Alan!
>
> > ---
> > drivers/gpu/drm/i915/pxp/intel_pxp.c | 65
> > +---
> > drivers/gpu/drm/i915/pxp/intel_pxp.h | 1 +
> > drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 2 +-
> > drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 6 +-
> > drivers/gpu/drm/i915/pxp/intel_pxp_session.h | 5 ++
> > 5 files changed, 66 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > index cfc9af8b3d21..9d4c7724e98e 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > @@ -270,6 +270,60 @@ static bool pxp_component_bound(struct
> > intel_pxp *pxp)
> > return bound;
> > }
> >
> > +static int __pxp_global_teardown_final(struct intel_pxp *pxp)
> > +{
> > + if (!pxp->arb_is_valid)
> > + return 0;
> > + /*
> > + * To ensure synchronous and coherent session teardown
> > completion
> > + * in response to suspend or shutdown triggers, don't use a
> > worker.
> > + */
> > + intel_pxp_mark_termination_in_progress(pxp);
> > + intel_pxp_terminate(pxp, false);
> > +
> > + if (!wait_for_completion_timeout(>termination,
> > msecs_to_jiffies(250)))
> > + return -ETIMEDOUT;
> > +
> > + return 0;
> > +}
> > +
> > +static int __pxp_global_teardown_restart(struct intel_pxp *pxp)
> > +{
> > + if (pxp->arb_is_valid)
> > + return 0;
> > + /*
> > + * The arb-session is currently inactive and we are doing a
> > reset and restart
> > + * due to a runtime event. Use the worker that was designed
> > for this.
> > + */
> > + pxp_queue_termination(pxp);
> > +
> > + if (!wait_for_completion_timeout(>termination,
> > msecs_to_jiffies(250)))
> > + return -ETIMEDOUT;
> > +
> > + return 0;
> > +}
> > +
> > +void intel_pxp_end(struct intel_pxp *pxp)
> > +{
> > + struct drm_i915_private *i915 = pxp->ctrl_gt->i915;
> > + intel_wakeref_t wakeref;
> > +
> > + if (!intel_pxp_is_enabled(pxp))
> > + return;
> > +
> > + wakeref = intel_runtime_pm_get(>runtime_pm);
> > +
> > + mutex_lock(>arb_mutex);
> > +
> > + if (__pxp_global_teardown_final(pxp))
> > + drm_dbg(>drm, "PXP end timed out\n");
> > +
> > + mutex_unlock(>arb_mutex);
> > +
> > + intel_pxp_fini_hw(pxp);
> > + intel_runtime_pm_put(>runtime_pm, wakeref);
> > +}
> > +
> > /*
> > * the arb session is restarted from the irq work when we receive
> > the
> > * termination completion interrupt
> > @@ -286,16 +340,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
> >
> > mutex_lock(>arb_mutex);
> >
> > - if (pxp->arb_is_valid)
> > - goto unlock;
> > -
> > - pxp_queue_termination(pxp);
> > -
> > - if (!wait_for_completion_timeout(>termination,
> > - msecs_to_jiffies(250))) {
> > - ret = -ETIMEDOUT;
> > + ret = __pxp_global_teardown_restart(pxp);
> > + if (ret)
> > goto unlock;
> > - }
> >
> > /* make sure the compiler doesn't optimize the double
> > access */
> > barrier();
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> > b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> > index 9658d3005222..3ded0890cd27 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> > @@ -27,6 +27,7 @@ void
> > intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp);
> > void intel_pxp_tee_end_arb_fw_session(struct intel_pxp *pxp, u32
> > arb_session_id);
> >
> > int
Re: [PATCH v1 12/14] drm/msm/disp/dpu1: revise timing engine programming to work for DSC
On 1/24/2023 1:11 AM, Dmitry Baryshkov wrote:
On 23/01/2023 20:24, Kuogee Hsieh wrote:
Current implementation timing engine programming does not consider
compression factors. This patch add consideration of DSC factors
while programming timing engine.
Signed-off-by: Kuogee Hsieh
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 14 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 132
+
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +-
5 files changed, 110 insertions(+), 54 deletions(-)
[skipped]
@@ -113,82 +124,96 @@ static void
dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
/* read interface_cfg */
intf_cfg = DPU_REG_READ(c, INTF_CONFIG);
- if (ctx->cap->type == INTF_DP)
+ if (ctx->cap->type == INTF_EDP || ctx->cap->type == INTF_DP)
dp_intf = true;
hsync_period = p->hsync_pulse_width + p->h_back_porch +
p->width +
- p->h_front_porch;
+ p->h_front_porch;
vsync_period = p->vsync_pulse_width + p->v_back_porch +
p->height +
- p->v_front_porch;
+ p->v_front_porch;
Actually I went on through the history and found the previous
submission, https://patchwork.freedesktop.org/patch/471505/.
Exactly the same piece of code. Did you expect that the comments will
be different this time?
I really hoped that at that time we already went through this. But it
seems I was wrong. That series went through v10 or v12 before being
accepted. And it was just adding wide_bus_en. Back at that time we
lightly discussed that the code will receive compression support. But
I never expected to see the original submission again.
It might sound bad, but could you please find somebody who can do
internal review for you? Good internal review.
That said, I really do not expect to see v2 before the whole series is
reworked, restructured and prepared for the review on your side.
This timing engine code is derived from our downstream code directly and
it has been used at many mobile devices by many vendors for many years
already.
On the other words, it had been tested very thorough and works on
dsi/dp/hdmi/dsc/widebus applications.
When i brought dsc v1.2 over, I just merged it over and did not consider
too much.
Can we adapt this code so that both upstream and down stream shared same
timing engine programming so that easier to maintain?
display_v_start = ((p->vsync_pulse_width + p->v_back_porch) *
- hsync_period) + p->hsync_skew;
+ hsync_period) + p->hsync_skew;
display_v_end = ((vsync_period - p->v_front_porch) *
hsync_period) +
- p->hsync_skew - 1;
+ p->hsync_skew - 1;
+
+ hsync_ctl = (hsync_period << 16) | p->hsync_pulse_width;
hsync_start_x = p->h_back_porch + p->hsync_pulse_width;
hsync_end_x = hsync_period - p->h_front_porch - 1;
- if (p->width != p->xres) { /* border fill added */
- active_h_start = hsync_start_x;
- active_h_end = active_h_start + p->xres - 1;
- } else {
- active_h_start = 0;
- active_h_end = 0;
- }
-
- if (p->height != p->yres) { /* border fill added */
- active_v_start = display_v_start;
- active_v_end = active_v_start + (p->yres * hsync_period) - 1;
- } else {
- active_v_start = 0;
- active_v_end = 0;
- }
-
- if (active_h_end) {
- active_hctl = (active_h_end << 16) | active_h_start;
- intf_cfg |= INTF_CFG_ACTIVE_H_EN;
- } else {
- active_hctl = 0;
- }
-
- if (active_v_end)
- intf_cfg |= INTF_CFG_ACTIVE_V_EN;
-
- hsync_ctl = (hsync_period << 16) | p->hsync_pulse_width;
- display_hctl = (hsync_end_x << 16) | hsync_start_x;
-
/*
* DATA_HCTL_EN controls data timing which can be different from
* video timing. It is recommended to enable it for all cases,
except
* if compression is enabled in 1 pixel per clock mode
*/
+ if (!p->compression_en || p->wide_bus_en)
+ intf_cfg2 |= INTF_CFG2_DATA_HCTL_EN;
+
if (p->wide_bus_en)
- intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN | INTF_CFG2_DATA_HCTL_EN;
+ intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN;
+ /*
+ * If widebus is disabled:
+ * For uncompressed stream, the data is valid for the entire active
+ * window period.
+ * For compressed stream, data is valid for a shorter time period
+ * inside the active window depending on the compression ratio.
+ *
+ * If widebus is enabled:
+ * For uncompressed stream, data is valid for only half the active
+ * window, since the data rate is doubled in this mode.
+ * p->width holds the adjusted width for DP but unadjusted width
for DSI
+ * For compressed stream, data validity window needs to be
adjusted for
+ * compression ratio and then further halved.
Re: Amdgpu module is references even after unbinding the vtcon
Hi Am 24.01.23 um 17:36 schrieb Alex Deucher: On Tue, Jan 24, 2023 at 11:15 AM Thomas Zimmermann wrote: Hi Am 24.01.23 um 15:12 schrieb Christian König: Hi Thomas, we ran into a problem with the general fbcon/fbdev implementation and though that you might have some idea. What happens is the following: 1. We load amdgpu and get our normal fbcon. 2. fbcon allocates a dump BO as backing store for the console. 3. GDM/X/Applications start, new framebuffers are created BOs imported, exported etc... 4. Somehow X or GDM iterated over all the framebuffer objects the kernels knows about and export them as DMA-buf. 5. Application/X/GDM are stopped, handles closed, framebuffers released etc... 6. We unbind vtcon. At this point the amdgpu module usually has a reference count of 0 and can be unloaded, but since GDM/X/Whoever iterated over all the known framebuffers and exported them as DMA-buf (for whatever reason idk) we now still have an exported DMA-buf and with it a reference to the module. Any idea how we could prevent that? No real clue, sorry. But does it change if you use a shadow buffer on top of the amdgpu BO? Set prefer_shadow_fbdev = 1. [1] I once tried to run generic fbdev without prefer_shadow_fbdev and it never worked. I suspected that some reference counting got wrong, but could never pin it down. Maybe your issue is similar. Is that equivalent to setting mode_config.prefer_shadow = 1? If so, we already do that. Yes it is, unfortunately. Best regards Thomas Alex That said, generic fbdev is not so super-optimal for TTM-based drivers. I'm working on improving that, but it's not there yet. Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.0/source/include/drm/drm_mode_config.h#L890 Thanks, Christian. -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH drm-next 03/14] drm: manager to keep track of GPUs VA mappings
On Tue, Jan 24, 2023 at 01:11:55AM +0100, Danilo Krummrich wrote: On 1/24/23 00:23, Niranjana Vishwanathapura wrote: On Wed, Jan 18, 2023 at 07:12:45AM +0100, Danilo Krummrich wrote: This adds the infrastructure for a manager implementation to keep track of GPU virtual address (VA) mappings. New UAPIs, motivated by Vulkan sparse memory bindings graphics drivers start implementing, allow userspace applications to request multiple and arbitrary GPU VA mappings of buffer objects. The DRM GPU VA manager is intended to serve the following purposes in this context. 1) Provide a dedicated range allocator to track GPU VA allocations and mappings, making use of the drm_mm range allocator. 2) Generically connect GPU VA mappings to their backing buffers, in particular DRM GEM objects. 3) Provide a common implementation to perform more complex mapping operations on the GPU VA space. In particular splitting and merging of GPU VA mappings, e.g. for intersecting mapping requests or partial unmap requests. Idea-suggested-by: Dave Airlie Signed-off-by: Danilo Krummrich --- Documentation/gpu/drm-mm.rst | 31 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_gem.c | 3 + drivers/gpu/drm/drm_gpuva_mgr.c | 1323 +++ include/drm/drm_drv.h | 6 + include/drm/drm_gem.h | 75 ++ include/drm/drm_gpuva_mgr.h | 527 7 files changed, 1966 insertions(+) create mode 100644 drivers/gpu/drm/drm_gpuva_mgr.c create mode 100644 include/drm/drm_gpuva_mgr.h diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index a52e6f4117d6..c9f120cfe730 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -466,6 +466,37 @@ DRM MM Range Allocator Function References .. kernel-doc:: drivers/gpu/drm/drm_mm.c :export: +DRM GPU VA Manager +== + +Overview + + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Overview + +Split and Merge +--- + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Split and Merge + +Locking +--- + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :doc: Locking + + +DRM GPU VA Manager Function References +-- + +.. kernel-doc:: include/drm/drm_gpuva_mgr.h + :internal: + +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c + :export: + DRM Buddy Allocator === diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 4fe190aee584..de2ffca3b6e4 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -45,6 +45,7 @@ drm-y := \ drm_vblank.o \ drm_vblank_work.o \ drm_vma_manager.o \ + drm_gpuva_mgr.o \ drm_writeback.o drm-$(CONFIG_DRM_LEGACY) += \ drm_agpsupport.o \ diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 59a0bb5ebd85..65115fe88627 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -164,6 +164,9 @@ void drm_gem_private_object_init(struct drm_device *dev, if (!obj->resv) obj->resv = >_resv; + if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA)) + drm_gem_gpuva_init(obj); + drm_vma_node_reset(>vma_node); INIT_LIST_HEAD(>lru_node); } diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c new file mode 100644 index ..e665f642689d --- /dev/null +++ b/drivers/gpu/drm/drm_gpuva_mgr.c @@ -0,0 +1,1323 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2022 Red Hat. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: + * Danilo Krummrich + * + */ + +#include +#include + +/** + * DOC: Overview + * + * The DRM GPU VA Manager, represented by struct drm_gpuva_manager keeps track + * of a GPU's virtual address (VA) space and manages the corresponding virtual + * mappings represented by _gpuva objects. It also keeps track of the + *
Re: [Intel-gfx] [PATCH 1/5] drm/i915: audit bo->resource usage v3
On Tue, 24 Jan 2023 at 13:48, Matthew Auld wrote: > > On Tue, 24 Jan 2023 at 12:57, Christian König > wrote: > > > > From: Christian König > > > > Make sure we can at least move and alloc TT objects without backing store. > > > > v2: clear the tt object even when no resource is allocated. > > v3: add Matthews changes for i915 as well. > > > > Signed-off-by: Christian König > Reviewed-by: Matthew Auld Ofc that assumes intel-gfx CI is now happy with the series.
Re: [Intel-gfx] [PATCH 5/5] drm/ttm: replace busy placement with flags v2
On Tue, 24 Jan 2023 at 12:57, Christian König
wrote:
>
> Instead of a list of separate busy placement add flags which indicate
> that a placement should only be used when there is room or if we need to
> evict.
>
> v2: add missing TTM_PL_FLAG_IDLE for i915
>
> Signed-off-by: Christian König
> ---
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index c2ec91cc845d..0ab24ca5f419 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -347,27 +347,6 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int
> align,
> return 0;
> }
>
> -static void
> -set_placement_list(struct ttm_place *pl, unsigned *n, uint32_t domain)
> -{
> - *n = 0;
> -
> - if (domain & NOUVEAU_GEM_DOMAIN_VRAM) {
> - pl[*n].mem_type = TTM_PL_VRAM;
> - pl[*n].flags = 0;
> - (*n)++;
> - }
> - if (domain & NOUVEAU_GEM_DOMAIN_GART) {
> - pl[*n].mem_type = TTM_PL_TT;
> - pl[*n].flags = 0;
> - (*n)++;
> - }
> - if (domain & NOUVEAU_GEM_DOMAIN_CPU) {
> - pl[*n].mem_type = TTM_PL_SYSTEM;
> - pl[(*n)++].flags = 0;
> - }
> -}
> -
> static void
> set_placement_range(struct nouveau_bo *nvbo, uint32_t domain)
> {
> @@ -395,10 +374,6 @@ set_placement_range(struct nouveau_bo *nvbo, uint32_t
> domain)
> nvbo->placements[i].fpfn = fpfn;
> nvbo->placements[i].lpfn = lpfn;
> }
> - for (i = 0; i < nvbo->placement.num_busy_placement; ++i) {
> - nvbo->busy_placements[i].fpfn = fpfn;
> - nvbo->busy_placements[i].lpfn = lpfn;
> - }
> }
> }
>
> @@ -406,15 +381,32 @@ void
> nouveau_bo_placement_set(struct nouveau_bo *nvbo, uint32_t domain,
> uint32_t busy)
> {
> - struct ttm_placement *pl = >placement;
> + struct ttm_place *pl = nvbo->placements;
> + unsigned *n = >placement.num_placement;
>
> - pl->placement = nvbo->placements;
> - set_placement_list(nvbo->placements, >num_placement, domain);
> + domain |= busy;
>
> - pl->busy_placement = nvbo->busy_placements;
> - set_placement_list(nvbo->busy_placements, >num_busy_placement,
> - domain | busy);
> + *n = 0;
> + if (domain & NOUVEAU_GEM_DOMAIN_VRAM) {
> + pl[*n].mem_type = TTM_PL_VRAM;
> + pl[*n].flags = busy & NOUVEAU_GEM_DOMAIN_VRAM ?
> + TTM_PL_FLAG_BUSY : 0;
> + (*n)++;
> + }
> + if (domain & NOUVEAU_GEM_DOMAIN_GART) {
> + pl[*n].mem_type = TTM_PL_TT;
> + pl[*n].flags = busy & NOUVEAU_GEM_DOMAIN_GART ?
> + TTM_PL_FLAG_BUSY : 0;
> + (*n)++;
> + }
> + if (domain & NOUVEAU_GEM_DOMAIN_CPU) {
> + pl[*n].mem_type = TTM_PL_SYSTEM;
> + pl[*n].flags = busy & NOUVEAU_GEM_DOMAIN_CPU ?
> + TTM_PL_FLAG_BUSY : 0;
> + (*n)++;
> + }
Should this not be something like:
non_busy = domain;
domain |= busy;
if (domain & VRAM) {
if (non_busy & VRAM)
flags = 0
else
flags = FLAG_BUSY
}
Otherwise if VRAM is set in both "busy" and "domain", it will only try
VRAM when all non-busy first fails, which looks like a change in
behaviour?
The rest of the patch looks good to me, so with the above fixed or explained,
Reviewed-by: Matthew Auld
[PATCH v2 4/8] drm/i915/guc: Update GuC messages in intel_guc_ct.c
Use new macros to have common prefix that also include GT#.
v2: drop unused helpers
Signed-off-by: Michal Wajdeczko
Cc: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 23 ---
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 2b22065e87bf..1803a633ed64 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -11,38 +11,23 @@
#include "i915_drv.h"
#include "intel_guc_ct.h"
-#include "gt/intel_gt.h"
+#include "intel_guc_print.h"
static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
{
return container_of(ct, struct intel_guc, ct);
}
-static inline struct intel_gt *ct_to_gt(struct intel_guc_ct *ct)
-{
- return guc_to_gt(ct_to_guc(ct));
-}
-
-static inline struct drm_i915_private *ct_to_i915(struct intel_guc_ct *ct)
-{
- return ct_to_gt(ct)->i915;
-}
-
-static inline struct drm_device *ct_to_drm(struct intel_guc_ct *ct)
-{
- return _to_i915(ct)->drm;
-}
-
#define CT_ERROR(_ct, _fmt, ...) \
- drm_err(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__)
+ guc_err(ct_to_guc(_ct), "CT: " _fmt, ##__VA_ARGS__)
#ifdef CONFIG_DRM_I915_DEBUG_GUC
#define CT_DEBUG(_ct, _fmt, ...) \
- drm_dbg(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__)
+ guc_dbg(ct_to_guc(_ct), "CT: " _fmt, ##__VA_ARGS__)
#else
#define CT_DEBUG(...) do { } while (0)
#endif
#define CT_PROBE_ERROR(_ct, _fmt, ...) \
- i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
+ guc_probe_error(ct_to_guc(ct), "CT: " _fmt, ##__VA_ARGS__)
/**
* DOC: CTB Blob
--
2.25.1
Re: [PATCH V12 1/4] drm: of: Add drm_of_get_dsi_bus helper function
On Mon, Jan 23, 2023 at 09:46:00AM -0600, Chris Morgan wrote: > From: Chris Morgan > > Add helper function to find DSI host for devices where DSI panel is not > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the > official Raspberry Pi touchscreen display). > > Co-developed-by: Maya Matuszczyk > Signed-off-by: Maya Matuszczyk > Signed-off-by: Chris Morgan Reviewed-by: Maxime Ripard Maxime
[PATCH v2 5/8] drm/i915/guc: Update GuC messages in intel_guc_fw.c
Use new macros to have common prefix that also include GT#.
Signed-off-by: Michal Wajdeczko
Cc: John Harrison
Reviewed-by: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 5b86b2e286e0..3d2249bda368 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -13,6 +13,7 @@
#include "gt/intel_gt_mcr.h"
#include "gt/intel_gt_regs.h"
#include "intel_guc_fw.h"
+#include "intel_guc_print.h"
#include "i915_drv.h"
static void guc_prepare_xfer(struct intel_gt *gt)
@@ -103,8 +104,10 @@ static inline bool guc_ready(struct intel_uncore *uncore,
u32 *status)
return uk_val == INTEL_GUC_LOAD_STATUS_READY;
}
-static int guc_wait_ucode(struct intel_uncore *uncore)
+static int guc_wait_ucode(struct intel_guc *guc)
{
+ struct intel_gt *gt = guc_to_gt(guc);
+ struct intel_uncore *uncore = gt->uncore;
u32 status;
int ret;
@@ -127,10 +130,8 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
*/
ret = wait_for(guc_ready(uncore, ), 200);
if (ret) {
- struct drm_device *drm = >i915->drm;
-
- drm_info(drm, "GuC load failed: status = 0x%08X\n", status);
- drm_info(drm, "GuC load failed: status: Reset = %d, "
+ guc_info(guc, "load failed: status = 0x%08X\n", status);
+ guc_info(guc, "load failed: status: Reset = %d, "
"BootROM = 0x%02X, UKernel = 0x%02X, "
"MIA = 0x%02X, Auth = 0x%02X\n",
REG_FIELD_GET(GS_MIA_IN_RESET, status),
@@ -140,12 +141,12 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
REG_FIELD_GET(GS_AUTH_STATUS_MASK, status));
if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
- drm_info(drm, "GuC firmware signature verification
failed\n");
+ guc_info(guc, "firmware signature verification
failed\n");
ret = -ENOEXEC;
}
if (REG_FIELD_GET(GS_UKERNEL_MASK, status) ==
INTEL_GUC_LOAD_STATUS_EXCEPTION) {
- drm_info(drm, "GuC firmware exception. EIP: %#x\n",
+ guc_info(guc, "firmware exception. EIP: %#x\n",
intel_uncore_read(uncore, SOFT_SCRATCH(13)));
ret = -ENXIO;
}
@@ -194,7 +195,7 @@ int intel_guc_fw_upload(struct intel_guc *guc)
if (ret)
goto out;
- ret = guc_wait_ucode(uncore);
+ ret = guc_wait_ucode(guc);
if (ret)
goto out;
--
2.25.1
