Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On 2022/06/02 16:38, Arnd Bergmann wrote:
I think that SmPL script worked great, almost every instance is
something that ought to be changed, as long as it stops reporting
those structures that are also __aligned(). I would extend it to
also report structures with 'bool', 'enum', or any pointer, but that
could give more false-positives. Maybe have a separate script
for those instances embedding atomics or spinlocks (very broken)
vs the other members (causes more harm than good or might
need alignment).
I extended my script to detect __packed struct or union without
__aligned. It is split in two scripts.
The first one is to search for problematic cases where __packed
structs/unions have atomic types or spinlock types. In this version,
types whose names contain "atomic" or "spinlock" are targeted.
== Scripts ==
@r@
type T;
identifier i;
type b =~ ".*(atomic|spinlock).*";
position p;
attribute name __packed, __aligned;
attribute at;
@@
T@p {
...
b i;
...
} at;
@script:python@
p
[PATCH] drm/msm: Fix double pm_runtime_disable() call
Following commit 17e822f7591f ("drm/msm: fix unbalanced
pm_runtime_enable in adreno_gpu_{init, cleanup}"), any call to
adreno_unbind() will disable runtime PM twice, as indicated by the call
trees below:
adreno_unbind()
-> pm_runtime_force_suspend()
-> pm_runtime_disable()
adreno_unbind()
-> gpu->funcs->destroy() [= aNxx_destroy()]
-> adreno_gpu_cleanup()
-> pm_runtime_disable()
Note that pm_runtime_force_suspend() is called right before
gpu->funcs->destroy() and both functions are called unconditionally.
With recent addition of the eDP AUX bus code, this problem manifests
itself when the eDP panel cannot be found yet and probing is deferred.
On the first probe attempt, we disable runtime PM twice as described
above. This then causes any later probe attempt to fail with
[drm:adreno_load_gpu [msm]] *ERROR* Couldn't power up the GPU: -13
preventing the driver from loading.
As there seem to be scenarios where the aNxx_destroy() functions are not
called from adreno_unbind(), simply removing pm_runtime_disable() from
inside adreno_unbind() does not seem to be the proper fix. This is what
commit 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
adreno_gpu_{init, cleanup}") intended to fix. Therefore, instead check
whether runtime PM is still enabled, and only disable it in that case.
Fixes: 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
adreno_gpu_{init, cleanup}")
Signed-off-by: Maximilian Luz
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 4e665c806a14..f944b69e2a25 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -1057,7 +1057,8 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++)
release_firmware(adreno_gpu->fw[i]);
- pm_runtime_disable(>gpu_pdev->dev);
+ if (pm_runtime_enabled(>gpu_pdev->dev))
+ pm_runtime_disable(>gpu_pdev->dev);
msm_gpu_cleanup(_gpu->base);
}
--
2.36.1
Re: [PATCH v4 5/8] drm/panel: panel-simple: Implement .get_orientation callback
On Tue, Jun 7, 2022 at 3:25 AM Sam Ravnborg wrote:
>
> Hi Hsin-Yi,
>
> On Mon, Jun 06, 2022 at 11:24:28PM +0800, Hsin-Yi Wang wrote:
> > To return the orientation property to drm/kms driver.
> >
> > Signed-off-by: Hsin-Yi Wang
> > Reviewed-by: Hans de Goede
> > Reviewed-by: Douglas Anderson
> > ---
> > drivers/gpu/drm/panel/panel-simple.c | 16 +++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > b/drivers/gpu/drm/panel/panel-simple.c
> > index 4a2e580a2f7b..f232b8cf4075 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -411,7 +411,12 @@ static int panel_simple_get_modes(struct drm_panel
> > *panel,
> > /* add hard-coded panel modes */
> > num += panel_simple_get_non_edid_modes(p, connector);
> >
> > - /* set up connector's "panel orientation" property */
> > + /*
> > + * drm drivers are expected to call drm_panel_get_orientation() to get
> > + * panel's orientation then drm_connector_set_panel_orientation() to
> > + * set the property before drm_dev_register(). Otherwise there will be
> > + * a WARN_ON if orientation is set after drm is registered.
> > + */
>
> This comment is not really relevant here. If we need to explain this
> then put it in drm_panel.c/h - as this applies for all panels and not
> just the panel_simple.
> Keep in mind, this is the source new panels often use a inspiration and
> no need to have this copied around.
>
Will update this.
> > drm_connector_set_panel_orientation(connector, p->orientation);
> >
> > return num;
> > @@ -434,6 +439,14 @@ static int panel_simple_get_timings(struct drm_panel
> > *panel,
> > return p->desc->num_timings;
> > }
> >
> > +static enum drm_panel_orientation panel_simple_get_orientation(struct
> > drm_panel *panel)
> > +{
> > + struct panel_simple *p = to_panel_simple(panel);
> > +
> > + return p->orientation;
> > +}
> > +
> > +
> > static const struct drm_panel_funcs panel_simple_funcs = {
> > .disable = panel_simple_disable,
> > .unprepare = panel_simple_unprepare,
> > @@ -441,6 +454,7 @@ static const struct drm_panel_funcs panel_simple_funcs
> > = {
> > .enable = panel_simple_enable,
> > .get_modes = panel_simple_get_modes,
> > .get_timings = panel_simple_get_timings,
> > + .get_orientation = panel_simple_get_orientation,
>
> I like the order in this list to match the order in the .h file.
> So my OCD would like you to move it up right after get_modes,
> but feel free to ignore this.
>
Sure.
> With the suggested changes:
> Reviewed-by: Sam Ravnborg
>
> > };
> >
> > static struct panel_desc panel_dpi;
> > --
> > 2.36.1.255.ge46751e96f-goog
Re: [PATCH v4 8/8] drm/mediatek: Config orientation property if panel provides it
On Tue, Jun 7, 2022 at 3:31 AM Sam Ravnborg wrote:
>
> Hi Hsin-Yi,
>
> On Mon, Jun 06, 2022 at 11:24:31PM +0800, Hsin-Yi Wang wrote:
> > Panel orientation property should be set before drm_dev_register().
> > Mediatek drm driver calls drm_dev_register() in .bind(). However, most
> > panels sets orientation property relatively late, mostly in .get_modes()
> > callback, since this is when they are able to get the connector and
> > binds the orientation property to it, though the value should be known
> > when the panel is probed.
> >
> > Let the drm driver check if the remote end point is a panel and if it
> > contains the orientation property. If it does, set it before
> > drm_dev_register() is called.
>
> I do not know the best way to do what you need to do here.
> But it seems wrong to introduce a deprecated function
> (drm_of_find_panel_or_bridge), to set the rotation property early.
>
How about of_drm_find_panel()?
> I think you need to find a way to do this utilising the panel_bridge or
> something.
>
> Sam
>
> >
> > Signed-off-by: Hsin-Yi Wang
> > Reviewed-by: Hans de Goede
> > Reviewed-by: AngeloGioacchino Del Regno
> >
> > ---
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index d9f10a33e6fa..c56282412bfa 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -185,6 +185,7 @@ struct mtk_dsi {
> > struct drm_encoder encoder;
> > struct drm_bridge bridge;
> > struct drm_bridge *next_bridge;
> > + struct drm_panel *panel;
> > struct drm_connector *connector;
> > struct phy *phy;
> >
> > @@ -822,6 +823,12 @@ static int mtk_dsi_encoder_init(struct drm_device
> > *drm, struct mtk_dsi *dsi)
> > ret = PTR_ERR(dsi->connector);
> > goto err_cleanup_encoder;
> > }
> > +
> > + /* Read panel orientation */
> > + if (dsi->panel)
> > + drm_connector_set_panel_orientation(dsi->connector,
> > + drm_panel_get_orientation(dsi->panel));
> > +
> > drm_connector_attach_encoder(dsi->connector, >encoder);
> >
> > return 0;
> > @@ -837,6 +844,9 @@ static int mtk_dsi_bind(struct device *dev, struct
> > device *master, void *data)
> > struct drm_device *drm = data;
> > struct mtk_dsi *dsi = dev_get_drvdata(dev);
> >
> > + /* Get panel if existed */
> > + drm_of_find_panel_or_bridge(dev->of_node, 0, 0, >panel, NULL);
> > +
> > ret = mtk_dsi_encoder_init(drm, dsi);
> > if (ret)
> > return ret;
> > --
> > 2.36.1.255.ge46751e96f-goog
Re: [PATCH v4 8/8] drm/mediatek: Config orientation property if panel provides it
On Tue, Jun 7, 2022 at 3:16 AM Stephen Boyd wrote:
>
> Quoting Hsin-Yi Wang (2022-06-06 08:24:31)
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index d9f10a33e6fa..c56282412bfa 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -822,6 +823,12 @@ static int mtk_dsi_encoder_init(struct drm_device
> > *drm, struct mtk_dsi *dsi)
> > ret = PTR_ERR(dsi->connector);
> > goto err_cleanup_encoder;
> > }
> > +
> > + /* Read panel orientation */
> > + if (dsi->panel)
> > + drm_connector_set_panel_orientation(dsi->connector,
> > + drm_panel_get_orientation(dsi->panel));
> > +
>
> It could be simplified like so?
>
> drm_connector_set_orientation_from_panel(dsi->connector, dsi->panel);
>
> Then the API could get the orientation if the panel pointer is valid.
> Does any code need to use/modify the orientation value besides
> drm_connector_set_panel_orientation()?
>
We can add another function to call
drm_connector_set_orientation_from_panel(), which will be like
void drm_connector_set_orientation_from_panel(connector, panel)
{
if (panel)
drm_connector_set_panel_orientation(connector,drm_panel_get_orientation(panel));
}
Though it's very should but I can add this if this can make the caller
more convenient.
> > drm_connector_attach_encoder(dsi->connector, >encoder);
> >
> > return 0;
RE: [PATCH v1 2/2] drm/aspeed: Add 1024x768 mode for AST2600
Hi Joel,
We have released related datasheet and update these registers
description.
SCU300: Clock Selection Register
...
10:8 Soc display clock selection
000b: D-PLL
001b: Reserved
010b: 40MHz from USB 2.0 port1 PHY
011b: GPIOC6
100b: Reserved
101b: E-PLL divided by SCU308[17:12]
110b: M-PLL divided by SCU308[17:12]
111b: H-PLL divided by SCU308[17:12]
SCU308: Clock Selection Register Set 3
17:12 Soc display clock divider selection
00b: div 1
011011b: div 16
Thanks,
By Tommy
> -Original Message-
> From: Tommy Huang
> Sent: Tuesday, April 26, 2022 4:28 PM
> To: Joel Stanley
> Cc: David Airlie ; Daniel Vetter ; Rob
> Herring ; Andrew Jeffery ;
> linux-aspeed ; open list:DRM DRIVERS
> ; devicetree ;
> Linux ARM ; Linux Kernel Mailing List
> ; BMC-SW
> Subject: RE: [PATCH v1 2/2] drm/aspeed: Add 1024x768 mode for AST2600
>
>
>
> > -Original Message-
> > From: Joel Stanley
> > Sent: Tuesday, April 26, 2022 3:48 PM
> > To: Tommy Huang
> > Cc: David Airlie ; Daniel Vetter ;
> > Rob Herring ; Andrew Jeffery ;
> > linux-aspeed ; open list:DRM DRIVERS
> > ; devicetree
> > ; Linux ARM
> > ; Linux Kernel Mailing List
> > ; BMC-SW
> > Subject: Re: [PATCH v1 2/2] drm/aspeed: Add 1024x768 mode for AST2600
> >
> > On Fri, 4 Mar 2022 at 06:32, Tommy Haung
>
> > wrote:
> > >
> > > Update the aspeed_gfx_set_clk with display width.
> > > At AST2600, the display clock could be coming from HPLL clock / 16 =
> > > 75MHz. It would fit 1024x768@70Hz.
> > > Another chip will still keep 800x600.
> > >
> > > Signed-off-by: Tommy Haung
> > > ---
> > > drivers/gpu/drm/aspeed/aspeed_gfx.h | 12 ++
> > > drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 29
> > > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> |
> > > 16 +++-- drivers/gpu/drm/aspeed/aspeed_gfx_out.c | 14
> > > +++-
> > > 4 files changed, 60 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > > b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > > index eb4c267cde5e..c7aefee0657a 100644
> > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > > @@ -109,11 +109,15 @@ int aspeed_gfx_create_output(struct drm_device
> > *drm);
> > > #define CRT_THROD_HIGH(x) ((x) << 8)
> > >
> > > /* SCU control */
> > > -#define SCU_G6_CLK_COURCE 0x300
> > > +#define G6_CLK_SOURCE 0x300
> > > +#define G6_CLK_SOURCE_MASK (BIT(8) | BIT(9) | BIT(10))
> > > +#define G6_CLK_SOURCE_HPLL (BIT(8) | BIT(9) | BIT(10))
> > > +#define G6_CLK_SOURCE_USB BIT(9)
> > > +#define G6_CLK_SEL30x308
> > > +#define G6_CLK_DIV_MASK0x3F000
> >
> > This register is defined in the data sheet as:
> >
> > 17:12 SOC Display clock selection when source is from DisplayPort PHY
> >
> > That doesn't match with what the code is doing. Can you clarify the
> > register definition?
>
> OK. I will clarify it and response it to you.
>
> >
> > > +#define G6_CLK_DIV_16
> > (BIT(16)|BIT(15)|BIT(13)|BIT(12))
> > > +#define G6_USB_40_CLK BIT(9)
> > >
> > > /* GFX FLAGS */
> > > #define CLK_MASK BIT(0)
> > > #define CLK_G6 BIT(0)
> > > -
> > > -#define G6_CLK_MASK(BIT(8) | BIT(9) | BIT(10))
> > > -#define G6_USB_40_CLK BIT(9)
> > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> > > b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> > > index a24fab22eac4..5829be9c7c67 100644
> > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> > > @@ -23,6 +23,28 @@ drm_pipe_to_aspeed_gfx(struct
> > drm_simple_display_pipe *pipe)
> > > return container_of(pipe, struct aspeed_gfx, pipe); }
> > >
> > > +static void aspeed_gfx_set_clock_source(struct aspeed_gfx *priv,
> > > +int
> > > +mode_width) {
> > > + regmap_update_bits(priv->scu, G6_CLK_SOURCE,
> > G6_CLK_SOURCE_MASK, 0x0);
> > > + regmap_update_bits(priv->scu, G6_CLK_SEL3,
> G6_CLK_DIV_MASK,
> > > +0x0);
> > > +
> > > + switch (mode_width) {
> > > + case 1024:
> > > + /* hpll div 16 = 75Mhz */
> > > + regmap_update_bits(priv->scu, G6_CLK_SOURCE,
> > > + G6_CLK_SOURCE_MASK, G6_CLK_SOURCE_HPLL);
> > > + regmap_update_bits(priv->scu, G6_CLK_SEL3,
> > > + G6_CLK_DIV_MASK, G6_CLK_DIV_16);
> > > + break;
> > > + case 800:
> > > + default:
> > > + /* usb 40Mhz */
> > > + regmap_update_bits(priv->scu, G6_CLK_SOURCE,
> > > + G6_CLK_SOURCE_MASK, G6_CLK_SOURCE_USB);
> > > + break;
> > > + }
> > > +}
> > > +
> > > static int
Re: [PATCH v2 0/2] drm/bridge: analogix_dp: Self-refresh state machine fixes
On Mon, Jun 6, 2022 at 1:30 PM Doug Anderson wrote: > On Fri, Jun 3, 2022 at 8:17 AM Doug Anderson wrote: > > On Fri, Jun 3, 2022 at 8:11 AM Sean Paul wrote: > > > Apologies for the delay. Please in future ping on irc/chat if you're > > > waiting for review from me, my inbox is often neglected. OK, I'll try to keep that in mind. I can't help myself with the semi-relevant XKCD though ;) https://xkcd.com/1254/ > > > The set still looks good to me, > > > > > > Reviewed-by: Sean Paul Thanks! > > Unless someone yells, I'll plan to apply both patches to > > drm-misc-fixes early next week, possibly Monday. Seems like if someone > > was going to object to these they've had plenty of time up until now. > > As promised, I pushed these to drm-misc-fixes: > > e54a4424925a drm/atomic: Force bridge self-refresh-exit on CRTC switch > ca871659ec16 drm/bridge: analogix_dp: Support PSR-exit to disable transition And thanks, Doug. Brian
[PATCH 2/2] vfio: Replace the iommu notifier with a device list
Instead of bouncing the function call to the driver op through a blocking
notifier just have the iommu layer call it directly.
Register each device that is being attached to the iommu with the lower
driver which then threads them on a linked list and calls the appropriate
driver op at the right time.
Currently the only use is if dma_unmap() is defined.
Also, fully lock all the debugging tests on the pinning path that a
dma_unmap is registered.
Signed-off-by: Jason Gunthorpe
---
drivers/vfio/vfio.c | 39 -
drivers/vfio/vfio.h | 14 ++-
drivers/vfio/vfio_iommu_type1.c | 74 -
include/linux/vfio.h| 2 +-
4 files changed, 58 insertions(+), 71 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index f005b644ab9e69..05623f52e38d32 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1077,17 +1077,6 @@ static void vfio_device_unassign_container(struct
vfio_device *device)
up_write(>group->group_rwsem);
}
-static int vfio_iommu_notifier(struct notifier_block *nb, unsigned long action,
- void *data)
-{
- struct vfio_device *vfio_device =
- container_of(nb, struct vfio_device, iommu_nb);
- struct vfio_iommu_type1_dma_unmap *unmap = data;
-
- vfio_device->ops->dma_unmap(vfio_device, unmap->iova, unmap->size);
- return NOTIFY_OK;
-}
-
static struct file *vfio_device_open(struct vfio_device *device)
{
struct vfio_iommu_driver *iommu_driver;
@@ -1123,15 +1112,9 @@ static struct file *vfio_device_open(struct vfio_device
*device)
}
iommu_driver = device->group->container->iommu_driver;
- if (device->ops->dma_unmap && iommu_driver &&
- iommu_driver->ops->register_notifier) {
- unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
-
- device->iommu_nb.notifier_call = vfio_iommu_notifier;
- iommu_driver->ops->register_notifier(
- device->group->container->iommu_data, ,
- >iommu_nb);
- }
+ if (iommu_driver && iommu_driver->ops->register_device)
+ iommu_driver->ops->register_device(
+ device->group->container->iommu_data, device);
up_read(>group->group_rwsem);
}
@@ -1171,11 +1154,9 @@ static struct file *vfio_device_open(struct vfio_device
*device)
device->ops->close_device(device);
iommu_driver = device->group->container->iommu_driver;
- if (device->ops->dma_unmap && iommu_driver &&
- iommu_driver->ops->register_notifier)
- iommu_driver->ops->unregister_notifier(
- device->group->container->iommu_data,
- >iommu_nb);
+ if (iommu_driver && iommu_driver->ops->register_device)
+ iommu_driver->ops->unregister_device(
+ device->group->container->iommu_data, device);
}
err_undo_count:
device->open_count--;
@@ -1380,11 +1361,9 @@ static int vfio_device_fops_release(struct inode *inode,
struct file *filep)
device->ops->close_device(device);
iommu_driver = device->group->container->iommu_driver;
- if (device->ops->dma_unmap && iommu_driver &&
- iommu_driver->ops->register_notifier)
- iommu_driver->ops->unregister_notifier(
- device->group->container->iommu_data,
- >iommu_nb);
+ if (iommu_driver && iommu_driver->ops->unregister_device)
+ iommu_driver->ops->unregister_device(
+ device->group->container->iommu_data, device);
up_read(>group->group_rwsem);
device->open_count--;
if (device->open_count == 0)
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index cb2e4e9baa8fe8..4a7db1f3c33e7e 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -33,11 +33,6 @@ enum vfio_iommu_notify_type {
VFIO_IOMMU_CONTAINER_CLOSE = 0,
};
-/* events for register_notifier() */
-enum {
- VFIO_IOMMU_NOTIFY_DMA_UNMAP = 1,
-};
-
/**
* struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks
*/
@@ -60,11 +55,10 @@ struct vfio_iommu_driver_ops {
unsigned long *phys_pfn);
int (*unpin_pages)(void *iommu_data,
unsigned long *user_pfn, int npage);
- int (*register_notifier)(void *iommu_data,
-unsigned long *events,
-struct notifier_block *nb);
- int (*unregister_notifier)(void *iommu_data,
-
[PATCH 1/2] vfio: Replace the DMA unmapping notifier with a callback
Instead of having drivers register the notifier with explicit code just
have them provide a dma_unmap callback op in their driver ops and rely on
the core code to wire it up.
Suggested-by: Christoph Hellwig
Signed-off-by: Jason Gunthorpe
---
drivers/gpu/drm/i915/gvt/gvt.h| 1 -
drivers/gpu/drm/i915/gvt/kvmgt.c | 75 ---
drivers/s390/cio/vfio_ccw_ops.c | 41 ++---
drivers/s390/cio/vfio_ccw_private.h | 1 -
drivers/s390/crypto/vfio_ap_ops.c | 54 +++
drivers/s390/crypto/vfio_ap_private.h | 3 -
drivers/vfio/vfio.c | 126 +-
drivers/vfio/vfio.h | 5 +
include/linux/vfio.h | 21 +
9 files changed, 92 insertions(+), 235 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index aee1a45da74bcb..705689e6401197 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -226,7 +226,6 @@ struct intel_vgpu {
unsigned long nr_cache_entries;
struct mutex cache_lock;
- struct notifier_block iommu_notifier;
atomic_t released;
struct kvm_page_track_notifier_node track_node;
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e2f6c56ab3420c..4000659ad715d4 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -729,34 +729,27 @@ int intel_gvt_set_edid(struct intel_vgpu *vgpu, int
port_num)
return ret;
}
-static int intel_vgpu_iommu_notifier(struct notifier_block *nb,
-unsigned long action, void *data)
+static void intel_vgpu_dma_unmap(struct vfio_device *vfio_dev, u64 iova,
+u64 length)
{
- struct intel_vgpu *vgpu =
- container_of(nb, struct intel_vgpu, iommu_notifier);
+ struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
+ struct gvt_dma *entry;
+ u64 iov_pfn, end_iov_pfn;
- if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) {
- struct vfio_iommu_type1_dma_unmap *unmap = data;
- struct gvt_dma *entry;
- unsigned long iov_pfn, end_iov_pfn;
+ iov_pfn = iova >> PAGE_SHIFT;
+ end_iov_pfn = iov_pfn + length / PAGE_SIZE;
- iov_pfn = unmap->iova >> PAGE_SHIFT;
- end_iov_pfn = iov_pfn + unmap->size / PAGE_SIZE;
+ mutex_lock(>cache_lock);
+ for (; iov_pfn < end_iov_pfn; iov_pfn++) {
+ entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
+ if (!entry)
+ continue;
- mutex_lock(>cache_lock);
- for (; iov_pfn < end_iov_pfn; iov_pfn++) {
- entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
- if (!entry)
- continue;
-
- gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
- entry->size);
- __gvt_cache_remove_entry(vgpu, entry);
- }
- mutex_unlock(>cache_lock);
+ gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
+ entry->size);
+ __gvt_cache_remove_entry(vgpu, entry);
}
-
- return NOTIFY_OK;
+ mutex_unlock(>cache_lock);
}
static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
@@ -783,36 +776,20 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
{
struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
- unsigned long events;
- int ret;
-
- vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier;
- events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
- ret = vfio_register_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, ,
->iommu_notifier);
- if (ret != 0) {
- gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n",
- ret);
- goto out;
- }
-
- ret = -EEXIST;
if (vgpu->attached)
- goto undo_iommu;
+ return -EEXIST;
- ret = -ESRCH;
if (!vgpu->vfio_device.kvm ||
vgpu->vfio_device.kvm->mm != current->mm) {
gvt_vgpu_err("KVM is required to use Intel vGPU\n");
- goto undo_iommu;
+ return -ESRCH;
}
kvm_get_kvm(vgpu->vfio_device.kvm);
- ret = -EEXIST;
if (__kvmgt_vgpu_exist(vgpu))
- goto undo_iommu;
+ return -EEXIST;
vgpu->attached = true;
@@ -831,12 +808,6 @@ static int intel_vgpu_open_device(struct vfio_device
*vfio_dev)
atomic_set(>released, 0);
return 0;
-
-undo_iommu:
- vfio_unregister_notifier(vfio_dev, VFIO_IOMMU_NOTIFY,
->iommu_notifier);
[PATCH 0/2] Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier
This is the last notifier toward the drivers, replace it with a simple op callback in the vfio_device_ops. Jason Gunthorpe (2): vfio: Replace the DMA unmapping notifier with a callback vfio: Replace the iommu notifier with a device list drivers/gpu/drm/i915/gvt/gvt.h| 1 - drivers/gpu/drm/i915/gvt/kvmgt.c | 75 +- drivers/s390/cio/vfio_ccw_ops.c | 41 +++--- drivers/s390/cio/vfio_ccw_private.h | 1 - drivers/s390/crypto/vfio_ap_ops.c | 54 +++-- drivers/s390/crypto/vfio_ap_private.h | 3 - drivers/vfio/vfio.c | 105 +- drivers/vfio/vfio.h | 9 +-- drivers/vfio/vfio_iommu_type1.c | 74 ++ include/linux/vfio.h | 21 +- 10 files changed, 114 insertions(+), 270 deletions(-) base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56 -- 2.36.1
RE: [PATCH v2 2/2] drm/virtio: fence created per cursor/plane update
Hi DW,
> Subject: [PATCH v2 2/2] drm/virtio: fence created per cursor/plane update
>
> Having one fence for a vgfb would cause conflict in case there are
> multiple planes referencing the same vgfb (e.g. Xorg screen covering
> two displays in extended mode) being flushed simultaneously. So it makes
> sence to use a separated fence for each plane update to prevent this.
>
> vgfb->fence is not required anymore with the suggested code change so
> both prepare_fb and cleanup_fb are removed since only fence creation/
> freeing are done in there.
>
> v2: - use the fence always as long as guest_blob is enabled on the
> scanout object
> - obj and fence initialized as NULL ptrs to avoid uninitialzed
> ptr problem (Reported by Dan Carpenter/kernel-test-robot)
>
> Reported-by: kernel test robot
> Reported-by: Dan Carpenter
> Cc: Gurchetan Singh
> Cc: Gerd Hoffmann
> Cc: Vivek Kasireddy
> Signed-off-by: Dongwon Kim
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 1 -
> drivers/gpu/drm/virtio/virtgpu_plane.c | 103 ++---
> 2 files changed, 39 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 0a194aaad419..4c59c1e67ca5 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -186,7 +186,6 @@ struct virtio_gpu_output {
>
> struct virtio_gpu_framebuffer {
> struct drm_framebuffer base;
> - struct virtio_gpu_fence *fence;
> };
> #define to_virtio_gpu_framebuffer(x) \
> container_of(x, struct virtio_gpu_framebuffer, base)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
> b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 6d3cc9e238a4..821023b7d57d 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -137,29 +137,37 @@ static void virtio_gpu_resource_flush(struct drm_plane
> *plane,
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct virtio_gpu_framebuffer *vgfb;
> struct virtio_gpu_object *bo;
> + struct virtio_gpu_object_array *objs = NULL;
> + struct virtio_gpu_fence *fence = NULL;
>
> vgfb = to_virtio_gpu_framebuffer(plane->state->fb);
> bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]);
> - if (vgfb->fence) {
> - struct virtio_gpu_object_array *objs;
>
> + if (!bo)
> + return;
[Kasireddy, Vivek] I think you can drop the above check as bo is guaranteed
to be valid in resource_flush as the necessary checks are already done early
in virtio_gpu_primary_plane_update().
> +
> + if (bo->dumb && bo->guest_blob)
> + fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context,
> +0);
> +
> + if (fence) {
> objs = virtio_gpu_array_alloc(1);
> - if (!objs)
> + if (!objs) {
> + kfree(fence);
> return;
> + }
> virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
> virtio_gpu_array_lock_resv(objs);
> - virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y,
> - width, height, objs, vgfb->fence);
> - virtio_gpu_notify(vgdev);
> + }
> +
> + virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y,
> + width, height, objs, fence);
> + virtio_gpu_notify(vgdev);
[Kasireddy, Vivek] I think it is OK to retain the existing style where all the
statements relevant for if (fence) would be lumped together. I do understand
that
the above two statements would be redundant in that case but it looks a bit
cleaner.
>
> - dma_fence_wait_timeout(>fence->f, true,
> + if (fence) {
> + dma_fence_wait_timeout(>f, true,
> msecs_to_jiffies(50));
> - dma_fence_put(>fence->f);
> - vgfb->fence = NULL;
> - } else {
> - virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y,
> - width, height, NULL, NULL);
> - virtio_gpu_notify(vgdev);
> + dma_fence_put(>f);
> }
> }
>
> @@ -239,47 +247,6 @@ static void virtio_gpu_primary_plane_update(struct
> drm_plane
> *plane,
> rect.y2 - rect.y1);
> }
>
> -static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
> -struct drm_plane_state *new_state)
> -{
> - struct drm_device *dev = plane->dev;
> - struct virtio_gpu_device *vgdev = dev->dev_private;
> - struct virtio_gpu_framebuffer *vgfb;
> - struct virtio_gpu_object *bo;
> -
> - if (!new_state->fb)
> - return 0;
> -
> - vgfb = to_virtio_gpu_framebuffer(new_state->fb);
> - bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]);
> - if (!bo ||
RE: [PATCH v2 1/2] drm/virtio: .release ops for virtgpu fence release
> virtio_gpu_fence_release is added to free virtio-gpu-fence
> upon release of dma_fence.
>
> Cc: Gurchetan Singh
> Cc: Gerd Hoffmann
> Cc: Vivek Kasireddy
> Signed-off-by: Dongwon Kim
> ---
> drivers/gpu/drm/virtio/virtgpu_fence.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c
> b/drivers/gpu/drm/virtio/virtgpu_fence.c
> index f28357dbde35..ba659ac2a51d 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_fence.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
> @@ -63,12 +63,20 @@ static void virtio_gpu_timeline_value_str(struct
> dma_fence *f,
> char *str,
>(u64)atomic64_read(>drv->last_fence_id));
> }
>
> +static void virtio_gpu_fence_release(struct dma_fence *f)
> +{
> + struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f);
> +
> + kfree(fence);
> +}
> +
> static const struct dma_fence_ops virtio_gpu_fence_ops = {
> .get_driver_name = virtio_gpu_get_driver_name,
> .get_timeline_name = virtio_gpu_get_timeline_name,
> .signaled= virtio_gpu_fence_signaled,
> .fence_value_str = virtio_gpu_fence_value_str,
> .timeline_value_str = virtio_gpu_timeline_value_str,
> + .release = virtio_gpu_fence_release,
Acked-by: Vivek Kasireddy
> };
>
> struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device
> *vgdev,
> --
> 2.20.1
Re: Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
Dne ponedeljek, 06. junij 2022 ob 10:17:20 CEST je Roman Stratiienko
napisal(a):
> Hello Jernej,
>
> Thank you for having a look.
>
> вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec :
> >
> > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko
napisal(a):
> > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > > "None", "Pre-multiplied", "Coverage"
> > >
> > > Add the blend mode property and route it to the appropriate registers.
> > >
> > > Note:
> > > "force_premulti" parameter was added to handle multi-overlay channel
> > > cases in future changes. It must be set to true for cases when more
> > > than 1 overlay layer is used within a channel and at least one of the
> > > overlay layers within a group uses premultiplied blending mode.
> >
> > Please remove this parameter. It's nothing special, so it can be easily
added
> > once it's actually needed. For now, it only complicates code.
>
> I would prefer keeping it if you do not have any strong opinion against it.
Actually I do. Patch will be smaller and easier to follow if there is no extra
variables with fixed values in it.
>
> I am working now on exposing all overlays, so it will be needed soon anyway.
Well, it will just be one patch more there, if at all.
Regards,
Jernej
>
> Also it helps to better understand the COV2PREMULT mode which has not
> the best description in the datasheet. Only after testing this
> register using devmem I became confident on its purpose.
>
> >
> > >
> > > Test:
> > > Manually tested all the modes using kmsxx python wrapper with and
> > > without 'force_premulti' flag enabled.
> > >
> > > Signed-off-by: Roman Stratiienko
> > > ---
> > > drivers/gpu/drm/sun4i/sun8i_mixer.h| 2 ++
> > > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 -
> > > drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++
> > > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++
> > > drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++
> > > 5 files changed, 94 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > > @@ -65,6 +65,8 @@
> > > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n) (0xf << ((n) << 2))
> > > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2)
> > >
> > > +#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe) BIT(pipe)
> > > +
> > > #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED BIT(1)
> > >
> > > #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch)
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 6ccbbca3176d..
29c0d9cca19a
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > @@ -58,24 +58,46 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer
> > > *mixer, int channel, }
> > >
> > > static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int
> > > channel, -int overlay, struct
> > drm_plane *plane)
> > > + int overlay, struct
> > drm_plane *plane,
> > > + unsigned int zpos,
> > bool force_premulti)
> > > {
> > > - u32 mask, val, ch_base;
> > > + u32 mask, val, ch_base, bld_base;
> > > + bool in_premulti, out_premulti;
> > >
> > > + bld_base = sun8i_blender_base(mixer);
> > > ch_base = sun8i_channel_base(mixer, channel);
> > >
> > > + in_premulti = plane->state->pixel_blend_mode ==
> > DRM_MODE_BLEND_PREMULTI;
> > > + out_premulti = force_premulti || in_premulti;
> > > +
> > > mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> > > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
> > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK;
> > >
> > > val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >>
> > 8);
> > >
> > > - val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
> > > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> > > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> > > + if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
> > > + } else {
> > > + val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE)
> > ?
> > > +
> > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> > > +
> > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> > > +
> > > + if (in_premulti)
> > > + val |=
> > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
> > > + }
> > > +
> > > + if (!in_premulti && out_premulti)
> > > + val |=
Re: [PATCH] drm/msm: Fix double pm_runtime_disable() call
On Mon, Jun 6, 2022 at 2:13 PM Maximilian Luz wrote:
>
> Following commit 17e822f7591f ("drm/msm: fix unbalanced
> pm_runtime_enable in adreno_gpu_{init, cleanup}"), any call to
> adreno_unbind() will disable runtime PM twice, as indicated by the call
> trees below:
>
> adreno_unbind()
>-> pm_runtime_force_suspend()
>-> pm_runtime_disable()
>
> adreno_unbind()
>-> gpu->funcs->destroy() [= aNxx_destroy()]
>-> adreno_gpu_cleanup()
>-> pm_runtime_disable()
>
> Note that pm_runtime_force_suspend() is called right before
> gpu->funcs->destroy() and both functions are called unconditionally.
>
> With recent addition of the eDP AUX bus code, this problem manifests
> itself when the eDP panel cannot be found yet and probing is deferred.
> On the first probe attempt, we disable runtime PM twice as described
> above. This then causes any later probe attempt to fail with
>
> [drm:adreno_load_gpu [msm]] *ERROR* Couldn't power up the GPU: -13
>
> preventing the driver from loading.
>
> As there seem to be scenarios where the aNxx_destroy() functions are not
> called from adreno_unbind(), simply removing pm_runtime_disable() from
> inside adreno_unbind() does not seem to be the proper fix. This is what
> commit 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
> adreno_gpu_{init, cleanup}") intended to fix. Therefore, instead check
> whether runtime PM is still enabled, and only disable it in that case.
>
> Fixes: 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
> adreno_gpu_{init, cleanup}")
> Signed-off-by: Maximilian Luz
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 4e665c806a14..f944b69e2a25 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -1057,7 +1057,8 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++)
> release_firmware(adreno_gpu->fw[i]);
>
> - pm_runtime_disable(>gpu_pdev->dev);
> + if (pm_runtime_enabled(>gpu_pdev->dev))
> + pm_runtime_disable(>gpu_pdev->dev);
>
> msm_gpu_cleanup(_gpu->base);
> }
> --
> 2.36.1
>
Re: [PATCH] drm/msm: Fix double pm_runtime_disable() call
On Mon 06 Jun 14:13 PDT 2022, Maximilian Luz wrote:
> Following commit 17e822f7591f ("drm/msm: fix unbalanced
> pm_runtime_enable in adreno_gpu_{init, cleanup}"), any call to
> adreno_unbind() will disable runtime PM twice, as indicated by the call
> trees below:
>
> adreno_unbind()
>-> pm_runtime_force_suspend()
>-> pm_runtime_disable()
>
> adreno_unbind()
>-> gpu->funcs->destroy() [= aNxx_destroy()]
>-> adreno_gpu_cleanup()
>-> pm_runtime_disable()
>
> Note that pm_runtime_force_suspend() is called right before
> gpu->funcs->destroy() and both functions are called unconditionally.
>
> With recent addition of the eDP AUX bus code, this problem manifests
> itself when the eDP panel cannot be found yet and probing is deferred.
> On the first probe attempt, we disable runtime PM twice as described
> above. This then causes any later probe attempt to fail with
>
> [drm:adreno_load_gpu [msm]] *ERROR* Couldn't power up the GPU: -13
>
> preventing the driver from loading.
>
> As there seem to be scenarios where the aNxx_destroy() functions are not
> called from adreno_unbind(), simply removing pm_runtime_disable() from
> inside adreno_unbind() does not seem to be the proper fix. This is what
> commit 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
> adreno_gpu_{init, cleanup}") intended to fix. Therefore, instead check
> whether runtime PM is still enabled, and only disable it in that case.
>
> Fixes: 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
> adreno_gpu_{init, cleanup}")
Tested-by: Bjorn Andersson
> Signed-off-by: Maximilian Luz
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 4e665c806a14..f944b69e2a25 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -1057,7 +1057,8 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++)
> release_firmware(adreno_gpu->fw[i]);
>
> - pm_runtime_disable(>gpu_pdev->dev);
> + if (pm_runtime_enabled(>gpu_pdev->dev))
> + pm_runtime_disable(>gpu_pdev->dev);
>
> msm_gpu_cleanup(_gpu->base);
> }
> --
> 2.36.1
>
Re: [PATCH v4 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Hi Hans, > > Please move this up so it is together with the other get_* methods, in > > alphabetic order. That is, right after get_modes(), and then this also > > matches the order in the .c file with is extra bonus. > > The downside of moving this up is that it will break drivers which don't > use c99 style named-struct-field initializers for there drm_panel_funcs. > > I admit that no drivers should be using the old style struct init, but > are we sure that that is the case? There is no in-tree driver that uses old style struct init for drm_panel_funcs - so we are safe here. I just did a quick git grep -A 4 drm_panel_funcs to verify it, browsing through the output did not reveal any old style users. Sam
RE: [RFC v3 1/3] drm/doc/rfc: VM_BIND feature design document
Regards, Oak > -Original Message- > From: Vishwanathapura, Niranjana > Sent: June 2, 2022 4:49 PM > To: Zeng, Oak > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Vetter, > Daniel ; Brost, Matthew ; > Hellstrom, Thomas ; ja...@jlekstrand.net; > Wilson, Chris P ; christian.koe...@amd.com > Subject: Re: [RFC v3 1/3] drm/doc/rfc: VM_BIND feature design document > > On Wed, Jun 01, 2022 at 07:13:16PM -0700, Zeng, Oak wrote: > > > > > >Regards, > >Oak > > > >> -Original Message- > >> From: dri-devel On Behalf Of > >> Niranjana Vishwanathapura > >> Sent: May 17, 2022 2:32 PM > >> To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > >> Vetter, > >> Daniel > >> Cc: Brost, Matthew ; Hellstrom, Thomas > >> ; ja...@jlekstrand.net; Wilson, Chris P > >> ; christian.koe...@amd.com > >> Subject: [RFC v3 1/3] drm/doc/rfc: VM_BIND feature design document > >> > >> VM_BIND design document with description of intended use cases. > >> > >> v2: Add more documentation and format as per review comments > >> from Daniel. > >> > >> Signed-off-by: Niranjana Vishwanathapura > >> > >> --- > >> Documentation/driver-api/dma-buf.rst | 2 + > >> Documentation/gpu/rfc/i915_vm_bind.rst | 304 > >> + > >> Documentation/gpu/rfc/index.rst| 4 + > >> 3 files changed, 310 insertions(+) > >> create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst > >> > >> diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver- > >> api/dma-buf.rst > >> index 36a76cbe9095..64cb924ec5bb 100644 > >> --- a/Documentation/driver-api/dma-buf.rst > >> +++ b/Documentation/driver-api/dma-buf.rst > >> @@ -200,6 +200,8 @@ DMA Fence uABI/Sync File > >> .. kernel-doc:: include/linux/sync_file.h > >> :internal: > >> > >> +.. _indefinite_dma_fences: > >> + > >> Indefinite DMA Fences > >> ~ > >> > >> diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst > >> b/Documentation/gpu/rfc/i915_vm_bind.rst > >> new file mode 100644 > >> index ..f1be560d313c > >> --- /dev/null > >> +++ b/Documentation/gpu/rfc/i915_vm_bind.rst > >> @@ -0,0 +1,304 @@ > >> +== > >> +I915 VM_BIND feature design and use cases > >> +== > >> + > >> +VM_BIND feature > >> + > >> +DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM > >> buffer > >> +objects (BOs) or sections of a BOs at specified GPU virtual addresses on a > >> +specified address space (VM). These mappings (also referred to as > >> persistent > >> +mappings) will be persistent across multiple GPU submissions (execbuff > >> calls) > >> +issued by the UMD, without user having to provide a list of all required > >> +mappings during each submission (as required by older execbuff mode). > >> + > >> +VM_BIND/UNBIND ioctls will support 'in' and 'out' fences to allow userpace > >> +to specify how the binding/unbinding should sync with other operations > >> +like the GPU job submission. These fences will be timeline 'drm_syncobj's > >> +for non-Compute contexts (See struct > >> drm_i915_vm_bind_ext_timeline_fences). > >> +For Compute contexts, they will be user/memory fences (See struct > >> +drm_i915_vm_bind_ext_user_fence). > >> + > >> +VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND. > >> +User has to opt-in for VM_BIND mode of binding for an address space (VM) > >> +during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND > >> extension. > >> + > >> +VM_BIND/UNBIND ioctl will immediately start binding/unbinding the > mapping in > >> an > >> +async worker. The binding and unbinding will work like a special GPU > >> engine. > >> +The binding and unbinding operations are serialized and will wait on > >> specified > >> +input fences before the operation and will signal the output fences upon > >> the > >> +completion of the operation. Due to serialization, completion of an > >> operation > >> +will also indicate that all previous operations are also complete. > > > >Hi, > > > >Is user required to wait for the out fence be signaled before submit a gpu > >job > using the vm_bind address? > >Or is user required to order the gpu job to make gpu job run after vm_bind > >out > fence signaled? > > > > Thanks Oak, > Either should be fine and up to user how to use vm_bind/unbind out-fence. > > >I think there could be different behavior on a non-faultable platform and a > faultable platform, such as on a non-faultable > >Platform, gpu job is required to be order after vm_bind out fence signaling; > >and > on a faultable platform, there is no such > >Restriction since vm bind can be finished in the fault handler? > > > > With GPU page faults handler, out fence won't be needed as residency is > purely managed by page fault handler populating page tables (there is a > mention of it in GPU Page Faults section below). > > >Should we document such thing? > > > >
Re: [PATCH v2 0/2] drm/bridge: analogix_dp: Self-refresh state machine fixes
Hi, On Fri, Jun 3, 2022 at 8:17 AM Doug Anderson wrote: > > Hi, > > On Fri, Jun 3, 2022 at 8:11 AM Sean Paul wrote: > > > > On Mon, May 23, 2022 at 5:51 PM Brian Norris > > wrote: > > > > > > On Thu, Mar 10, 2022 at 3:50 PM Brian Norris > > > wrote: > > > > On Mon, Feb 28, 2022 at 12:25 PM Brian Norris > > > > wrote: > > > > > > > Ping for review? Sean, perhaps? (You already reviewed this on the > > > > Chromium tracker.) > > > > > > Ping > > > > Apologies for the delay. Please in future ping on irc/chat if you're > > waiting for review from me, my inbox is often neglected. > > > > The set still looks good to me, > > > > Reviewed-by: Sean Paul > > Unless someone yells, I'll plan to apply both patches to > drm-misc-fixes early next week, possibly Monday. Seems like if someone > was going to object to these they've had plenty of time up until now. As promised, I pushed these to drm-misc-fixes: e54a4424925a drm/atomic: Force bridge self-refresh-exit on CRTC switch ca871659ec16 drm/bridge: analogix_dp: Support PSR-exit to disable transition -Doug
Re: [PATCH v4] drm/probe-helper: Default to 640x480 if no EDID on DP
Hi, On Wed, Jun 1, 2022 at 11:23 AM Douglas Anderson wrote: > > If we're unable to read the EDID for a display because it's corrupt / > bogus / invalid then we'll add a set of standard modes for the > display. Since we have no true information about the connected > display, these modes are essentially guesses but better than nothing. > At the moment, none of the modes returned is marked as preferred, but > the modes are sorted such that the higher resolution modes are listed > first. > > When userspace sees these modes presented by the kernel it needs to > figure out which one to pick. At least one userspace, ChromeOS [1] > seems to use the rules (which seem pretty reasonable): > 1. Try to pick the first mode marked as preferred. > 2. Try to pick the mode which matches the first detailed timing >descriptor in the EDID. > 3. If no modes were marked as preferred then pick the first mode. > > Unfortunately, userspace's rules combined with what the kernel is > doing causes us to fail section 4.2.2.6 (EDID Corruption Detection) of > the DP 1.4a Link CTS. That test case says that, while it's OK to allow > some implementation-specific fall-back modes if the EDID is bad that > userspace should _default_ to 640x480. > > Let's fix this by marking 640x480 as default for DP in the no-EDID > case. > > NOTES: > - In the discussion around v3 of this patch [2] there was talk about > solving this in userspace and I even implemented a patch that would > have solved this for ChromeOS, but then the discussion turned back > to solving this in the kernel. > - Also in the discussion of v3 [2] it was requested to limit this > 83;40900;0c change to just DP since folks were worried that it would break > some > subtle corner case on VGA or HDMI. > > [1] > https://source.chromium.org/chromium/chromium/src/+/a051f741d0a15caff2251301efe081c30e0f4a96:ui/ozone/platform/drm/common/drm_util.cc;l=488 > [2] > https://lore.kernel.org/r/20220513130533.v3.1.I31ec454f8d4ffce51a7708a8092f8a6f9c929092@changeid > > Signed-off-by: Douglas Anderson > Reviewed-by: Abhinav Kumar > --- > I put Abhinav's Reviewed-by tag from v2 here since this is nearly the > same as v2. Hope this is OK. > > Changes in v4: > - Code is back to v2, but limit to just DP. > - Beefed up the commit message. > > Changes in v3: > - Don't set preferred, just disable the sort. > > Changes in v2: > - Don't modify drm_add_modes_noedid() 'cause that'll break others > - Set 640x480 as preferred in drm_helper_probe_single_connector_modes() > > drivers/gpu/drm/drm_probe_helper.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) Pushed to drm-misc-next after cleaning up the turd that I somehow left in the commit message. fae7d186403e drm/probe-helper: Default to 640x480 if no EDID on DP -Doug
Re: [PATCH v4 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Hi,
On 6/6/22 21:20, Sam Ravnborg wrote:
> Hi Hsin-Yi,
> On Mon, Jun 06, 2022 at 11:24:24PM +0800, Hsin-Yi Wang wrote:
>> Panels usually call drm_connector_set_panel_orientation(), which is
>> later than drm/kms driver calling drm_dev_register(). This leads to a
>> WARN().
>>
>> The orientation property is known earlier. For example, some panels
>> parse the property through device tree during probe.
>>
>> Add an API to return the property from panel to drm/kms driver, so the
>> drivers are able to call drm_connector_set_panel_orientation() before
>> drm_dev_register().
>>
>> Suggested-by: Hans de Goede
>> Signed-off-by: Hsin-Yi Wang
>> Reviewed-by: Hans de Goede
>> Reviewed-by: Douglas Anderson
>> ---
>> v3->v4: Add a blank line.
>> ---
>> drivers/gpu/drm/drm_panel.c | 9 +
>> include/drm/drm_panel.h | 10 ++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index f634371c717a..e12056cfeca8 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -223,6 +223,15 @@ int drm_panel_get_modes(struct drm_panel *panel,
>> }
>> EXPORT_SYMBOL(drm_panel_get_modes);
>>
>> +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel
>> *panel)
> const as mentioned by Stephen.
>
>> +{
>> +if (panel && panel->funcs && panel->funcs->get_orientation)
>> +return panel->funcs->get_orientation(panel);
>> +
>> +return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
>> +}
>> +EXPORT_SYMBOL(drm_panel_get_orientation);
>> +
>> #ifdef CONFIG_OF
>> /**
>> * of_drm_find_panel - look up a panel using a device tree node
>> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
>> index d279ee455f01..5dadbf3b0370 100644
>> --- a/include/drm/drm_panel.h
>> +++ b/include/drm/drm_panel.h
>> @@ -133,6 +133,15 @@ struct drm_panel_funcs {
>> * Allows panels to create panels-specific debugfs files.
>> */
>> void (*debugfs_init)(struct drm_panel *panel, struct dentry *root);
>> +
>> +/**
>> + * @get_orientation:
>> + *
>> + * Return the panel orientation set by device tree or EDID.
>> + *
>> + * This function is optional.
>> + */
>> +enum drm_panel_orientation (*get_orientation)(struct drm_panel *panel);
>
> Please move this up so it is together with the other get_* methods, in
> alphabetic order. That is, right after get_modes(), and then this also
> matches the order in the .c file with is extra bonus.
The downside of moving this up is that it will break drivers which don't
use c99 style named-struct-field initializers for there drm_panel_funcs.
I admit that no drivers should be using the old style struct init, but
are we sure that that is the case?
Regards,
Hans
>
> With the two fixes:
> Reviewed-by: Sam Ravnborg
>
>> };
>>
>> /**
>> @@ -202,6 +211,7 @@ int drm_panel_enable(struct drm_panel *panel);
>> int drm_panel_disable(struct drm_panel *panel);
>>
>> int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector
>> *connector);
>> +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel
>> *panel);
>>
>> #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
>> struct drm_panel *of_drm_find_panel(const struct device_node *np);
>> --
>> 2.36.1.255.ge46751e96f-goog
>
[PATCH v2 2/2] drm/msm: Expose client engine utilization via fdinfo
From: Rob Clark
Similar to AMD commit
874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
infrastructure added in previous patches, we add basic client info
and GPU engine utilisation for msm.
Example output:
# cat /proc/`pgrep glmark2`/fdinfo/6
pos:0
flags: 0242
mnt_id: 21
ino:162
drm-driver: msm
drm-client-id: 7
drm-engine-gpu: 1734371319 ns
drm-cycles-gpu: 1153645024
drm-maxfreq-gpu:8 Hz
See also: https://patchwork.freedesktop.org/patch/468505/
Signed-off-by: Rob Clark
---
Documentation/gpu/drm-usage-stats.rst | 21 +
drivers/gpu/drm/msm/msm_drv.c | 19 ++-
drivers/gpu/drm/msm/msm_gpu.c | 21 +++--
drivers/gpu/drm/msm/msm_gpu.h | 19 +++
4 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpu/drm-usage-stats.rst
b/Documentation/gpu/drm-usage-stats.rst
index 6c9f166a8d6f..60e5cc9c13ad 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -105,6 +105,27 @@ object belong to this client, in the respective memory
region.
Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
indicating kibi- or mebi-bytes.
+- drm-cycles-
+
+Engine identifier string must be the same as the one specified in the
+drm-engine- tag and shall contain the number of busy cycles for the given
+engine.
+
+Values are not required to be constantly monotonic if it makes the driver
+implementation easier, but are required to catch up with the previously
reported
+larger value within a reasonable period. Upon observing a value lower than what
+was previously read, userspace is expected to stay with that larger previous
+value until a monotonic update is seen.
+
+- drm-maxfreq- [Hz|MHz|KHz]
+
+Engine identifier string must be the same as the one specified in the
+drm-engine- tag and shall contain the maxium frequence for the given
+engine. Taken together with drm-cycles-, this can be used to calculate
+percentage utilization of the engine, whereas drm-engine- only refects
+time active without considering what frequency the engine is operating as a
+percentage of it's maximum frequency.
+
===
Driver specific implementations
===
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 14ab9a627d8b..57a66093e671 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -948,7 +948,24 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query,
DRM_RENDER_ALLOW),
};
-DEFINE_DRM_GEM_FOPS(fops);
+static void msm_fop_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ struct drm_file *file = f->private_data;
+ struct drm_device *dev = file->minor->dev;
+ struct msm_drm_private *priv = dev->dev_private;
+ struct drm_printer p = drm_seq_file_printer(m);
+
+ if (!priv->gpu)
+ return;
+
+ msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, );
+}
+
+static const struct file_operations fops = {
+ .owner = THIS_MODULE,
+ DRM_GEM_FOPS,
+ .show_fdinfo = msm_fop_show_fdinfo,
+};
static const struct drm_driver msm_driver = {
.driver_features= DRIVER_GEM |
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index eb8a6663f309..333a9a299b41 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -4,6 +4,8 @@
* Author: Rob Clark
*/
+#include "drm/drm_drv.h"
+
#include "msm_gpu.h"
#include "msm_gem.h"
#include "msm_mmu.h"
@@ -146,6 +148,16 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
return 0;
}
+void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
+struct drm_printer *p)
+{
+ drm_printf(p, "drm-driver:\t%s\n", gpu->dev->driver->name);
+ drm_printf(p, "drm-client-id:\t%u\n", ctx->seqno);
+ drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns);
+ drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles);
+ drm_printf(p, "drm-maxfreq-gpu:\t%lu Hz\n", gpu->fast_rate);
+}
+
int msm_gpu_hw_init(struct msm_gpu *gpu)
{
int ret;
@@ -652,7 +664,7 @@ static void retire_submit(struct msm_gpu *gpu, struct
msm_ringbuffer *ring,
{
int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
volatile struct msm_gpu_submit_stats *stats;
- u64 elapsed, clock = 0;
+ u64 elapsed, clock = 0, cycles;
unsigned long flags;
stats = >memptrs->stats[index];
@@ -660,12 +672,17 @@ static void retire_submit(struct msm_gpu *gpu, struct
msm_ringbuffer *ring,
elapsed = (stats->alwayson_end - stats->alwayson_start) * 1;
do_div(elapsed, 192);
+ cycles = stats->cpcycles_end -
[PATCH v2 1/2] drm: Add DRM_GEM_FOPS
From: Rob Clark
The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
provide additional file ops, like show_fdinfo().
Signed-off-by: Rob Clark
---
include/drm/drm_gem.h | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 9d7c61a122dc..dc88d4a2cdf6 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -314,6 +314,23 @@ struct drm_gem_object {
const struct drm_gem_object_funcs *funcs;
};
+/**
+ * DRM_GEM_FOPS - Default drm GEM file operations
+ *
+ * This macro provides a shorthand for setting the GEM file ops in the
+ * _operations structure.
+ */
+#define DRM_GEM_FOPS \
+ .open = drm_open,\
+ .release= drm_release,\
+ .unlocked_ioctl = drm_ioctl,\
+ .compat_ioctl = drm_compat_ioctl,\
+ .poll = drm_poll,\
+ .read = drm_read,\
+ .llseek = noop_llseek,\
+ .mmap = drm_gem_mmap
+
+
/**
* DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
* @name: name for the generated structure
@@ -330,14 +347,7 @@ struct drm_gem_object {
#define DEFINE_DRM_GEM_FOPS(name) \
static const struct file_operations name = {\
.owner = THIS_MODULE,\
- .open = drm_open,\
- .release= drm_release,\
- .unlocked_ioctl = drm_ioctl,\
- .compat_ioctl = drm_compat_ioctl,\
- .poll = drm_poll,\
- .read = drm_read,\
- .llseek = noop_llseek,\
- .mmap = drm_gem_mmap,\
+ DRM_GEM_FOPS,\
}
void drm_gem_object_release(struct drm_gem_object *obj);
--
2.36.1
Re: [PATCH v4 8/8] drm/mediatek: Config orientation property if panel provides it
Hi Hsin-Yi,
On Mon, Jun 06, 2022 at 11:24:31PM +0800, Hsin-Yi Wang wrote:
> Panel orientation property should be set before drm_dev_register().
> Mediatek drm driver calls drm_dev_register() in .bind(). However, most
> panels sets orientation property relatively late, mostly in .get_modes()
> callback, since this is when they are able to get the connector and
> binds the orientation property to it, though the value should be known
> when the panel is probed.
>
> Let the drm driver check if the remote end point is a panel and if it
> contains the orientation property. If it does, set it before
> drm_dev_register() is called.
I do not know the best way to do what you need to do here.
But it seems wrong to introduce a deprecated function
(drm_of_find_panel_or_bridge), to set the rotation property early.
I think you need to find a way to do this utilising the panel_bridge or
something.
Sam
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Hans de Goede
> Reviewed-by: AngeloGioacchino Del Regno
>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index d9f10a33e6fa..c56282412bfa 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -185,6 +185,7 @@ struct mtk_dsi {
> struct drm_encoder encoder;
> struct drm_bridge bridge;
> struct drm_bridge *next_bridge;
> + struct drm_panel *panel;
> struct drm_connector *connector;
> struct phy *phy;
>
> @@ -822,6 +823,12 @@ static int mtk_dsi_encoder_init(struct drm_device *drm,
> struct mtk_dsi *dsi)
> ret = PTR_ERR(dsi->connector);
> goto err_cleanup_encoder;
> }
> +
> + /* Read panel orientation */
> + if (dsi->panel)
> + drm_connector_set_panel_orientation(dsi->connector,
> + drm_panel_get_orientation(dsi->panel));
> +
> drm_connector_attach_encoder(dsi->connector, >encoder);
>
> return 0;
> @@ -837,6 +844,9 @@ static int mtk_dsi_bind(struct device *dev, struct device
> *master, void *data)
> struct drm_device *drm = data;
> struct mtk_dsi *dsi = dev_get_drvdata(dev);
>
> + /* Get panel if existed */
> + drm_of_find_panel_or_bridge(dev->of_node, 0, 0, >panel, NULL);
> +
> ret = mtk_dsi_encoder_init(drm, dsi);
> if (ret)
> return ret;
> --
> 2.36.1.255.ge46751e96f-goog
Re: [PATCH v4 0/8] Add a panel API to return panel orientation
Hi Hsin-Yi, thanks for this nice series. On Mon, Jun 06, 2022 at 11:24:23PM +0800, Hsin-Yi Wang wrote: > Panels usually call drm_connector_set_panel_orientation(), which is > later than drm/kms driver calling drm_dev_register(). This leads to a > WARN()[1]. > > The orientation property is known earlier. For example, some panels > parse the property through device tree during probe. > > The series add a panel API drm_panel_get_orientation() for drm/kms > drivers. The drivers can use the API to get panel's orientation, so they > can call drm_connector_set_panel_orientation() before drm_dev_register(). > > Panel needs to implement .get_orientation callback to return the property. The following comment appears in every panel driver: + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ Please move it to the drm_panel c or h file, it is noise to add it to all panel drivers. Sam
Re: [PATCH v4 5/8] drm/panel: panel-simple: Implement .get_orientation callback
Hi Hsin-Yi,
On Mon, Jun 06, 2022 at 11:24:28PM +0800, Hsin-Yi Wang wrote:
> To return the orientation property to drm/kms driver.
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Hans de Goede
> Reviewed-by: Douglas Anderson
> ---
> drivers/gpu/drm/panel/panel-simple.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 4a2e580a2f7b..f232b8cf4075 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -411,7 +411,12 @@ static int panel_simple_get_modes(struct drm_panel
> *panel,
> /* add hard-coded panel modes */
> num += panel_simple_get_non_edid_modes(p, connector);
>
> - /* set up connector's "panel orientation" property */
> + /*
> + * drm drivers are expected to call drm_panel_get_orientation() to get
> + * panel's orientation then drm_connector_set_panel_orientation() to
> + * set the property before drm_dev_register(). Otherwise there will be
> + * a WARN_ON if orientation is set after drm is registered.
> + */
This comment is not really relevant here. If we need to explain this
then put it in drm_panel.c/h - as this applies for all panels and not
just the panel_simple.
Keep in mind, this is the source new panels often use a inspiration and
no need to have this copied around.
> drm_connector_set_panel_orientation(connector, p->orientation);
>
> return num;
> @@ -434,6 +439,14 @@ static int panel_simple_get_timings(struct drm_panel
> *panel,
> return p->desc->num_timings;
> }
>
> +static enum drm_panel_orientation panel_simple_get_orientation(struct
> drm_panel *panel)
> +{
> + struct panel_simple *p = to_panel_simple(panel);
> +
> + return p->orientation;
> +}
> +
> +
> static const struct drm_panel_funcs panel_simple_funcs = {
> .disable = panel_simple_disable,
> .unprepare = panel_simple_unprepare,
> @@ -441,6 +454,7 @@ static const struct drm_panel_funcs panel_simple_funcs = {
> .enable = panel_simple_enable,
> .get_modes = panel_simple_get_modes,
> .get_timings = panel_simple_get_timings,
> + .get_orientation = panel_simple_get_orientation,
I like the order in this list to match the order in the .h file.
So my OCD would like you to move it up right after get_modes,
but feel free to ignore this.
With the suggested changes:
Reviewed-by: Sam Ravnborg
> };
>
> static struct panel_desc panel_dpi;
> --
> 2.36.1.255.ge46751e96f-goog
Re: [PATCH v3] dt-bindings: display: bridge: sil, sii9022: Convert to json-schema
On Thu, 19 May 2022 15:41:06 +0200, Geert Uytterhoeven wrote: > Convert the Silicon Image sii902x HDMI bridge Device Tree binding > documentation to json-schema. > > Add missing sil,sii9022-cpi and sil,sii9022-tpi compatible values. > > Signed-off-by: Geert Uytterhoeven > --- > v3: > - Add comments clarifying CPI/TPI, > - Improve wording, > - Drop port@2, > - Add port descriptions, > - Add schema references to individual ports. > > v2: > - Rework sil,i2s-data-lanes, > - Add schema reference to ports. > --- > .../bindings/display/bridge/sii902x.txt | 78 --- > .../bindings/display/bridge/sil,sii9022.yaml | 131 ++ > 2 files changed, 131 insertions(+), 78 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/bridge/sii902x.txt > create mode 100644 > Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml > Applied, thanks!
Re: [PATCH v4 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Hi Hsin-Yi,
On Mon, Jun 06, 2022 at 11:24:24PM +0800, Hsin-Yi Wang wrote:
> Panels usually call drm_connector_set_panel_orientation(), which is
> later than drm/kms driver calling drm_dev_register(). This leads to a
> WARN().
>
> The orientation property is known earlier. For example, some panels
> parse the property through device tree during probe.
>
> Add an API to return the property from panel to drm/kms driver, so the
> drivers are able to call drm_connector_set_panel_orientation() before
> drm_dev_register().
>
> Suggested-by: Hans de Goede
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Hans de Goede
> Reviewed-by: Douglas Anderson
> ---
> v3->v4: Add a blank line.
> ---
> drivers/gpu/drm/drm_panel.c | 9 +
> include/drm/drm_panel.h | 10 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index f634371c717a..e12056cfeca8 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -223,6 +223,15 @@ int drm_panel_get_modes(struct drm_panel *panel,
> }
> EXPORT_SYMBOL(drm_panel_get_modes);
>
> +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel)
const as mentioned by Stephen.
> +{
> + if (panel && panel->funcs && panel->funcs->get_orientation)
> + return panel->funcs->get_orientation(panel);
> +
> + return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +}
> +EXPORT_SYMBOL(drm_panel_get_orientation);
> +
> #ifdef CONFIG_OF
> /**
> * of_drm_find_panel - look up a panel using a device tree node
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index d279ee455f01..5dadbf3b0370 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -133,6 +133,15 @@ struct drm_panel_funcs {
>* Allows panels to create panels-specific debugfs files.
>*/
> void (*debugfs_init)(struct drm_panel *panel, struct dentry *root);
> +
> + /**
> + * @get_orientation:
> + *
> + * Return the panel orientation set by device tree or EDID.
> + *
> + * This function is optional.
> + */
> + enum drm_panel_orientation (*get_orientation)(struct drm_panel *panel);
Please move this up so it is together with the other get_* methods, in
alphabetic order. That is, right after get_modes(), and then this also
matches the order in the .c file with is extra bonus.
With the two fixes:
Reviewed-by: Sam Ravnborg
> };
>
> /**
> @@ -202,6 +211,7 @@ int drm_panel_enable(struct drm_panel *panel);
> int drm_panel_disable(struct drm_panel *panel);
>
> int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector
> *connector);
> +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel
> *panel);
>
> #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
> struct drm_panel *of_drm_find_panel(const struct device_node *np);
> --
> 2.36.1.255.ge46751e96f-goog
Re: [PATCH v4 8/8] drm/mediatek: Config orientation property if panel provides it
Quoting Hsin-Yi Wang (2022-06-06 08:24:31) > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index d9f10a33e6fa..c56282412bfa 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -822,6 +823,12 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, > struct mtk_dsi *dsi) > ret = PTR_ERR(dsi->connector); > goto err_cleanup_encoder; > } > + > + /* Read panel orientation */ > + if (dsi->panel) > + drm_connector_set_panel_orientation(dsi->connector, > + drm_panel_get_orientation(dsi->panel)); > + It could be simplified like so? drm_connector_set_orientation_from_panel(dsi->connector, dsi->panel); Then the API could get the orientation if the panel pointer is valid. Does any code need to use/modify the orientation value besides drm_connector_set_panel_orientation()? > drm_connector_attach_encoder(dsi->connector, >encoder); > > return 0;
Re: [PATCH v4 4/8] drm/panel: lvds: Implement .get_orientation callback
Quoting Hsin-Yi Wang (2022-06-06 08:24:27)
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> b/drivers/gpu/drm/panel/panel-lvds.c
> index f11252fb00fe..491b64c2c8d6 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -99,15 +99,30 @@ static int panel_lvds_get_modes(struct drm_panel *panel,
> drm_display_info_set_bus_formats(>display_info,
> >bus_format, 1);
> connector->display_info.bus_flags = lvds->bus_flags;
> +
> + /*
> +* drm drivers are expected to call drm_panel_get_orientation() to get
> +* panel's orientation then drm_connector_set_panel_orientation() to
> +* set the property before drm_dev_register(). Otherwise there will be
> +* a WARN_ON if orientation is set after drm is registered.
> +*/
Should this comment also be a "TODO: Remove once all drm drivers call
drm_connector_set_panel_orientation()"?
> drm_connector_set_panel_orientation(connector, lvds->orientation);
>
> return 1;
> }
>
> +static enum drm_panel_orientation panel_lvds_get_orientation,(struct
> drm_panel *panel)
Stray comma here---^
> +{
> + struct panel_lvds *lvds = to_panel_lvds(panel);
> +
> + return lvds->orientation;
> +}
> +
Re: [PATCH v4 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Quoting Hsin-Yi Wang (2022-06-06 08:24:24)
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index f634371c717a..e12056cfeca8 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -223,6 +223,15 @@ int drm_panel_get_modes(struct drm_panel *panel,
> }
> EXPORT_SYMBOL(drm_panel_get_modes);
>
> +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel)
Should 'panel' be marked const to indicate that it can't be modified?
> +{
> + if (panel && panel->funcs && panel->funcs->get_orientation)
> + return panel->funcs->get_orientation(panel);
[PATCH] drm/doc: Add KUnit documentation
Explain how to run the KUnit tests present in the DRM subsystem and clarify why the UML-only options were not added to the configuration file present in drivers/gpu/drm/.kunitconfig [1] [2]. [1] https://lore.kernel.org/dri-devel/CABVgOSn8i=lo5p7830h2xu1jgg0krn0qtnxkomhf1otgxja...@mail.gmail.com/ [2] https://lore.kernel.org/dri-devel/CAGS_qxqpiCim_sy1LDK7PLwVgWf-LKW+uNFTGM=t7ydk-dy...@mail.gmail.com/ Signed-off-by: José Expósito --- Documentation/gpu/drm-internals.rst | 31 + 1 file changed, 31 insertions(+) diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index 38afed24a75c..08f115417381 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -207,6 +207,37 @@ Utilities :internal: +Unit testing + + +KUnit +- + +KUnit (Kernel unit testing framework) provides a common framework for unit tests +within the Linux kernel. + +This section covers the specifics for the DRM subsystem. For general information +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst. + +How to run the tests? +~ + +In order to facilitate running the test suite, a configuration file is present +in ``drivers/gpu/drm/.kunitconfig``. It can be used by ``kunit.py`` as follows: + +.. code-block:: bash + + $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ + --kconfig_add CONFIG_VIRTIO_UML=y \ + --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y + +.. note:: + The configuration included in ``.kunitconfig`` should be as generic as + possible. + ``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not + included in it because they are only required for User Mode Linux. + + Legacy Support Code === -- 2.25.1
[PATCH v2] drm/msm/dp: check core_initialized before disable interrupts at dp_display_unbind()
During msm initialize phase, dp_display_unbind() will be called to undo
initializations had been done by dp_display_bind() previously if there is
error happen at msm_drm_bind. In this case, core_initialized flag had to
be check to make sure clocks is on before update DP controller register
to disable HPD interrupts. Otherwise system will crash due to below NOC
fatal error.
QTISECLIB [01f01a7ad]CNOC2 ERROR: ERRLOG0_LOW = 0x00061007
QTISECLIB [01f01a7ad]GEM_NOC ERROR: ERRLOG0_LOW = 0x1007
QTISECLIB [01f0371a0]CNOC2 ERROR: ERRLOG0_HIGH = 0x0003
QTISECLIB [01f055297]GEM_NOC ERROR: ERRLOG0_HIGH = 0x0003
QTISECLIB [01f072beb]CNOC2 ERROR: ERRLOG1_LOW = 0x0024
QTISECLIB [01f0914b8]GEM_NOC ERROR: ERRLOG1_LOW = 0x0042
QTISECLIB [01f0ae639]CNOC2 ERROR: ERRLOG1_HIGH = 0x4002
QTISECLIB [01f0cc73f]GEM_NOC ERROR: ERRLOG1_HIGH = 0x4002
QTISECLIB [01f0ea092]CNOC2 ERROR: ERRLOG2_LOW = 0x0009020c
QTISECLIB [01f10895f]GEM_NOC ERROR: ERRLOG2_LOW = 0x0ae9020c
QTISECLIB [01f125ae1]CNOC2 ERROR: ERRLOG2_HIGH = 0x
QTISECLIB [01f143be7]GEM_NOC ERROR: ERRLOG2_HIGH = 0x
QTISECLIB [01f16153a]CNOC2 ERROR: ERRLOG3_LOW = 0x
QTISECLIB [01f17fe07]GEM_NOC ERROR: ERRLOG3_LOW = 0x
QTISECLIB [01f19cf89]CNOC2 ERROR: ERRLOG3_HIGH = 0x
QTISECLIB [01f1bb08e]GEM_NOC ERROR: ERRLOG3_HIGH = 0x
QTISECLIB [01f1d8a31]CNOC2 ERROR: SBM1 FAULTINSTATUS0_LOW = 0x0002
QTISECLIB [01f1f72a4]GEM_NOC ERROR: SBM0 FAULTINSTATUS0_LOW = 0x0001
QTISECLIB [01f21a217]CNOC3 ERROR: ERRLOG0_LOW = 0x0006
QTISECLIB [01f23dfd3]NOC error fatal
changes in v2:
-- drop the first patch (drm/msm: enable msm irq after all initializations are
done successfully at msm_drm_init()) since the problem had been fixed by other
patch
Fixes: a65c95ff88f2 ("drm/msm/dp: stop event kernel thread when DP unbind")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index da5c03a..2b72639 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -309,7 +309,8 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct msm_drm_private *priv = dev_get_drvdata(master);
/* disable all HPD interrupts */
- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+ if (dp->core_initialized)
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK,
false);
kthread_stop(dp->ev_tsk);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH 2/2] vfio/pci: Remove console drivers
Console drivers can create conflicts with PCI resources resulting in userspace getting mmap failures to memory BARs. This is especially evident when trying to re-use the system primary console for userspace drivers. Attempt to remove all nature of conflicting drivers as part of our VGA initialization. Reported-by: Laszlo Ersek Tested-by: Laszlo Ersek Suggested-by: Gerd Hoffmann Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_core.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index a0d69ddaf90d..e0cbcbc2aee1 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -29,6 +30,8 @@ #include +#include + #define DRIVER_AUTHOR "Alex Williamson " #define DRIVER_DESC "core driver for VFIO based PCI devices" @@ -1793,6 +1796,20 @@ static int vfio_pci_vga_init(struct vfio_pci_core_device *vdev) if (!vfio_pci_is_vga(pdev)) return 0; +#if IS_REACHABLE(CONFIG_DRM) + drm_aperture_detach_platform_drivers(pdev); +#endif + +#if IS_REACHABLE(CONFIG_FB) + ret = remove_conflicting_pci_framebuffers(pdev, vdev->vdev.ops->name); + if (ret) + return ret; +#endif + + ret = vga_remove_vgacon(pdev); + if (ret) + return ret; + ret = vga_client_register(pdev, vfio_pci_set_decode); if (ret) return ret;
[PATCH 0/2] Improve vfio-pci primary GPU assignment behavior
Users attempting to enable vfio PCI device assignment with a GPU will often block the default PCI driver from the device to avoid conflicts with the device initialization or release path. This means that vfio-pci is sometimes the first PCI driver to bind to the device. In the case of assigning the primary graphics device, low-level console drivers may still generate resource conflicts. Users often employ kernel command line arguments to disable conflicting drivers or perform unbinding in userspace to avoid this, but the actual solution is often distribution/kernel config specific based on the included drivers. We can instead allow vfio-pci to copy the behavior of drm_aperture_remove_conflicting_pci_framebuffers() in order to remove these low-level drivers with conflicting resources. vfio-pci is not however a DRM driver, nor does vfio-pci depend on DRM config options, thus we split out and export the necessary DRM apterture support and mirror the framebuffer and VGA support. I'd be happy to pull this series in through the vfio branch if approved by the DRM maintainers. Thanks, Alex --- Alex Williamson (2): drm/aperture: Split conflicting platform driver removal vfio/pci: Remove console drivers drivers/gpu/drm/drm_aperture.c | 33 +++- drivers/vfio/pci/vfio_pci_core.c | 17 include/drm/drm_aperture.h | 2 ++ 3 files changed, 43 insertions(+), 9 deletions(-)
[PATCH 1/2] drm/aperture: Split conflicting platform driver removal
Split the removal of platform drivers conflicting with PCI resources from
drm_aperture_remove_conflicting_pci_framebuffers() to support both non-DRM
callers and better modularity. This is intended to support the vfio-pci
driver, which can acquire ownership of PCI graphics devices, but is not
itself a DRM or FB driver, and therefore has no Kconfig dependencies. The
remaining actions of drm_aperture_remove_conflicting_pci_framebuffers() are
already exported from their respective subsystems, therefore this allows
vfio-pci to separate each set of conflicts independently based on the
configured features.
Reported-by: Laszlo Ersek
Tested-by: Laszlo Ersek
Suggested-by: Gerd Hoffmann
Signed-off-by: Alex Williamson
---
drivers/gpu/drm/drm_aperture.c | 33 -
include/drm/drm_aperture.h |2 ++
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 74bd4a76b253..5b2500f7fb8b 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -313,6 +313,28 @@ int
drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_
}
EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
+/**
+ * drm_aperture_detach_platform_drivers - detach platform drivers conflicting
with PCI device
+ * @pdev: PCI device
+ *
+ * This function detaches platform drivers with resource conflicts to the
memory
+ * bars of the provided @pdev.
+ */
+void drm_aperture_detach_platform_drivers(struct pci_dev *pdev)
+{
+ resource_size_t base, size;
+ int bar;
+
+ for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
+ if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
+ continue;
+ base = pci_resource_start(pdev, bar);
+ size = pci_resource_len(pdev, bar);
+ drm_aperture_detach_drivers(base, size);
+ }
+}
+EXPORT_SYMBOL(drm_aperture_detach_platform_drivers);
+
/**
* drm_aperture_remove_conflicting_pci_framebuffers - remove existing
framebuffers for PCI devices
* @pdev: PCI device
@@ -328,16 +350,9 @@
EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
const struct drm_driver
*req_driver)
{
- resource_size_t base, size;
- int bar, ret = 0;
+ int ret = 0;
- for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
- if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
- continue;
- base = pci_resource_start(pdev, bar);
- size = pci_resource_len(pdev, bar);
- drm_aperture_detach_drivers(base, size);
- }
+ drm_aperture_detach_platform_drivers(pdev);
/*
* WARNING: Apparently we must kick fbdev drivers before vgacon,
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
index 7096703c3949..53fd36fa258e 100644
--- a/include/drm/drm_aperture.h
+++ b/include/drm/drm_aperture.h
@@ -15,6 +15,8 @@ int devm_aperture_acquire_from_firmware(struct drm_device
*dev, resource_size_t
int drm_aperture_remove_conflicting_framebuffers(resource_size_t base,
resource_size_t size,
bool primary, const struct
drm_driver *req_driver);
+void drm_aperture_detach_platform_drivers(struct pci_dev *pdev);
+
int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
const struct drm_driver
*req_driver);
Re: [PATCH] drm/i2c: tda9950: Lower severity of log message about missing interrupts
On Mon, Jun 06, 2022 at 06:14:36PM +0100, Mark Brown wrote: > The tda9950 driver prints an error message if it is instantiated without > an interrupt being available since the device is non-functional in that > case. Unfortunately due to packaging of tda9950 with tda998x series devices > the tda998x driver unconditionally instantiates a tda9950 so systems with a > tda998x configured without an interrupt will trigger this error message > during boot if tda9950 support is available. Reduce the severity to debug > level so this is less likely to be presented to end users, the information > is still there for system integrators who run into problems. > > We could add a check for an interrupt to the tda998x driver instead but > this feels better from an encapsulation point of view, there's still a log > message to help anyone doing system integration. As the tda998x also makes use of the interrupt, it would be trivial to avoid instantiating the tda9950 device if there's no interrupt. tda9950 does require it, and if it's missing, then it isn't functional. No point wasting memory on the struct device. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v1 1/2] drm/msm: enable msm irq after all initializations are done successfully at msm_drm_init()
I will drop this patch since the problem had been fixed by Dmitry
Baryshkov patch.
https://gitlab.freedesktop.org/drm/msm/-/commit/577e2a9dfc8fba7938aaf75db63fae7e328cc3cb
On 6/3/2022 2:28 PM, Kuogee Hsieh wrote:
At msm initialize phase, msm_drm_init() is called to initialize modules
sequentially. It will call msm_drm_uninit() to clean up initialized phase
if any module initialization return failed. This patch move msm_irq_install()
to the last step of msm_drm_init() after all modules are initialized
successfully
so that no msm_irq_unistall() required at msm_drm_uninit() if any module
initialization failed happen at msm_drm_init().
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/msm_drv.c | 29 ++---
1 file changed, 6 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 6665c5a..ab42e9a 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -132,15 +132,6 @@ static int msm_irq_install(struct drm_device *dev,
unsigned int irq)
return 0;
}
-static void msm_irq_uninstall(struct drm_device *dev)
-{
- struct msm_drm_private *priv = dev->dev_private;
- struct msm_kms *kms = priv->kms;
-
- kms->funcs->irq_uninstall(kms);
- free_irq(kms->irq, dev);
-}
-
struct msm_vblank_work {
struct work_struct work;
int crtc_id;
@@ -232,10 +223,6 @@ static int msm_drm_uninit(struct device *dev)
drm_mode_config_cleanup(ddev);
- pm_runtime_get_sync(dev);
- msm_irq_uninstall(ddev);
- pm_runtime_put_sync(dev);
-
if (kms && kms->funcs)
kms->funcs->destroy(kms);
@@ -437,16 +424,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
goto err_msm_uninit;
}
- if (kms) {
- pm_runtime_get_sync(dev);
- ret = msm_irq_install(ddev, kms->irq);
- pm_runtime_put_sync(dev);
- if (ret < 0) {
- DRM_DEV_ERROR(dev, "failed to install IRQ handler\n");
- goto err_msm_uninit;
- }
- }
-
ret = drm_dev_register(ddev, 0);
if (ret)
goto err_msm_uninit;
@@ -467,6 +444,12 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
if (ret)
goto err_msm_uninit;
+ if (kms) {
+ pm_runtime_get_sync(dev);
+ msm_irq_install(ddev, kms->irq);
+ pm_runtime_put_sync(dev);
+ }
+
drm_kms_helper_poll_init(ddev);
return 0;
Re: [PATCH] MAINTAINERS: rectify entries for ARM DRM DRIVERS after dt conversion
On Wed, 01 Jun 2022 06:17:46 +0200, Lukas Bulwahn wrote:
> The three commits:
>
> 36fd2a65bcaf ("dt-bindings: display: convert Arm HDLCD to DT schema")
> 0f6983509ea1 ("dt-bindings: display: convert Arm Komeda to DT schema")
> 2c8b082a3ab1 ("dt-bindings: display: convert Arm Mali-DP to DT schema")
>
> convert the arm display dt-bindings, arm,*.txt to arm,*.yaml, but miss to
> adjust its reference in MAINTAINERS.
>
> Hence, ./scripts/get_maintainer.pl --self-test=patterns complains about
> broken references.
>
> Repair these file references in ARM HDLCD DRM DRIVER, ARM KOMEDA DRM-KMS
> DRIVER and ARM MALI-DP DRM DRIVER.
>
> Signed-off-by: Lukas Bulwahn
> ---
> Andre, please ack.
> Rob, Krzysztof, please pick this minor non-urgent clean-up patch in
> your -next dt tree.
>
> MAINTAINERS | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Applied, thanks!
[PATCH] drm/i2c: tda9950: Lower severity of log message about missing interrupts
The tda9950 driver prints an error message if it is instantiated without
an interrupt being available since the device is non-functional in that
case. Unfortunately due to packaging of tda9950 with tda998x series devices
the tda998x driver unconditionally instantiates a tda9950 so systems with a
tda998x configured without an interrupt will trigger this error message
during boot if tda9950 support is available. Reduce the severity to debug
level so this is less likely to be presented to end users, the information
is still there for system integrators who run into problems.
We could add a check for an interrupt to the tda998x driver instead but
this feels better from an encapsulation point of view, there's still a log
message to help anyone doing system integration.
Signed-off-by: Mark Brown
---
drivers/gpu/drm/i2c/tda9950.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
index 5b03fdd1eaa4..781d5665cd04 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -397,7 +397,7 @@ static int tda9950_probe(struct i2c_client *client,
/* We must have an interrupt to be functional. */
if (client->irq <= 0) {
- dev_err(>dev, "driver requires an interrupt\n");
+ dev_dbg(>dev, "driver requires an interrupt\n");
return -ENXIO;
}
--
2.30.2
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hi! Javier Martinez Canillas wrote: > Hello José, > > On 6/6/22 11:55, José Expósito wrote: > > Test the conversion from XRGB to RGB332. > > > > What is tested? > > > > - Different values for the X in XRGB to make sure it is ignored > > - Different clip values: Single pixel and full and partial buffer > > - Well known colors: White, black, red, green, blue, magenta, yellow > >and cyan > > - Other colors: Randomly picked > > - Destination pitch > > > > How to run the tests? > > > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ > > --kconfig_add CONFIG_VIRTIO_UML=y \ > > --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y > > > > Suggested-by: Javier Martinez Canillas > > Signed-off-by: José Expósito > > > > --- > > Thanks for addressing the issues pointed out. Patch looks good to me now. > > Reviewed-by: Javier Martinez Canillas Thanks for the quick review Javier :) Javier Martinez Canillas wrote: > By the way, I think you should request an account [0], so that you can push > patches to drm-misc directly. Specially since AFAIU the plan is to add more > KUnit tests in future patch series. > > [0]: https://www.freedesktop.org/wiki/AccountRequests/ I'll request one, thanks for the tip. --- Maxime Ripard wrote: > > > The following works correctly but it won't use User Mode Linux: > > > > > > ./tools/testing/kunit/kunit.py run > > > --kunitconfig=drivers/gpu/drm/.kunitconfig --arch=x86_64 > > > > > > But then, can't we add them to .kunitconfig? > > > > > > > That's what I asked in the previous RFC too. Daniel mentioned that it > > shouldn't > > go there because is platform specific (AFAIU, one might want to test it on > > x86, > > aarch64, etc) but then I asked why we couldn't have a arch/um/.kunitconfig. > > > > The answer was that's not that simple and some agreement on how to do it is > > needed: > > > > https://lists.freedesktop.org/archives/dri-devel/2022-June/357617.html > > We should probably just document it somewhere in KMS then? It doesn't > have to be in this patch series, but I have the feeling that we will end > up with that discussion a lot from people frustrated to have spent too > much time figuring it out :) My understanding from Daniel's comment [1] is also that at the moment it is not easy to support this use case, so yes, at least copy and pasting the command in the docs should help everyone figure out how to run the tests. Documentation/gpu/drm-internals.rst seems like a good place to add some information about how to run and add tests. I'll send a patch with the docs ASAP. Jose [1] https://lore.kernel.org/dri-devel/CAGS_qxqpiCim_sy1LDK7PLwVgWf-LKW+uNFTGM=t7ydk-dy...@mail.gmail.com/
Re: [PATCH v4] dma-buf: Add a capabilities directory
On 2022-06-06 16:22, Greg KH wrote: On Mon, Jun 06, 2022 at 04:10:09PM +0100, Robin Murphy wrote: On 2022-06-02 07:47, Daniel Vetter wrote: On Thu, 2 Jun 2022 at 08:34, Simon Ser wrote: On Thursday, June 2nd, 2022 at 08:25, Greg KH wrote: On Thu, Jun 02, 2022 at 06:17:31AM +, Simon Ser wrote: On Thursday, June 2nd, 2022 at 07:40, Greg KH g...@kroah.com wrote: On Wed, Jun 01, 2022 at 04:13:14PM +, Simon Ser wrote: To discover support for new DMA-BUF IOCTLs, user-space has no choice but to try to perform the IOCTL on an existing DMA-BUF. Which is correct and how all kernel features work (sorry I missed the main goal of this patch earlier and focused only on the sysfs stuff). However, user-space may want to figure out whether or not the IOCTL is available before it has a DMA-BUF at hand, e.g. at initialization time in a Wayland compositor. Why not just do the ioctl in a test way? That's how we determine kernel features, we do not poke around in sysfs to determine what is, or is not, present at runtime. Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF subsystem to advertise supported features. Add a sync_file_import_export entry which indicates that importing and exporting sync_files from/to DMA-BUFs is supported. No, sorry, this is not a sustainable thing to do for all kernel features over time. Please just do the ioctl and go from there. sysfs is not for advertising what is and is not enabled/present in a kernel with regards to functionality or capabilities of the system. If sysfs were to export this type of thing, it would have to do it for everything, not just some random tiny thing of one kernel driver. I'd argue that DMA-BUF is a special case here. So this is special and unique just like everything else? :) To check whether the import/export IOCTLs are available, user-space needs a DMA-BUF to try to perform the IOCTL. To get a DMA-BUF, user-space needs to enumerate GPUs, pick one at random, load GBM or Vulkan, use that heavy-weight API to allocate a "fake" buffer on the GPU, export that buffer into a DMA-BUF, try the IOCTL, then teardown all of this. There is no other way. This sounds like a roundabout way to answer the simple question "is the IOCTL available?". Do you have another suggestion to address this problem? What does userspace do differently if the ioctl is present or not? Globally enable a synchronization API for Wayland clients, for instance in the case of a Wayland compositor. And why is this somehow more special than of the tens of thousands of other ioctl calls where you have to do exactly the same thing you list above to determine if it is present or not? For other IOCTLs it's not as complicated to obtain a FD to do the test with. Two expand on this: - compositor opens the drm render /dev node - compositor initializes the opengl or vulkan userspace driver on top of that - compositor asks that userspace driver to allocate some buffer, which can be pretty expensive - compositor asks the userspace driver to export that buffer into a dma-buf - compositor can finally do the test ioctl, realizes support isn't there and tosses the entire thing read() on a sysfs file is so much more reasonable it's not even funny. Just a drive-by observation, so apologies if I'm overlooking something obvious, but it sounds like the ideal compromise would be to expose a sysfs file which behaves as a dummy exported dma-buf. That way userspace could just open() it and try ioctl() directly - assuming that supported operations can fail distinctly from unsupported ones, or succeed as a no-op - which seems even simpler still. ioctl() will not work on a sysfs file, sorry. Ah, fair enough - TBH I should have just said "a file", since I presume some sort of /dev/dma-buf might also be an option, if a bit more work to implement. I'll scuttle back to my low-level DMA corner now :) Cheers, Robin.
Re: [PATCH 2/2] arm64: dts: mt8183: Add panel rotation
On Mon, May 30, 2022 at 7:30 PM Hsin-Yi Wang wrote:
>
> krane, kakadu, and kodama boards have a default panel rotation.
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Enric Balletbo i Serra
> Tested-by: Enric Balletbo i Serra
> ---
Hi Matthias,
The series ("Add a panel API to return panel orientation") might land
in drm-misc. With this series applied, we can add this patch to give
the correct default orientation for mt8183 kukui devices.
I didn't send this patch again with the series, since they might land
in different trees.
Thanks.
> arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index 8d5bf73a9099..f0dd5a06629d 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -276,6 +276,7 @@ panel: panel@0 {
> avee-supply = <_lcd>;
> pp1800-supply = <_lcd>;
> backlight = <_lcd0>;
> + rotation = <270>;
> port {
> panel_in: endpoint {
> remote-endpoint = <_out>;
> --
> 2.36.1.124.g0e6072fb45-goog
>
Re: [PATCH v3 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
On Mon, Jun 6, 2022 at 10:21 PM Doug Anderson wrote:
>
> Hi,
>
> On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote:
> >
> > Panels usually call drm_connector_set_panel_orientation(), which is
> > later than drm/kms driver calling drm_dev_register(). This leads to a
> > WARN().
> >
> > The orientation property is known earlier. For example, some panels
> > parse the property through device tree during probe.
> >
> > Add an API to return the property from panel to drm/kms driver, so the
> > drivers are able to call drm_connector_set_panel_orientation() before
> > drm_dev_register().
> >
> > Suggested-by: Hans de Goede
> > Signed-off-by: Hsin-Yi Wang
> > Reviewed-by: Hans de Goede
> > ---
> > v2->v3: no change
> > ---
> > drivers/gpu/drm/drm_panel.c | 8
> > include/drm/drm_panel.h | 10 ++
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> > index f634371c717a..4a512ca80673 100644
> > --- a/drivers/gpu/drm/drm_panel.c
> > +++ b/drivers/gpu/drm/drm_panel.c
> > @@ -223,6 +223,14 @@ int drm_panel_get_modes(struct drm_panel *panel,
> > }
> > EXPORT_SYMBOL(drm_panel_get_modes);
> >
> > +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel
> > *panel)
> > +{
> > + if (panel && panel->funcs && panel->funcs->get_orientation)
> > + return panel->funcs->get_orientation(panel);
> > +
> > + return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > +}
> > +EXPORT_SYMBOL(drm_panel_get_orientation);
> > #ifdef CONFIG_OF
>
> nit: there used to be a blank line before the #ifdef but there no
> longer is after your patch.
>
Added in v4.
> Other than that...
>
> Reviewed-by: Douglas Anderson
Re: [PATCH v3 4/8] drm/panel: lvds: Implement .get_orientation callback
On Mon, Jun 6, 2022 at 10:27 PM Doug Anderson wrote:
>
> Hi,
>
> On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote:
> >
> > To return the orientation property to drm/kms driver.
> >
> > Signed-off-by: Hsin-Yi Wang
> > Reviewed-by: Hans de Goede
> > ---
> > v2->v3: add comments for notice.
> > ---
> > drivers/gpu/drm/panel/panel-lvds.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> > b/drivers/gpu/drm/panel/panel-lvds.c
> > index 27a1c9923b09..239527409b00 100644
> > --- a/drivers/gpu/drm/panel/panel-lvds.c
> > +++ b/drivers/gpu/drm/panel/panel-lvds.c
> > @@ -102,15 +102,29 @@ static int panel_lvds_get_modes(struct drm_panel
> > *panel,
> > connector->display_info.bus_flags = lvds->data_mirror
> > ? DRM_BUS_FLAG_DATA_LSB_TO_MSB
> > : DRM_BUS_FLAG_DATA_MSB_TO_LSB;
>
> Can you rebase your patch and send again? There's a context conflict
> with the above line because your tree is lacking commit 83c784e70036
> ("drm/panel: lvds: Use bus_flags from DT panel-timing property")
>
Rebased in v4.
> In any case:
>
> Reviewed-by: Douglas Anderson
[PATCH v4 8/8] drm/mediatek: Config orientation property if panel provides it
Panel orientation property should be set before drm_dev_register().
Mediatek drm driver calls drm_dev_register() in .bind(). However, most
panels sets orientation property relatively late, mostly in .get_modes()
callback, since this is when they are able to get the connector and
binds the orientation property to it, though the value should be known
when the panel is probed.
Let the drm driver check if the remote end point is a panel and if it
contains the orientation property. If it does, set it before
drm_dev_register() is called.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index d9f10a33e6fa..c56282412bfa 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -185,6 +185,7 @@ struct mtk_dsi {
struct drm_encoder encoder;
struct drm_bridge bridge;
struct drm_bridge *next_bridge;
+ struct drm_panel *panel;
struct drm_connector *connector;
struct phy *phy;
@@ -822,6 +823,12 @@ static int mtk_dsi_encoder_init(struct drm_device *drm,
struct mtk_dsi *dsi)
ret = PTR_ERR(dsi->connector);
goto err_cleanup_encoder;
}
+
+ /* Read panel orientation */
+ if (dsi->panel)
+ drm_connector_set_panel_orientation(dsi->connector,
+ drm_panel_get_orientation(dsi->panel));
+
drm_connector_attach_encoder(dsi->connector, >encoder);
return 0;
@@ -837,6 +844,9 @@ static int mtk_dsi_bind(struct device *dev, struct device
*master, void *data)
struct drm_device *drm = data;
struct mtk_dsi *dsi = dev_get_drvdata(dev);
+ /* Get panel if existed */
+ drm_of_find_panel_or_bridge(dev->of_node, 0, 0, >panel, NULL);
+
ret = mtk_dsi_encoder_init(drm, dsi);
if (ret)
return ret;
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 7/8] drm/panel: elida-kd35t133: Implement .get_orientation callback
To return the orientation property to drm/kms driver.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
drivers/gpu/drm/panel/panel-elida-kd35t133.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-elida-kd35t133.c
b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
index 80227617a4d6..fa85a288afdc 100644
--- a/drivers/gpu/drm/panel/panel-elida-kd35t133.c
+++ b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
@@ -217,15 +217,29 @@ static int kd35t133_get_modes(struct drm_panel *panel,
connector->display_info.width_mm = mode->width_mm;
connector->display_info.height_mm = mode->height_mm;
drm_mode_probed_add(connector, mode);
+ /*
+* drm drivers are expected to call drm_panel_get_orientation() to get
+* panel's orientation then drm_connector_set_panel_orientation() to
+* set the property before drm_dev_register(). Otherwise there will be
+* a WARN_ON if orientation is set after drm is registered.
+*/
drm_connector_set_panel_orientation(connector, ctx->orientation);
return 1;
}
+static enum drm_panel_orientation kd35t133_get_orientation(struct drm_panel
*panel)
+{
+ struct kd35t133 *ctx = panel_to_kd35t133(panel);
+
+ return ctx->orientation;
+}
+
static const struct drm_panel_funcs kd35t133_funcs = {
.unprepare = kd35t133_unprepare,
.prepare= kd35t133_prepare,
.get_modes = kd35t133_get_modes,
+ .get_orientation = kd35t133_get_orientation,
};
static int kd35t133_probe(struct mipi_dsi_device *dsi)
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 6/8] drm/panel: ili9881c: Implement .get_orientation callback
To return the orientation property to drm/kms driver.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index ba30d11547ad..c098a0ed6be7 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -853,17 +853,31 @@ static int ili9881c_get_modes(struct drm_panel *panel,
connector->display_info.width_mm = mode->width_mm;
connector->display_info.height_mm = mode->height_mm;
+ /*
+* drm drivers are expected to call drm_panel_get_orientation() to get
+* panel's orientation then drm_connector_set_panel_orientation() to
+* set the property before drm_dev_register(). Otherwise there will be
+* a WARN_ON if orientation is set after drm is registered.
+*/
drm_connector_set_panel_orientation(connector, ctx->orientation);
return 1;
}
+static enum drm_panel_orientation ili9881c_get_orientation(struct drm_panel
*panel)
+{
+ struct ili9881c *ctx = panel_to_ili9881c(panel);
+
+ return ctx->orientation;
+}
+
static const struct drm_panel_funcs ili9881c_funcs = {
.prepare= ili9881c_prepare,
.unprepare = ili9881c_unprepare,
.enable = ili9881c_enable,
.disable= ili9881c_disable,
.get_modes = ili9881c_get_modes,
+ .get_orientation = ili9881c_get_orientation,
};
static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 5/8] drm/panel: panel-simple: Implement .get_orientation callback
To return the orientation property to drm/kms driver.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
drivers/gpu/drm/panel/panel-simple.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 4a2e580a2f7b..f232b8cf4075 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -411,7 +411,12 @@ static int panel_simple_get_modes(struct drm_panel *panel,
/* add hard-coded panel modes */
num += panel_simple_get_non_edid_modes(p, connector);
- /* set up connector's "panel orientation" property */
+ /*
+* drm drivers are expected to call drm_panel_get_orientation() to get
+* panel's orientation then drm_connector_set_panel_orientation() to
+* set the property before drm_dev_register(). Otherwise there will be
+* a WARN_ON if orientation is set after drm is registered.
+*/
drm_connector_set_panel_orientation(connector, p->orientation);
return num;
@@ -434,6 +439,14 @@ static int panel_simple_get_timings(struct drm_panel
*panel,
return p->desc->num_timings;
}
+static enum drm_panel_orientation panel_simple_get_orientation(struct
drm_panel *panel)
+{
+ struct panel_simple *p = to_panel_simple(panel);
+
+ return p->orientation;
+}
+
+
static const struct drm_panel_funcs panel_simple_funcs = {
.disable = panel_simple_disable,
.unprepare = panel_simple_unprepare,
@@ -441,6 +454,7 @@ static const struct drm_panel_funcs panel_simple_funcs = {
.enable = panel_simple_enable,
.get_modes = panel_simple_get_modes,
.get_timings = panel_simple_get_timings,
+ .get_orientation = panel_simple_get_orientation,
};
static struct panel_desc panel_dpi;
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 4/8] drm/panel: lvds: Implement .get_orientation callback
To return the orientation property to drm/kms driver.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
v3->v4: rebase to latest linux-next to solve conflict.
---
drivers/gpu/drm/panel/panel-lvds.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-lvds.c
b/drivers/gpu/drm/panel/panel-lvds.c
index f11252fb00fe..491b64c2c8d6 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -99,15 +99,30 @@ static int panel_lvds_get_modes(struct drm_panel *panel,
drm_display_info_set_bus_formats(>display_info,
>bus_format, 1);
connector->display_info.bus_flags = lvds->bus_flags;
+
+ /*
+* drm drivers are expected to call drm_panel_get_orientation() to get
+* panel's orientation then drm_connector_set_panel_orientation() to
+* set the property before drm_dev_register(). Otherwise there will be
+* a WARN_ON if orientation is set after drm is registered.
+*/
drm_connector_set_panel_orientation(connector, lvds->orientation);
return 1;
}
+static enum drm_panel_orientation panel_lvds_get_orientation,(struct drm_panel
*panel)
+{
+ struct panel_lvds *lvds = to_panel_lvds(panel);
+
+ return lvds->orientation;
+}
+
static const struct drm_panel_funcs panel_lvds_funcs = {
.unprepare = panel_lvds_unprepare,
.prepare = panel_lvds_prepare,
.get_modes = panel_lvds_get_modes,
+ .get_orientation = panel_lvds_get_orientation,
};
static int panel_lvds_parse_dt(struct panel_lvds *lvds)
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 3/8] drm/panel: panel-edp: Implement .get_orientation callback
To return the orientation property to drm/kms driver.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
drivers/gpu/drm/panel/panel-edp.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index c96014464355..c0a43bc7d24a 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -586,7 +586,12 @@ static int panel_edp_get_modes(struct drm_panel *panel,
else if (!num)
dev_warn(p->base.dev, "No display modes\n");
- /* set up connector's "panel orientation" property */
+ /*
+* drm drivers are expected to call drm_panel_get_orientation() to get
+* panel's orientation then drm_connector_set_panel_orientation() to
+* set the property before drm_dev_register(). Otherwise there will be
+* a WARN_ON if orientation is set after drm is registered.
+*/
drm_connector_set_panel_orientation(connector, p->orientation);
return num;
@@ -609,6 +614,13 @@ static int panel_edp_get_timings(struct drm_panel *panel,
return p->desc->num_timings;
}
+static enum drm_panel_orientation panel_edp_get_orientation(struct drm_panel
*panel)
+{
+ struct panel_edp *p = to_panel_edp(panel);
+
+ return p->orientation;
+}
+
static int detected_panel_show(struct seq_file *s, void *data)
{
struct drm_panel *panel = s->private;
@@ -637,6 +649,7 @@ static const struct drm_panel_funcs panel_edp_funcs = {
.prepare = panel_edp_prepare,
.enable = panel_edp_enable,
.get_modes = panel_edp_get_modes,
+ .get_orientation = panel_edp_get_orientation,
.get_timings = panel_edp_get_timings,
.debugfs_init = panel_edp_debugfs_init,
};
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 2/8] drm/panel: boe-tv101wum-nl6: Implement .get_orientation callback
To return the orientation property to drm/kms driver.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 1be150ac758f..a9cd07234179 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1511,16 +1511,30 @@ static int boe_panel_get_modes(struct drm_panel *panel,
connector->display_info.width_mm = boe->desc->size.width_mm;
connector->display_info.height_mm = boe->desc->size.height_mm;
connector->display_info.bpc = boe->desc->bpc;
+ /*
+* drm drivers are expected to call drm_panel_get_orientation() to get
+* panel's orientation then drm_connector_set_panel_orientation() to
+* set the property before drm_dev_register(). Otherwise there will be
+* a WARN_ON if orientation is set after drm is registered.
+*/
drm_connector_set_panel_orientation(connector, boe->orientation);
return 1;
}
+static enum drm_panel_orientation boe_panel_get_orientation(struct drm_panel
*panel)
+{
+ struct boe_panel *boe = to_boe_panel(panel);
+
+ return boe->orientation;
+}
+
static const struct drm_panel_funcs boe_panel_funcs = {
.unprepare = boe_panel_unprepare,
.prepare = boe_panel_prepare,
.enable = boe_panel_enable,
.get_modes = boe_panel_get_modes,
+ .get_orientation = boe_panel_get_orientation,
};
static int boe_panel_add(struct boe_panel *boe)
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Panels usually call drm_connector_set_panel_orientation(), which is
later than drm/kms driver calling drm_dev_register(). This leads to a
WARN().
The orientation property is known earlier. For example, some panels
parse the property through device tree during probe.
Add an API to return the property from panel to drm/kms driver, so the
drivers are able to call drm_connector_set_panel_orientation() before
drm_dev_register().
Suggested-by: Hans de Goede
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Hans de Goede
Reviewed-by: Douglas Anderson
---
v3->v4: Add a blank line.
---
drivers/gpu/drm/drm_panel.c | 9 +
include/drm/drm_panel.h | 10 ++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371c717a..e12056cfeca8 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -223,6 +223,15 @@ int drm_panel_get_modes(struct drm_panel *panel,
}
EXPORT_SYMBOL(drm_panel_get_modes);
+enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel)
+{
+ if (panel && panel->funcs && panel->funcs->get_orientation)
+ return panel->funcs->get_orientation(panel);
+
+ return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+}
+EXPORT_SYMBOL(drm_panel_get_orientation);
+
#ifdef CONFIG_OF
/**
* of_drm_find_panel - look up a panel using a device tree node
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index d279ee455f01..5dadbf3b0370 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -133,6 +133,15 @@ struct drm_panel_funcs {
* Allows panels to create panels-specific debugfs files.
*/
void (*debugfs_init)(struct drm_panel *panel, struct dentry *root);
+
+ /**
+* @get_orientation:
+*
+* Return the panel orientation set by device tree or EDID.
+*
+* This function is optional.
+*/
+ enum drm_panel_orientation (*get_orientation)(struct drm_panel *panel);
};
/**
@@ -202,6 +211,7 @@ int drm_panel_enable(struct drm_panel *panel);
int drm_panel_disable(struct drm_panel *panel);
int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector
*connector);
+enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel);
#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
struct drm_panel *of_drm_find_panel(const struct device_node *np);
--
2.36.1.255.ge46751e96f-goog
[PATCH v4 0/8] Add a panel API to return panel orientation
Panels usually call drm_connector_set_panel_orientation(), which is later than drm/kms driver calling drm_dev_register(). This leads to a WARN()[1]. The orientation property is known earlier. For example, some panels parse the property through device tree during probe. The series add a panel API drm_panel_get_orientation() for drm/kms drivers. The drivers can use the API to get panel's orientation, so they can call drm_connector_set_panel_orientation() before drm_dev_register(). Panel needs to implement .get_orientation callback to return the property. [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20220530081910.3947168-2-hsi...@chromium.org/ Hsin-Yi Wang (8): drm/panel: Add an API drm_panel_get_orientation() to return panel orientation drm/panel: boe-tv101wum-nl6: Implement .get_orientation callback drm/panel: panel-edp: Implement .get_orientation callback drm/panel: lvds: Implement .get_orientation callback drm/panel: panel-simple: Implement .get_orientation callback drm/panel: ili9881c: Implement .get_orientation callback drm/panel: elida-kd35t133: Implement .get_orientation callback drm/mediatek: Config orientation property if panel provides it drivers/gpu/drm/drm_panel.c| 9 + drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++ drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 14 ++ drivers/gpu/drm/panel/panel-edp.c | 15 ++- drivers/gpu/drm/panel/panel-elida-kd35t133.c | 14 ++ drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++ drivers/gpu/drm/panel/panel-lvds.c | 15 +++ drivers/gpu/drm/panel/panel-simple.c | 16 +++- include/drm/drm_panel.h| 10 ++ 9 files changed, 115 insertions(+), 2 deletions(-) -- 2.36.1.255.ge46751e96f-goog
Re: [PATCH v4] dma-buf: Add a capabilities directory
On Mon, Jun 06, 2022 at 04:10:09PM +0100, Robin Murphy wrote: > On 2022-06-02 07:47, Daniel Vetter wrote: > > On Thu, 2 Jun 2022 at 08:34, Simon Ser wrote: > > > > > > On Thursday, June 2nd, 2022 at 08:25, Greg KH wrote: > > > > > > > On Thu, Jun 02, 2022 at 06:17:31AM +, Simon Ser wrote: > > > > > > > > > On Thursday, June 2nd, 2022 at 07:40, Greg KH g...@kroah.com wrote: > > > > > > > > > > > On Wed, Jun 01, 2022 at 04:13:14PM +, Simon Ser wrote: > > > > > > > > > > > > > To discover support for new DMA-BUF IOCTLs, user-space has no > > > > > > > choice but to try to perform the IOCTL on an existing DMA-BUF. > > > > > > > > > > > > Which is correct and how all kernel features work (sorry I missed > > > > > > the > > > > > > main goal of this patch earlier and focused only on the sysfs > > > > > > stuff). > > > > > > > > > > > > > However, user-space may want to figure out whether or not the > > > > > > > IOCTL is available before it has a DMA-BUF at hand, e.g. at > > > > > > > initialization time in a Wayland compositor. > > > > > > > > > > > > Why not just do the ioctl in a test way? That's how we determine > > > > > > kernel > > > > > > features, we do not poke around in sysfs to determine what is, or is > > > > > > not, present at runtime. > > > > > > > > > > > > > Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF > > > > > > > subsystem to advertise supported features. Add a > > > > > > > sync_file_import_export entry which indicates that importing and > > > > > > > exporting sync_files from/to DMA-BUFs is supported. > > > > > > > > > > > > No, sorry, this is not a sustainable thing to do for all kernel > > > > > > features > > > > > > over time. Please just do the ioctl and go from there. sysfs is not > > > > > > for advertising what is and is not enabled/present in a kernel with > > > > > > regards to functionality or capabilities of the system. > > > > > > > > > > > > If sysfs were to export this type of thing, it would have to do it > > > > > > for > > > > > > everything, not just some random tiny thing of one kernel driver. > > > > > > > > > > I'd argue that DMA-BUF is a special case here. > > > > > > > > So this is special and unique just like everything else? :) > > > > > > > > > To check whether the import/export IOCTLs are available, user-space > > > > > needs a DMA-BUF to try to perform the IOCTL. To get a DMA-BUF, > > > > > user-space needs to enumerate GPUs, pick one at random, load GBM or > > > > > Vulkan, use that heavy-weight API to allocate a "fake" buffer on the > > > > > GPU, export that buffer into a DMA-BUF, try the IOCTL, then teardown > > > > > all of this. There is no other way. > > > > > > > > > > This sounds like a roundabout way to answer the simple question "is > > > > > the > > > > > IOCTL available?". Do you have another suggestion to address this > > > > > problem? > > > > > > > > What does userspace do differently if the ioctl is present or not? > > > > > > Globally enable a synchronization API for Wayland clients, for instance > > > in the case of a Wayland compositor. > > > > > > > And why is this somehow more special than of the tens of thousands of > > > > other ioctl calls where you have to do exactly the same thing you list > > > > above to determine if it is present or not? > > > > > > For other IOCTLs it's not as complicated to obtain a FD to do the test > > > with. > > > > Two expand on this: > > > > - compositor opens the drm render /dev node > > - compositor initializes the opengl or vulkan userspace driver on top of > > that > > - compositor asks that userspace driver to allocate some buffer, which > > can be pretty expensive > > - compositor asks the userspace driver to export that buffer into a dma-buf > > - compositor can finally do the test ioctl, realizes support isn't > > there and tosses the entire thing > > > > read() on a sysfs file is so much more reasonable it's not even funny. > > Just a drive-by observation, so apologies if I'm overlooking something > obvious, but it sounds like the ideal compromise would be to expose a sysfs > file which behaves as a dummy exported dma-buf. That way userspace could > just open() it and try ioctl() directly - assuming that supported operations > can fail distinctly from unsupported ones, or succeed as a no-op - which > seems even simpler still. ioctl() will not work on a sysfs file, sorry.
Re: [PATCH] drm/i915/dg2: Catch and log more unexpected values in DG1_MSTR_TILE_INTR
On Mon, Jun 06, 2022 at 12:55:20PM +0100, Tvrtko Ursulin wrote:
>
> On 27/05/2022 19:42, Matt Roper wrote:
> > On Thu, May 26, 2022 at 11:18:17AM +0100, Tvrtko Ursulin wrote:
> > > On 25/05/2022 19:05, Matt Roper wrote:
> > > > On Wed, May 25, 2022 at 05:03:13PM +0100, Tvrtko Ursulin wrote:
> > > > >
> > > > > On 24/05/2022 18:51, Matt Roper wrote:
> > > > > > On Tue, May 24, 2022 at 10:43:39AM +0100, Tvrtko Ursulin wrote:
> > > > > > > From: Tvrtko Ursulin
> > > > > > >
> > > > > > > Catch and log any garbage in the register, including no tiles
> > > > > > > marked, or
> > > > > > > multiple tiles marked.
> > > > > > >
> > > > > > > Signed-off-by: Tvrtko Ursulin
> > > > > > > Cc: Matt Roper
> > > > > > > ---
> > > > > > > We caught garbage in DG1_MSTR_TILE_INTR with DG2 (actual value
> > > > > > > 0xF9D2C008)
> > > > > > > during glmark and more badness. So I thought lets log all
> > > > > > > possible failure
> > > > > > > modes from here and also use per device logging.
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/i915_irq.c | 33
> > > > > > > ++---
> > > > > > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > > > > > 2 files changed, 23 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > > > > b/drivers/gpu/drm/i915/i915_irq.c
> > > > > > > index 73cebc6aa650..79853d3fc1ed 100644
> > > > > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > > > > @@ -2778,24 +2778,30 @@ static irqreturn_t dg1_irq_handler(int
> > > > > > > irq, void *arg)
> > > > > > > u32 gu_misc_iir;
> > > > > > > if (!intel_irqs_enabled(i915))
> > > > > > > - return IRQ_NONE;
> > > > > > > + goto none;
> > > > > > > master_tile_ctl = dg1_master_intr_disable(regs);
> > > > > > > - if (!master_tile_ctl) {
> > > > > > > - dg1_master_intr_enable(regs);
> > > > > > > - return IRQ_NONE;
> > > > > > > + if (!master_tile_ctl)
> > > > > > > + goto enable_none;
> > > > > > > +
> > > > > > > + if (master_tile_ctl & ~(DG1_MSTR_IRQ | DG1_MSTR_TILE_MASK)) {
> > > > > > > + drm_warn(>drm, "Garbage in master_tile_ctl:
> > > > > > > 0x%08x!\n",
> > > > > > > + master_tile_ctl);
> > > > > >
> > > > > > I know we have a bunch of them already, but shouldn't we be avoiding
> > > > > > printk-based stuff like this inside interrupt handlers? Should we
> > > > > > be
> > > > > > migrating all these error messages over to trace_printk or something
> > > > > > similar that's safer to use?
> > > > >
> > > > > Not sure - I kind of think some really unexpected and worrying
> > > > > situations
> > > > > should be loud and on by default. Risk is then spam if not
> > > > > ratelimited.
> > > > > Maybe we should instead ratelimit most errors/warnings coming for irq
> > > > > handlers?
> > > >
> > > > It's not the risk of spam that's the problem, but rather that
> > > > printk-based stuff eventually calls into the console code to flush its
> > > > buffers. That's way more overhead than you want in an interrupt handler
> > > > so it's bad on its own, but if you're using something slow like a serial
> > > > console, it becomes even more of a problem.
> > >
> > > Is it a problem for messages which we never expect to see?
> >
> > Kind of. While not as catastrophic, it's the same argument for why we
> > don't use BUG() anymore...when the impossible does manage to happen
> > there's unnecessary collateral damage on things outside of graphics. If
> > we're adding huge delays inside an interrupt handler (while other
> > interrupts are disabled) that impacts the system-wide usability, not
> > just our own driver.
> >
> > I'd also argue that these messages actually are semi-expected. Random
> > bits being set shouldn't happen, but in the world of dgpu's, we do
> > occasionally see cases where the PCI link itself goes down for reasons
> > outside our control and then all registers read back as 0x,
> > which will probably trigger error messages here (as well as a bunch of
> > other places).
>
> Could you expand a bit on what is semi-expected and when? I mean the
> circumstances of PCI link going down. We certainly don't have any code to
> survive that.
Yeah, I'm referring to the "Lost access to MMIO BAR" errors; in the past
most of them have ultimately been tracked down to bugs in early
firmware, so flashing an updated IFWI/BIOS onto the device usually
solved the problems. Generally those buggy firmwares are an internal
problem that never make it into the wild, but I think we have also seen
cases where they get triggered by physical/electrical problems on a
specific part; that can potentially happen to anyone who's unlucky
enough to get a defective/damaged unit.
Basically "hardware returns all F's" happens because the CPU initiates
an MMIO transaction with the hardware, the hardware fails to
Re: [PATCH v4] dma-buf: Add a capabilities directory
On 2022-06-02 07:47, Daniel Vetter wrote: On Thu, 2 Jun 2022 at 08:34, Simon Ser wrote: On Thursday, June 2nd, 2022 at 08:25, Greg KH wrote: On Thu, Jun 02, 2022 at 06:17:31AM +, Simon Ser wrote: On Thursday, June 2nd, 2022 at 07:40, Greg KH g...@kroah.com wrote: On Wed, Jun 01, 2022 at 04:13:14PM +, Simon Ser wrote: To discover support for new DMA-BUF IOCTLs, user-space has no choice but to try to perform the IOCTL on an existing DMA-BUF. Which is correct and how all kernel features work (sorry I missed the main goal of this patch earlier and focused only on the sysfs stuff). However, user-space may want to figure out whether or not the IOCTL is available before it has a DMA-BUF at hand, e.g. at initialization time in a Wayland compositor. Why not just do the ioctl in a test way? That's how we determine kernel features, we do not poke around in sysfs to determine what is, or is not, present at runtime. Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF subsystem to advertise supported features. Add a sync_file_import_export entry which indicates that importing and exporting sync_files from/to DMA-BUFs is supported. No, sorry, this is not a sustainable thing to do for all kernel features over time. Please just do the ioctl and go from there. sysfs is not for advertising what is and is not enabled/present in a kernel with regards to functionality or capabilities of the system. If sysfs were to export this type of thing, it would have to do it for everything, not just some random tiny thing of one kernel driver. I'd argue that DMA-BUF is a special case here. So this is special and unique just like everything else? :) To check whether the import/export IOCTLs are available, user-space needs a DMA-BUF to try to perform the IOCTL. To get a DMA-BUF, user-space needs to enumerate GPUs, pick one at random, load GBM or Vulkan, use that heavy-weight API to allocate a "fake" buffer on the GPU, export that buffer into a DMA-BUF, try the IOCTL, then teardown all of this. There is no other way. This sounds like a roundabout way to answer the simple question "is the IOCTL available?". Do you have another suggestion to address this problem? What does userspace do differently if the ioctl is present or not? Globally enable a synchronization API for Wayland clients, for instance in the case of a Wayland compositor. And why is this somehow more special than of the tens of thousands of other ioctl calls where you have to do exactly the same thing you list above to determine if it is present or not? For other IOCTLs it's not as complicated to obtain a FD to do the test with. Two expand on this: - compositor opens the drm render /dev node - compositor initializes the opengl or vulkan userspace driver on top of that - compositor asks that userspace driver to allocate some buffer, which can be pretty expensive - compositor asks the userspace driver to export that buffer into a dma-buf - compositor can finally do the test ioctl, realizes support isn't there and tosses the entire thing read() on a sysfs file is so much more reasonable it's not even funny. Just a drive-by observation, so apologies if I'm overlooking something obvious, but it sounds like the ideal compromise would be to expose a sysfs file which behaves as a dummy exported dma-buf. That way userspace could just open() it and try ioctl() directly - assuming that supported operations can fail distinctly from unsupported ones, or succeed as a no-op - which seems even simpler still. Robin.
Re: [RFC PATCH v2 00/27] DRM.debug on DYNAMIC_DEBUG, add trace events
On Wed, May 25, 2022 at 9:02 AM Daniel Vetter wrote: > On Mon, May 16, 2022 at 04:56:13PM -0600, Jim Cromie wrote: > > DRM.debug API is 23 macros, issuing 10 exclusive categories of debug > > messages. By rough count, they are used 5140 times in the kernel. > > These all call drm_dbg or drm_devdbg, which call drm_debug_enabled(), > > which checks bits in global __drm_debug. Some of these are page-flips > > and vblanks, and get called often. > > > > DYNAMIC_DEBUG (with CONFIG_JUMP_LABEL) is built to avoid this kind of > > work, with NOOPd jump/callsites. > > > > This patchset is RFC because: > > - it touches 2.5 subsystems: dyndbg, drm, tracefs (new events) > > - dyndbg class support is built for drm, needs it for validation > > - new api, used by drm > > - big memory impact, with 5100 new pr-debug callsites. > > - drm class bikeshedding opportunities > > - others, names etc. > > Thanks a lot for keeping on pushing this! > > > > DYNAMIC_DEBUG: > > > > RFC: > > > > dynamic_debug_register_classes() cannot act early enough to be in > > effect at module-load. So this will not work as you'd reasonably > > expect: > > > > modprobe test_dynamic_debug dyndbg='+pfm; class FOO +pfmlt' > > > > The 1st query:+pfm will be enabled during load, but in the 2nd query, > > "class FOO" will be unknown at load time. Early class enablement > > would be nice. DYNAMIC_DEBUG_CLASSES is a static initializer, which > > is certainly early enough, but Im missing a trick, suggestions? > > So maybe I'm just totally overloading this work here so feel free to > ignore or postpone, but: Could we do the dynamic_debug_register_classes() > automatically at module load as a new special section? And then throw in a > bit of kbuild so that in a given subsystem every driver gets the same > class names by default and everything would just work, without having to > sprinkle calls to dynamic_debug_register_classes() all over the place? > This is now done; Ive added __dyndbg_classes section. load_module() now grabs it from the .ko and ddebug_add_module() attaches it to the module's ddebug_table record. for builtins, dynamic_debug_init feeds the builtin class-maps to ddebug_add_module bash-5.1# modprobe test_dynamic_debug dyndbg="class FOO +p" [ 88.374722] dyndbg: class[0]: nm:test_dynamic_debug base:20 len:7 ty:1 [ 88.375158] dyndbg: 0: EMERG [ 88.375345] dyndbg: 1: DANGER [ 88.375540] dyndbg: 2: ERROR [ 88.375726] dyndbg: 3: WARNING [ 88.375930] dyndbg: 4: NOTICE [ 88.376130] dyndbg: 5: INFO [ 88.376310] dyndbg: 6: DEBUG [ 88.376499] dyndbg: class[1]: nm:test_dynamic_debug base:12 len:3 ty:1 [ 88.376903] dyndbg: 0: ONE [ 88.377079] dyndbg: 1: TWO [ 88.377253] dyndbg: 2: THREE [ 88.377441] dyndbg: class[2]: nm:test_dynamic_debug base:8 len:3 ty:0 [ 88.377837] dyndbg: 0: bing [ 88.378022] dyndbg: 1: bong [ 88.378203] dyndbg: 2: boom [ 88.378387] dyndbg: class[3]: nm:test_dynamic_debug base:4 len:3 ty:0 [ 88.378800] dyndbg: 0: Foo [ 88.378986] dyndbg: 1: Bar [ 88.379167] dyndbg: 2: Buzz [ 88.379348] dyndbg: class[4]: nm:test_dynamic_debug base:0 len:3 ty:0 [ 88.379757] dyndbg: 0: FOO [ 88.379938] dyndbg: 1: BAR [ 88.380136] dyndbg: 2: BUZZ [ 88.380410] dyndbg: module:test_dynamic_debug attached 5 classes [ 88.380881] dyndbg: 24 debug prints in module test_dynamic_debug [ 88.381315] dyndbg: module: test_dynamic_debug dyndbg="class FOO +p" [ 88.381714] dyndbg: query 0: "class FOO +p" mod:test_dynamic_debug [ 88.382109] dyndbg: split into words: "class" "FOO" "+p" [ 88.382445] dyndbg: op='+' [ 88.382616] dyndbg: flags=0x1 [ 88.382802] dyndbg: *flagsp=0x1 *maskp=0x [ 88.383101] dyndbg: parsed: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=FOO [ 88.383740] dyndbg: applied: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=FOO [ 88.384324] dyndbg: processed 1 queries, with 2 matches, 0 errs bash-5.1# so its working at module-load time. > For the entire class approach, did you spot another subsystem that could > benefit from this and maybe make a more solid case that this is something > good? > I had been working on the premise that ~4k drm*dbg callsites was a good case. verbosity-levels - with x /sys/module/foo/parameters/debug_verbosity and effectively does echo < /proc/dynamic_debug/control module foo class V1 +p module foo class V2 +p module foo class V3 +p module foo class V4 -p module foo class V5 -p module foo class V6 -p module foo class V7 -p module foo class V8 -p EOQRY since only real +/-p changes incur kernel-patching costs, the remaining overheads are minimal. > RFC for DRM: > > - decoration flags "fmlt" do not work on drm_*dbg(). > (drm_*dbg() dont use pr_debug, they *become* one flavor of them) > this could (should?) be added, and maybe tailored for drm. > some of the device prefixes are very long, a "d" flag could optionalize them. I'm lost what the fmlt decoration
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hello Daniel, On 6/6/22 16:19, Daniel Latypov wrote: [snip] >> That's what I asked in the previous RFC too. Daniel mentioned that it >> shouldn't >> go there because is platform specific (AFAIU, one might want to test it on >> x86, > > Slight correction, it was David who explicitly suggested it shouldn't > go in there. > https://lists.freedesktop.org/archives/dri-devel/2022-June/357611.html > Ups, sorry for getting that wrong. I should had looked at the thread again before writing my answer. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v3 6/8] drm/panel: ili9881c: Implement .get_orientation callback
Hi, On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote: > > To return the orientation property to drm/kms driver. > > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Hans de Goede > --- > v2->v3: add comments for notice. > --- > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Douglas Anderson
Re: [PATCH v3 7/8] drm/panel: elida-kd35t133: Implement .get_orientation callback
Hi, On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote: > > To return the orientation property to drm/kms driver. > > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Hans de Goede > --- > v2->v3: add comments for notice. > --- > drivers/gpu/drm/panel/panel-elida-kd35t133.c | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Douglas Anderson
Re: [RESEND 12/14] leds: mt6370: Add Mediatek MT6370 Indicator support
Hi ChiaEn,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pavel-leds/for-next]
[also build test ERROR on lee-mfd/for-mfd-next lee-backlight/for-backlight-next
v5.19-rc1 next-20220606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/ChiaEn-Wu/Add-Mediatek-MT6370-PMIC-support/20220531-211432
base: git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git
for-next
config: nios2-allyesconfig
(https://download.01.org/0day-ci/archive/20220606/202206062207.q0s8wxpj-...@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.3.0
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
#
https://github.com/intel-lab-lkp/linux/commit/4de9e5ff11aeade155aa744bcaf9efca82b3d4ee
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
ChiaEn-Wu/Add-Mediatek-MT6370-PMIC-support/20220531-211432
git checkout 4de9e5ff11aeade155aa744bcaf9efca82b3d4ee
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1
O=build_dir ARCH=nios2 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/leds/leds-mt6370.c: In function 'mt6370_check_vendor_info':
>> drivers/leds/leds-mt6370.c:873:15: error: implicit declaration of function
>> 'FIELD_GET'; did you mean 'FOLL_GET'? [-Werror=implicit-function-declaration]
873 | vid = FIELD_GET(MT6370_VENID_MASK, devinfo);
| ^
| FOLL_GET
cc1: some warnings being treated as errors
vim +873 drivers/leds/leds-mt6370.c
863
864 static int mt6370_check_vendor_info(struct mt6370_priv *priv)
865 {
866 unsigned int devinfo, vid;
867 int ret;
868
869 ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, );
870 if (ret)
871 return ret;
872
> 873 vid = FIELD_GET(MT6370_VENID_MASK, devinfo);
874 if (vid == 0x9 || vid == 0xb) {
875 priv->reg_fields = mt6372_reg_fields;
876 priv->ranges = mt6372_led_ranges;
877 priv->is_mt6372 = true;
878 } else {
879 priv->reg_fields = common_reg_fields;
880 priv->ranges = common_led_ranges;
881 }
882
883 return 0;
884 }
885
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Re: [PATCH v3 5/8] drm/panel: panel-simple: Implement .get_orientation callback
Hi, On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote: > > To return the orientation property to drm/kms driver. > > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Hans de Goede > --- > v2->v3: add comments for notice. > --- > drivers/gpu/drm/panel/panel-simple.c | 16 +++- > 1 file changed, 15 insertions(+), 1 deletion(-) Reviewed-by: Douglas Anderson
Re: [PATCH v3 4/8] drm/panel: lvds: Implement .get_orientation callback
Hi,
On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote:
>
> To return the orientation property to drm/kms driver.
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Hans de Goede
> ---
> v2->v3: add comments for notice.
> ---
> drivers/gpu/drm/panel/panel-lvds.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> b/drivers/gpu/drm/panel/panel-lvds.c
> index 27a1c9923b09..239527409b00 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -102,15 +102,29 @@ static int panel_lvds_get_modes(struct drm_panel *panel,
> connector->display_info.bus_flags = lvds->data_mirror
> ? DRM_BUS_FLAG_DATA_LSB_TO_MSB
> : DRM_BUS_FLAG_DATA_MSB_TO_LSB;
Can you rebase your patch and send again? There's a context conflict
with the above line because your tree is lacking commit 83c784e70036
("drm/panel: lvds: Use bus_flags from DT panel-timing property")
In any case:
Reviewed-by: Douglas Anderson
Re: [PATCH v3 3/8] drm/panel: panel-edp: Implement .get_orientation callback
Hi, On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote: > > To return the orientation property to drm/kms driver. > > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Hans de Goede > --- > v2->v3: add comments for notice. > --- > drivers/gpu/drm/panel/panel-edp.c | 15 ++- > 1 file changed, 14 insertions(+), 1 deletion(-) Reviewed-by: Douglas Anderson
Re: [PATCH v3 2/8] drm/panel: boe-tv101wum-nl6: Implement .get_orientation callback
Hi, On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote: > > To return the orientation property to drm/kms driver. > > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Hans de Goede > --- > v2->v3: add comments for notice. > --- > drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Douglas Anderson
Re: [PATCH v3 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Hi,
On Sun, Jun 5, 2022 at 9:47 PM Hsin-Yi Wang wrote:
>
> Panels usually call drm_connector_set_panel_orientation(), which is
> later than drm/kms driver calling drm_dev_register(). This leads to a
> WARN().
>
> The orientation property is known earlier. For example, some panels
> parse the property through device tree during probe.
>
> Add an API to return the property from panel to drm/kms driver, so the
> drivers are able to call drm_connector_set_panel_orientation() before
> drm_dev_register().
>
> Suggested-by: Hans de Goede
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Hans de Goede
> ---
> v2->v3: no change
> ---
> drivers/gpu/drm/drm_panel.c | 8
> include/drm/drm_panel.h | 10 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index f634371c717a..4a512ca80673 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -223,6 +223,14 @@ int drm_panel_get_modes(struct drm_panel *panel,
> }
> EXPORT_SYMBOL(drm_panel_get_modes);
>
> +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel)
> +{
> + if (panel && panel->funcs && panel->funcs->get_orientation)
> + return panel->funcs->get_orientation(panel);
> +
> + return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +}
> +EXPORT_SYMBOL(drm_panel_get_orientation);
> #ifdef CONFIG_OF
nit: there used to be a blank line before the #ifdef but there no
longer is after your patch.
Other than that...
Reviewed-by: Douglas Anderson
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
On Mon, Jun 06, 2022 at 03:57:45PM +0200, Javier Martinez Canillas wrote: > Hello Maxime, > > On 6/6/22 15:52, Maxime Ripard wrote: > > hi, > > > > On Mon, Jun 06, 2022 at 03:49:57PM +0200, Javier Martinez Canillas wrote: > >> Hello Maxime, > >> > >> On 6/6/22 15:42, Maxime Ripard wrote: > >>> Hi, > >>> > >>> On Mon, Jun 06, 2022 at 11:55:16AM +0200, José Expósito wrote: > Test the conversion from XRGB to RGB332. > > What is tested? > > - Different values for the X in XRGB to make sure it is ignored > - Different clip values: Single pixel and full and partial buffer > - Well known colors: White, black, red, green, blue, magenta, yellow > and cyan > - Other colors: Randomly picked > - Destination pitch > > How to run the tests? > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ > --kconfig_add CONFIG_VIRTIO_UML=y \ > --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y > >>> > >>> It's not clear to me why you would need VIRTIO here? The Kunit config > >>> file should be enough to run the tests properly > >>> > >> > >> It's needed or otherwise KUnit will complain with: > >> > >> ./tools/testing/kunit/kunit.py run > >> --kunitconfig=drivers/gpu/drm/.kunitconfig > >> [15:47:31] Configuring KUnit Kernel ... > >> Regenerating .config ... > >> Populating config with: > >> $ make ARCH=um O=.kunit olddefconfig > >> ERROR:root:Not all Kconfig options selected in kunitconfig were in the > >> generated .config. > >> This is probably due to unsatisfied dependencies. > >> Missing: CONFIG_DRM=y, CONFIG_DRM_KUNIT_TEST=y > >> Note: many Kconfig options aren't available on UML. You can try running on > >> a different architecture with something like "--arch=x86_64". > >> > >> The following works correctly but it won't use User Mode Linux: > >> > >> ./tools/testing/kunit/kunit.py run > >> --kunitconfig=drivers/gpu/drm/.kunitconfig --arch=x86_64 > > > > But then, can't we add them to .kunitconfig? > > > > That's what I asked in the previous RFC too. Daniel mentioned that it > shouldn't > go there because is platform specific (AFAIU, one might want to test it on > x86, > aarch64, etc) but then I asked why we couldn't have a arch/um/.kunitconfig. > > The answer was that's not that simple and some agreement on how to do it is > needed: > > https://lists.freedesktop.org/archives/dri-devel/2022-June/357617.html Thanks for pointing this out. So yeah, it's indeed not very optimal We should probably just document it somewhere in KMS then? It doesn't have to be in this patch series, but I have the feeling that we will end up with that discussion a lot from people frustrated to have spent too much time figuring it out :) Maxime signature.asc Description: PGP signature
Re: [PATCH 13/14] drm/vc4: crtc: Fix out of order frames during asynchronous page flips
Hi,
On Thu, May 12, 2022 at 09:44:42AM -0100, Melissa Wen wrote:
> On 05/09, Melissa Wen wrote:
> > O 05/09, Melissa Wen wrote:
> > > On 05/03, Maxime Ripard wrote:
> > > > When doing an asynchronous page flip (PAGE_FLIP ioctl with the
> > > > DRM_MODE_PAGE_FLIP_ASYNC flag set), the current code waits for the
> > > > possible GPU buffer being rendered through a call to
> > > > vc4_queue_seqno_cb().
> > > >
> > > > On the BCM2835-37, the GPU driver is part of the vc4 driver and that
> > > > function is defined in vc4_gem.c to wait for the buffer to be rendered,
> > > > and once it's done, call a callback.
> > > >
> > > > However, on the BCM2711 used on the RaspberryPi4, the GPU driver is
> > > > separate (v3d) and that function won't do anything. This was working
> > > > because we were going into a path, due to uninitialized variables, that
> > > > was always scheduling the callback.
> > > >
> > > > However, we were never actually waiting for the buffer to be rendered
> > > > which was resulting in frames being displayed out of order.
> > > >
> > > > The generic API to signal those kind of completion in the kernel are the
> > > > DMA fences, and fortunately the v3d drivers supports them and signal
> > > > when its job is done. That API also provides an equivalent function that
> > > > allows to have a callback being executed when the fence is signalled as
> > > > done.
> > > >
> > > > Let's change our driver a bit to rely on the previous function for the
> > > > older SoCs, and on DMA fences for the BCM2711.
> > > >
> > > > Signed-off-by: Maxime Ripard
> > > > ---
> > > > drivers/gpu/drm/vc4/vc4_crtc.c | 41 --
> > > > 1 file changed, 39 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c
> > > > b/drivers/gpu/drm/vc4/vc4_crtc.c
> > > > index e0ae7bef08fa..8e1369fca937 100644
> > > > --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> > > > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> > > > @@ -776,6 +776,7 @@ struct vc4_async_flip_state {
> > > > struct drm_pending_vblank_event *event;
> > > >
> > > > union {
> > > > + struct dma_fence_cb fence;
> > > > struct vc4_seqno_cb seqno;
> > > > } cb;
> > > > };
> > > > @@ -835,6 +836,43 @@ static void
> > > > vc4_async_page_flip_seqno_complete(struct vc4_seqno_cb *cb)
> > > > vc4_bo_dec_usecnt(bo);
> > > > }
> > > >
> > > > +static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
> > > > + struct dma_fence_cb *cb)
> > > > +{
> > > > + struct vc4_async_flip_state *flip_state =
> > > > + container_of(cb, struct vc4_async_flip_state, cb.fence);
> > > > +
> > > > + vc4_async_page_flip_complete(flip_state);
> > > > + dma_fence_put(fence);
> > > > +}
> > > > +
> > > > +static int vc4_async_set_fence_cb(struct drm_device *dev,
> > > > + struct vc4_async_flip_state
> > > > *flip_state)
> > > > +{
> > > > + struct drm_framebuffer *fb = flip_state->fb;
> > > > + struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb,
> > > > 0);
> > > > + struct vc4_dev *vc4 = to_vc4_dev(dev);
> > > > + struct dma_fence *fence;
> > > > + int ret;
> > > > +
> > > > + if (!vc4->is_vc5) {
> > > > + struct vc4_bo *bo = to_vc4_bo(_bo->base);
> > > > +
> > > > + return vc4_queue_seqno_cb(dev, _state->cb.seqno,
> > > > bo->seqno,
> > > > +
> > > > vc4_async_page_flip_seqno_complete);
> > > > + }
> > > > +
> > > > + ret = dma_resv_get_singleton(cma_bo->base.resv, false, );
> > + for kernel bot complaint, I replaced false with `DMA_RESV_USAGE_READ`
> > to run some tests
> >
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (dma_fence_add_callback(fence, _state->cb.fence,
> me again :)
>
> I was thinking if we should add a check here for !fence and just complete the
> page flip,
> instead of letting `dma_fence_add_callback` warns whenever fence is NULL.
> I think there are situation in which fence is NULL and it is not an
> issue, right? Does it make sense?
I'm not sure. What situation do you have in mind?
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hello Maxime, On 6/6/22 15:52, Maxime Ripard wrote: > hi, > > On Mon, Jun 06, 2022 at 03:49:57PM +0200, Javier Martinez Canillas wrote: >> Hello Maxime, >> >> On 6/6/22 15:42, Maxime Ripard wrote: >>> Hi, >>> >>> On Mon, Jun 06, 2022 at 11:55:16AM +0200, José Expósito wrote: Test the conversion from XRGB to RGB332. What is tested? - Different values for the X in XRGB to make sure it is ignored - Different clip values: Single pixel and full and partial buffer - Well known colors: White, black, red, green, blue, magenta, yellow and cyan - Other colors: Randomly picked - Destination pitch How to run the tests? $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ --kconfig_add CONFIG_VIRTIO_UML=y \ --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y >>> >>> It's not clear to me why you would need VIRTIO here? The Kunit config >>> file should be enough to run the tests properly >>> >> >> It's needed or otherwise KUnit will complain with: >> >> ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig >> [15:47:31] Configuring KUnit Kernel ... >> Regenerating .config ... >> Populating config with: >> $ make ARCH=um O=.kunit olddefconfig >> ERROR:root:Not all Kconfig options selected in kunitconfig were in the >> generated .config. >> This is probably due to unsatisfied dependencies. >> Missing: CONFIG_DRM=y, CONFIG_DRM_KUNIT_TEST=y >> Note: many Kconfig options aren't available on UML. You can try running on a >> different architecture with something like "--arch=x86_64". >> >> The following works correctly but it won't use User Mode Linux: >> >> ./tools/testing/kunit/kunit.py run >> --kunitconfig=drivers/gpu/drm/.kunitconfig --arch=x86_64 > > But then, can't we add them to .kunitconfig? > That's what I asked in the previous RFC too. Daniel mentioned that it shouldn't go there because is platform specific (AFAIU, one might want to test it on x86, aarch64, etc) but then I asked why we couldn't have a arch/um/.kunitconfig. The answer was that's not that simple and some agreement on how to do it is needed: https://lists.freedesktop.org/archives/dri-devel/2022-June/357617.html -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
hi, On Mon, Jun 06, 2022 at 03:49:57PM +0200, Javier Martinez Canillas wrote: > Hello Maxime, > > On 6/6/22 15:42, Maxime Ripard wrote: > > Hi, > > > > On Mon, Jun 06, 2022 at 11:55:16AM +0200, José Expósito wrote: > >> Test the conversion from XRGB to RGB332. > >> > >> What is tested? > >> > >> - Different values for the X in XRGB to make sure it is ignored > >> - Different clip values: Single pixel and full and partial buffer > >> - Well known colors: White, black, red, green, blue, magenta, yellow > >>and cyan > >> - Other colors: Randomly picked > >> - Destination pitch > >> > >> How to run the tests? > >> > >> $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ > >> --kconfig_add CONFIG_VIRTIO_UML=y \ > >> --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y > > > > It's not clear to me why you would need VIRTIO here? The Kunit config > > file should be enough to run the tests properly > > > > It's needed or otherwise KUnit will complain with: > > ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig > [15:47:31] Configuring KUnit Kernel ... > Regenerating .config ... > Populating config with: > $ make ARCH=um O=.kunit olddefconfig > ERROR:root:Not all Kconfig options selected in kunitconfig were in the > generated .config. > This is probably due to unsatisfied dependencies. > Missing: CONFIG_DRM=y, CONFIG_DRM_KUNIT_TEST=y > Note: many Kconfig options aren't available on UML. You can try running on a > different architecture with something like "--arch=x86_64". > > The following works correctly but it won't use User Mode Linux: > > ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig > --arch=x86_64 But then, can't we add them to .kunitconfig? We should avoid that gotcha entirely Maxime signature.asc Description: PGP signature
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hello Maxime, On 6/6/22 15:42, Maxime Ripard wrote: > Hi, > > On Mon, Jun 06, 2022 at 11:55:16AM +0200, José Expósito wrote: >> Test the conversion from XRGB to RGB332. >> >> What is tested? >> >> - Different values for the X in XRGB to make sure it is ignored >> - Different clip values: Single pixel and full and partial buffer >> - Well known colors: White, black, red, green, blue, magenta, yellow >>and cyan >> - Other colors: Randomly picked >> - Destination pitch >> >> How to run the tests? >> >> $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ >> --kconfig_add CONFIG_VIRTIO_UML=y \ >> --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y > > It's not clear to me why you would need VIRTIO here? The Kunit config > file should be enough to run the tests properly > It's needed or otherwise KUnit will complain with: ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig [15:47:31] Configuring KUnit Kernel ... Regenerating .config ... Populating config with: $ make ARCH=um O=.kunit olddefconfig ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. This is probably due to unsatisfied dependencies. Missing: CONFIG_DRM=y, CONFIG_DRM_KUNIT_TEST=y Note: many Kconfig options aren't available on UML. You can try running on a different architecture with something like "--arch=x86_64". The following works correctly but it won't use User Mode Linux: ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig --arch=x86_64 -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v13 1/3] phy: qcom-edp: add regulator_set_load to edp phy
On Wed 25 May 14:02 PDT 2022, Kuogee Hsieh wrote:
> This patch add regulator_set_load() before enable regulator at
> eDP phy driver.
>
> Signed-off-by: Kuogee Hsieh
> Reviewed-by: Douglas Anderson
Reviewed-by: Bjorn Andersson
> ---
> drivers/phy/qualcomm/phy-qcom-edp.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c
> b/drivers/phy/qualcomm/phy-qcom-edp.c
> index cacd32f..7e357078 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -639,6 +639,18 @@ static int qcom_edp_phy_probe(struct platform_device
> *pdev)
> if (ret)
> return ret;
>
> + ret = regulator_set_load(edp->supplies[0].consumer, 21800); /* 1.2 V
> vdda-phy */
> + if (ret) {
> + dev_err(dev, "failed to set load at %s\n",
> edp->supplies[0].supply);
> + return ret;
> + }
> +
> + ret = regulator_set_load(edp->supplies[1].consumer, 36000); /* 0.9 V
> vdda-pll */
> + if (ret) {
> + dev_err(dev, "failed to set load at %s\n",
> edp->supplies[1].supply);
> + return ret;
> + }
> +
> ret = qcom_edp_clks_register(edp, pdev->dev.of_node);
> if (ret)
> return ret;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hi, On Mon, Jun 06, 2022 at 11:55:16AM +0200, José Expósito wrote: > Test the conversion from XRGB to RGB332. > > What is tested? > > - Different values for the X in XRGB to make sure it is ignored > - Different clip values: Single pixel and full and partial buffer > - Well known colors: White, black, red, green, blue, magenta, yellow >and cyan > - Other colors: Randomly picked > - Destination pitch > > How to run the tests? > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ > --kconfig_add CONFIG_VIRTIO_UML=y \ > --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y It's not clear to me why you would need VIRTIO here? The Kunit config file should be enough to run the tests properly Maxime signature.asc Description: PGP signature
Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hello José, On 6/6/22 11:55, José Expósito wrote: > Test the conversion from XRGB to RGB332. > > What is tested? > > - Different values for the X in XRGB to make sure it is ignored > - Different clip values: Single pixel and full and partial buffer > - Well known colors: White, black, red, green, blue, magenta, yellow >and cyan > - Other colors: Randomly picked > - Destination pitch > > How to run the tests? > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \ > --kconfig_add CONFIG_VIRTIO_UML=y \ > --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y > > Suggested-by: Javier Martinez Canillas > Signed-off-by: José Expósito > > --- Thanks for addressing the issues pointed out. Patch looks good to me now. Reviewed-by: Javier Martinez Canillas By the way, I think you should request an account [0], so that you can push patches to drm-misc directly. Specially since AFAIU the plan is to add more KUnit tests in future patch series. [0]: https://www.freedesktop.org/wiki/AccountRequests/ -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH] drm/ttm: fix missing NULL check in ttm_device_swapout
Am 2022-06-03 um 06:46 schrieb Christian König:
Resources about to be destructed are not tied to BOs any more.
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/ttm/ttm_device.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index a0562ab386f5..e7147e304637 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -156,8 +156,12 @@ int ttm_device_swapout(struct ttm_device *bdev, struct
ttm_operation_ctx *ctx,
ttm_resource_manager_for_each_res(man, , res) {
struct ttm_buffer_object *bo = res->bo;
- uint32_t num_pages = PFN_UP(bo->base.size);
+ uint32_t num_pages;
+ if (!bo)
+ continue;
+
+ num_pages = PFN_UP(bo->base.size);
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the lru_lock */
if (!ret)
Re: [RESEND v4 1/3] dt-bindings: mediatek: add vdosys1 RDMA definition for mt8195
On Mon, 06 Jun 2022 13:11:29 +0800, Bo-Chen Chen wrote:
> From: "Nancy.Lin"
>
> Add vdosys1 RDMA definition.
>
> Signed-off-by: Nancy.Lin
> Signed-off-by: Bo-Chen Chen
> Reviewed-by: AngeloGioacchino Del Regno
>
> Reviewed-by: Krzysztof Kozlowski
> Tested-by: AngeloGioacchino Del Regno
>
> ---
> .../display/mediatek/mediatek,mdp-rdma.yaml | 88 +++
> 1 file changed, 88 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml:
properties:compatible: [{'const': 'mediatek,mt8195-vdo1-rdma'}] is not of type
'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml:
ignoring, error in schema: properties: compatible
Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.example.dtb:0:0:
/example-0/soc/rdma@1c104000: failed to match any schema with compatible:
['mediatek,mt8195-vdo1-rdma']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Re: [RESEND v4 3/3] dt-bindings: mediatek: add ethdr definition for mt8195
On Mon, 06 Jun 2022 13:11:31 +0800, Bo-Chen Chen wrote:
> From: "Nancy.Lin"
>
> Add vdosys1 ETHDR definition.
>
> Signed-off-by: Nancy.Lin
> Signed-off-by: Bo-Chen Chen
> Reviewed-by: Chun-Kuang Hu
> Reviewed-by: AngeloGioacchino Del Regno
>
> Reviewed-by: Krzysztof Kozlowski
> Tested-by: AngeloGioacchino Del Regno
>
> ---
> .../display/mediatek/mediatek,ethdr.yaml | 188 ++
> 1 file changed, 188 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml:
properties:compatible: [{'const': 'mediatek,mt8195-disp-ethdr'}] is not of
type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml:
ignoring, error in schema: properties: compatible
Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.example.dtb:0:0:
/example-0/soc/hdr-engine@1c114000: failed to match any schema with
compatible: ['mediatek,mt8195-disp-ethdr']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Re: [PATCH] drm/i915/dg2: Catch and log more unexpected values in DG1_MSTR_TILE_INTR
On 27/05/2022 19:42, Matt Roper wrote:
On Thu, May 26, 2022 at 11:18:17AM +0100, Tvrtko Ursulin wrote:
On 25/05/2022 19:05, Matt Roper wrote:
On Wed, May 25, 2022 at 05:03:13PM +0100, Tvrtko Ursulin wrote:
On 24/05/2022 18:51, Matt Roper wrote:
On Tue, May 24, 2022 at 10:43:39AM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Catch and log any garbage in the register, including no tiles marked, or
multiple tiles marked.
Signed-off-by: Tvrtko Ursulin
Cc: Matt Roper
---
We caught garbage in DG1_MSTR_TILE_INTR with DG2 (actual value 0xF9D2C008)
during glmark and more badness. So I thought lets log all possible failure
modes from here and also use per device logging.
---
drivers/gpu/drm/i915/i915_irq.c | 33 ++---
drivers/gpu/drm/i915/i915_reg.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 73cebc6aa650..79853d3fc1ed 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2778,24 +2778,30 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
u32 gu_misc_iir;
if (!intel_irqs_enabled(i915))
- return IRQ_NONE;
+ goto none;
master_tile_ctl = dg1_master_intr_disable(regs);
- if (!master_tile_ctl) {
- dg1_master_intr_enable(regs);
- return IRQ_NONE;
+ if (!master_tile_ctl)
+ goto enable_none;
+
+ if (master_tile_ctl & ~(DG1_MSTR_IRQ | DG1_MSTR_TILE_MASK)) {
+ drm_warn(>drm, "Garbage in master_tile_ctl: 0x%08x!\n",
+master_tile_ctl);
I know we have a bunch of them already, but shouldn't we be avoiding
printk-based stuff like this inside interrupt handlers? Should we be
migrating all these error messages over to trace_printk or something
similar that's safer to use?
Not sure - I kind of think some really unexpected and worrying situations
should be loud and on by default. Risk is then spam if not ratelimited.
Maybe we should instead ratelimit most errors/warnings coming for irq
handlers?
It's not the risk of spam that's the problem, but rather that
printk-based stuff eventually calls into the console code to flush its
buffers. That's way more overhead than you want in an interrupt handler
so it's bad on its own, but if you're using something slow like a serial
console, it becomes even more of a problem.
Is it a problem for messages which we never expect to see?
Kind of. While not as catastrophic, it's the same argument for why we
don't use BUG() anymore...when the impossible does manage to happen
there's unnecessary collateral damage on things outside of graphics. If
we're adding huge delays inside an interrupt handler (while other
interrupts are disabled) that impacts the system-wide usability, not
just our own driver.
I'd also argue that these messages actually are semi-expected. Random
bits being set shouldn't happen, but in the world of dgpu's, we do
occasionally see cases where the PCI link itself goes down for reasons
outside our control and then all registers read back as 0x,
which will probably trigger error messages here (as well as a bunch of
other places).
Could you expand a bit on what is semi-expected and when? I mean the
circumstances of PCI link going down. We certainly don't have any code
to survive that.
While the unexpected bits in the master tile register are strange and
may point to a bigger problem somewhere else, they're also harmless on
their own since we should just ignore those bits and only process the
valid tiles.
Yes, I was expecting that a patch belonging to multi-tile enablement would
be incoming soon, which would be changing:
+ if (REG_FIELD_GET(DG1_MSTR_TILE_MASK, master_tile_ctl) !=
+ DG1_MSTR_TILE(0)) {
+ drm_warn(>drm, "Unexpected irq from tile %u!\n",
+ilog2(REG_FIELD_GET(DG1_MSTR_TILE_MASK,
+master_tile_ctl)));
+ goto enable_none;
}
From this patch, into something completely different like walking bit by
bit, handling the present tiles, and warning on unexpected ones. What should
remain though is warning on no tiles signaled (which what we saw, together
with garbage in reserved bits).
Yeah. Although I still feel the interrupt handler should really just be
flagging the errors so that the actual prints themselves can happen
outside the interrupt.
In this particular case at least DRM_ERROR with no device info is the odd
one out in the entire file so I'd suggest changing at least that, if the
rest of my changes is of questionable benefit.
Changing DRM_ERROR -> drm_err would probably be fine in the short term
since it doesn't really make us any worse off. Changing to drm_warn
might not be great since we're generating a lot more lines of output and
Sorry I don't follow - why does replacing
[PATCH 3/3] drm/komeda - Fix handling of pending crtc state commit to avoid lock-up
From: Carsten Haitzler
Sometimes there is an extra dcm crtc state in the pipeline whose
penting vblank event has not been handled yet. We would previously
throw this out and the vblank event never triggers leading to hard
lockups higher up the stack where an expected vlank event never comes
back (screen freezes).
This fixes that by tracking a pending crtc state that needs handling
and handle it producing a vlank event next vblank if it had not
laready been handled before. This fixes the hangs and ensures our
display keeps updating seamlessly and is certainly a much better state
to be in than after some time ending up with a mysteriously frozen
screen and a lot of kernle messages complaining about this too.
Signed-off-by: Carsten Haitzler
---
.../gpu/drm/arm/display/komeda/komeda_crtc.c | 10 ++
.../gpu/drm/arm/display/komeda/komeda_kms.c | 19 ++-
.../gpu/drm/arm/display/komeda/komeda_kms.h | 3 +++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 59172acb9738..b7f0a5f97222 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -227,6 +227,16 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
complete_all(kcrtc->disable_done);
kcrtc->disable_done = NULL;
} else if (crtc->state->event) {
+ if (kcrtc->state_needs_handling) {
+ event = kcrtc->state_needs_handling->event;
+ if (event) {
+ kcrtc->state_needs_handling->event =
NULL;
+ kcrtc->state_needs_handling = NULL;
+ drm_crtc_send_vblank_event(crtc, event);
+ } else {
+ kcrtc->state_needs_handling = NULL;
+ }
+ }
event = crtc->state->event;
/*
* Consume event before notifying drm core that flip
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index 93b7f09b96ca..bbc051a1896a 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -226,10 +226,27 @@ static int komeda_kms_check(struct drm_device *dev,
return 0;
}
+static int komeda_kms_commit(struct drm_device *drm,
+ struct drm_atomic_state *state,
+ bool nonblock)
+{
+ int i;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+ struct komeda_crtc *kcrtc;
+
+ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
+ kcrtc = to_kcrtc(crtc);
+ kcrtc->state_needs_handling = crtc->state;
+ }
+ return drm_atomic_helper_commit(drm, state, nonblock);
+}
+
static const struct drm_mode_config_funcs komeda_mode_config_funcs = {
.fb_create = komeda_fb_create,
.atomic_check = komeda_kms_check,
- .atomic_commit = drm_atomic_helper_commit,
+ .atomic_commit = komeda_kms_commit,
};
static void komeda_kms_mode_config_init(struct komeda_kms_dev *kms,
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 456f3c435719..8ff3ad04dfe4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -84,6 +84,9 @@ struct komeda_crtc {
/** @disable_done: this flip_done is for tracing the disable */
struct completion *disable_done;
+
+ /** @state_needs_handling: Has not had it's vblank event handled yet */
+ struct drm_crtc_state *state_needs_handling;
};
/**
--
2.32.0
[PATCH 2/3] drm/komeda - At init write GCU control block to handle already on DPU
From: Carsten Haitzler If something has already set up the DPU before the komeda driver comes up, it will fail to init because it was just writing to the SRST bit in the GCU control register and ignoring others. This resulted in TBU bringup stalling and init failing. By writing completely we also set the mode back to 0 (inactive) too and thus TBU bringup works. Signed-off-by: Carsten Haitzler --- drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c index 00fa56c29b3e..39618c1a4c81 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c @@ -309,8 +309,7 @@ static int d71_reset(struct d71_dev *d71) u32 __iomem *gcu = d71->gcu_addr; int ret; - malidp_write32_mask(gcu, BLK_CONTROL, - GCU_CONTROL_SRST, GCU_CONTROL_SRST); + malidp_write32(gcu, BLK_CONTROL, GCU_CONTROL_SRST); ret = dp_wait_cond(!(malidp_read32(gcu, BLK_CONTROL) & GCU_CONTROL_SRST), 100, 1000, 1); -- 2.32.0
[PATCH 1/3] drm/komeda - Add legacy FB support so VT's work as expected
From: Carsten Haitzler The komeda driver doesn't come up with a visible text (FB) mode VT by default as it was missing legacy FB support. It's useful to have a working text VT on a system for debug and general usability, so enable it. You can always toggle CONFIG_FRAMEBUFFER_CONSOLE. Signed-off-by: Carsten Haitzler --- drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c index e7933930a657..c0c7933a9631 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "komeda_dev.h" #include "komeda_kms.h" @@ -71,6 +72,7 @@ static int komeda_bind(struct device *dev) } dev_set_drvdata(dev, mdrv); + drm_fbdev_generic_setup(>kms->base, 32); return 0; -- 2.32.0
Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
Hi Maxime, пн, 6 июн. 2022 г. в 12:22, Maxime Ripard : > > On Mon, Jun 06, 2022 at 11:20:06AM +0200, Maxime Ripard wrote: > > On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote: > > > Hello Jernej, > > > > > > Thank you for having a look. > > > > > > вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec : > > > > > > > > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko > > > > napisal(a): > > > > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes: > > > > > "None", "Pre-multiplied", "Coverage" > > > > > > > > > > Add the blend mode property and route it to the appropriate registers. > > > > > > > > > > Note: > > > > > "force_premulti" parameter was added to handle multi-overlay channel > > > > > cases in future changes. It must be set to true for cases when more > > > > > than 1 overlay layer is used within a channel and at least one of the > > > > > overlay layers within a group uses premultiplied blending mode. > > > > > > > > Please remove this parameter. It's nothing special, so it can be easily > > > > added > > > > once it's actually needed. For now, it only complicates code. > > > > > > I would prefer keeping it if you do not have any strong opinion against > > > it. > > > > > > I am working now on exposing all overlays, so it will be needed soon > > > anyway. > > > > KMS assumes pre-multiplied alpha anyway, so we shouldn't need a > > parameter to force it: we're always going to force it. > > My bad, I got confused with your other patch. > > Still, I agree with Jernej, if it's not needed it only complicates the > code for no particular reason. If you need it at some point in the > future, add it then. Otherwise, it's hard to reason about, since we > don't know what are the expectations that those future patches will > bring. Well, existing code already has some sort of support for the sub-overlays: For example 'int overlay, ' parameter is always 0, but it still passed through the call stack. Therefore this patch is just aligned with already existing traditions to keep sub-overlays in mind. > > Maxime Regards, Roman
Re: [RFC PATCH 3/5] drm/amdgpu: Allow explicit sync for VM ops.
On Mon, Jun 6, 2022 at 12:35 PM Christian König wrote: > > Am 06.06.22 um 12:30 schrieb Bas Nieuwenhuizen: > > On Mon, Jun 6, 2022 at 12:15 PM Christian König > > wrote: > >> > >> > >> Am 03.06.22 um 21:11 schrieb Bas Nieuwenhuizen: > >>> On Fri, Jun 3, 2022 at 8:41 PM Christian König > >>> wrote: > Am 03.06.22 um 19:50 schrieb Bas Nieuwenhuizen: > > [SNIP] > Yeah, but that's exactly the bubble we try to avoid. Isn't it? > >>> For this series, not really. To clarify there are two sides for > >>> getting GPU bubbles and no overlap: > >>> > >>> (1) VM operations implicitly wait for earlier CS submissions > >>> (2) CS submissions implicitly wait for earlier VM operations > >>> > >>> Together, these combine to ensure that you get a (potentially small) > >>> bubble any time VM work happens. > >>> > >>> Your series (and further ideas) tackles (2), and is a worthwhile thing > >>> to do. However, while writing the userspace for this I noticed this > >>> isn't enough to get rid of all our GPU bubbles. In particular when > >>> doing a non-sparse map of a new BO, that tends to need to be waited on > >>> for the next CS anyway for API semantics. Due to VM operations > >>> happening on a single timeline that means this high priority map can > >>> end up being blocked by earlier sparse maps and hence the bubble in > >>> that case still exists. > >>> > >>> So in this series I try to tackle (1) instead. Since GPU work > >>> typically lags behind CPU submissions and VM operations aren't that > >>> slow, we can typically execute VM operations early enough that any > >>> implicit syncs from (2) are less/no issue. > >> Ok, once more since you don't seem to understand what I want to say: It > >> isn't possible to fix #1 before you have fixed #2. > >> > >> The VM unmap operation here is a barrier which divides the CS > >> operations > >> in a before and after. This is intentional design. > > Why is that barrier needed? The two barriers I got and understood and > > I think we can deal with: > > > > 1) the VM unmap is a barrier between prior CS and later memory free. > > 2) The TLB flush need to happen between a VM unmap and later CS. > > > > But why do we need the VM unmap to be a strict barrier between prior > > CS and later CS? > Exactly because of the two reasons you mentioned. > >>> This is the part I'm not seeing. I get that removing #2 is a > >>> nightmare, which is why I did something that doesn't violate that > >>> constraint. > >>> > >>> Like if an explicit CS that was running before the VM operation runs > >>> till after the VM operation (and hence possibly till after the TLB > >>> flush, or otherwise have the TLB flush not apply due to lack of async > >>> TLB flush support), that is not an issue. It might see the state from > >>> before the unmap, or after the unmap, or some intermediate state and > >>> all of those would be okay. > >>> > >>> We still get the constraint that the TLB flush happens between the VM > >>> unmap and later CS and hence the unmap is certainly visible to them. > >> So you basically just want to set the sync mode in > >> amdgpu_vm_update_range() to AMDGPU_SYNC_EXPLICIT even when it is an unmap? > > Yes, with the caveat that I want to do that only for > > DMA_RESV_USAGE_BOOKKEEP or higher (i.e. if we submit a CS with > > implicit sync we get the old implicit behavior, if we submit a CS with > > explicit sync we get the new explicit behavior). The rest of the > > series is basically just for enabling explicit sync submissions. > > That part won't work at all and would cause additional synchronization > problems. > > First of all for implicit synced CS we should use READ, not BOOKKEEP. > Because BOOKKEEP would incorrectly be ignored by OpenGL importers. I've > fixed that this causes memory corruption, but it is still nice to avoid. Yes, what I'm saying is that on implicit sync CS submission should add READ fences to the dma resv and on explicit sync CS submission should add BOOKKEEP fences. > > BOOKKEEP can only be used by VM updates themselves. So that they don't > interfere with CS. That is the point why we would go BOOKKEEP for explicit sync CS submissions, no? Explicit submission shouldn't interfere with any other CS submissions. That includes being totally ignored by GL importers (if we want to have synchronization there between an explicit submission and GL, userspace is expected to use Jason's dmabuf fence import/export IOCTLs) > > Then the second problem is that the VM IOCTL has absolutely no idea what > the CS IOCTL would be doing. That's why we have added the EXPLICIT sync > flag on the BO. It doesn't need to? We just use a different sync_mode for BOOKKEEP fences vs others: https://patchwork.freedesktop.org/patch/487887/?series=104578=2 (the nice thing about doing it this way is that it is independent of the
Re: [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
Am 05.06.22 um 18:47 schrieb Daniel Vetter:
On Fri, 27 May 2022 at 01:55, Dmitry Osipenko
wrote:
Introduce a common DRM SHMEM shrinker framework that allows to reduce
code duplication among DRM drivers by replacing theirs custom shrinker
implementations with the generic shrinker.
In order to start using DRM SHMEM shrinker drivers should:
1. Implement new evict() shmem object callback.
2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device).
3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to
activate shrinking of shmem GEMs.
This patch is based on a ideas borrowed from Rob's Clark MSM shrinker,
Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker.
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
So I guess I get a price for being blind since forever, because this
thing existed since at least 2013. I just stumbled over
llist_lru.[hc], a purpose built list helper for shrinkers. I think we
should try to adopt that so that our gpu shrinkers look more like
shrinkers for everything else.
What the heck are you talking about?
I can't find any llist_lru.[hc] in the linux kernel sources.
Christian.
Apologies for this, since I fear this might cause a bit of churn.
Hopefully it's all contained to the list manipulation code in shmem
helpers, I don't think this should leak any further.
-Daniel
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 540 --
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +-
drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +
include/drm/drm_device.h | 4 +
include/drm/drm_gem_shmem_helper.h| 87 ++-
5 files changed, 594 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 555fe212bd98..4cd0b5913492 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -126,6 +126,42 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct
drm_device *dev, size_t
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
+static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem)
+{
+ return (shmem->madv >= 0) && shmem->evict &&
+ shmem->eviction_enabled && shmem->pages_use_count &&
+ !shmem->pages_pin_count && !shmem->base.dma_buf &&
+ !shmem->base.import_attach && shmem->sgt && !shmem->evicted;
+}
+
+static void
+drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = >base;
+ struct drm_gem_shmem_shrinker *gem_shrinker = obj->dev->shmem_shrinker;
+
+ dma_resv_assert_held(shmem->base.resv);
+
+ if (!gem_shrinker || obj->import_attach)
+ return;
+
+ mutex_lock(_shrinker->lock);
+
+ if (drm_gem_shmem_is_evictable(shmem) ||
+ drm_gem_shmem_is_purgeable(shmem))
+ list_move_tail(>madv_list, _shrinker->lru_evictable);
+ else if (shmem->madv < 0)
+ list_del_init(>madv_list);
+ else if (shmem->evicted)
+ list_move_tail(>madv_list, _shrinker->lru_evicted);
+ else if (!shmem->pages)
+ list_del_init(>madv_list);
+ else
+ list_move_tail(>madv_list, _shrinker->lru_pinned);
+
+ mutex_unlock(_shrinker->lock);
+}
+
/**
* drm_gem_shmem_free - Free resources associated with a shmem GEM object
* @shmem: shmem GEM object to free
@@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
} else {
dma_resv_lock(shmem->base.resv, NULL);
+ /* take out shmem GEM object from the memory shrinker */
+ drm_gem_shmem_madvise(shmem, -1);
+
WARN_ON(shmem->vmap_use_count);
if (shmem->sgt) {
@@ -150,7 +189,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
- if (shmem->pages)
+ if (shmem->pages_use_count)
drm_gem_shmem_put_pages(shmem);
WARN_ON(shmem->pages_use_count);
@@ -163,18 +202,82 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
-static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+/**
+ * drm_gem_shmem_set_evictable() - Make GEM evictable by memory shrinker
+ * @shmem: shmem GEM object
+ *
+ * Tell memory shrinker that this GEM can be evicted. Initially eviction is
+ * disabled for all GEMs. If GEM was purged, then -ENOMEM is returned.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_set_evictable(struct drm_gem_shmem_object *shmem)
+{
+ dma_resv_lock(shmem->base.resv, NULL);
+
+ if (shmem->madv < 0)
+ return -ENOMEM;
+
+
Re: [RFC PATCH 3/5] drm/amdgpu: Allow explicit sync for VM ops.
Am 06.06.22 um 12:30 schrieb Bas Nieuwenhuizen: On Mon, Jun 6, 2022 at 12:15 PM Christian König wrote: Am 03.06.22 um 21:11 schrieb Bas Nieuwenhuizen: On Fri, Jun 3, 2022 at 8:41 PM Christian König wrote: Am 03.06.22 um 19:50 schrieb Bas Nieuwenhuizen: [SNIP] Yeah, but that's exactly the bubble we try to avoid. Isn't it? For this series, not really. To clarify there are two sides for getting GPU bubbles and no overlap: (1) VM operations implicitly wait for earlier CS submissions (2) CS submissions implicitly wait for earlier VM operations Together, these combine to ensure that you get a (potentially small) bubble any time VM work happens. Your series (and further ideas) tackles (2), and is a worthwhile thing to do. However, while writing the userspace for this I noticed this isn't enough to get rid of all our GPU bubbles. In particular when doing a non-sparse map of a new BO, that tends to need to be waited on for the next CS anyway for API semantics. Due to VM operations happening on a single timeline that means this high priority map can end up being blocked by earlier sparse maps and hence the bubble in that case still exists. So in this series I try to tackle (1) instead. Since GPU work typically lags behind CPU submissions and VM operations aren't that slow, we can typically execute VM operations early enough that any implicit syncs from (2) are less/no issue. Ok, once more since you don't seem to understand what I want to say: It isn't possible to fix #1 before you have fixed #2. The VM unmap operation here is a barrier which divides the CS operations in a before and after. This is intentional design. Why is that barrier needed? The two barriers I got and understood and I think we can deal with: 1) the VM unmap is a barrier between prior CS and later memory free. 2) The TLB flush need to happen between a VM unmap and later CS. But why do we need the VM unmap to be a strict barrier between prior CS and later CS? Exactly because of the two reasons you mentioned. This is the part I'm not seeing. I get that removing #2 is a nightmare, which is why I did something that doesn't violate that constraint. Like if an explicit CS that was running before the VM operation runs till after the VM operation (and hence possibly till after the TLB flush, or otherwise have the TLB flush not apply due to lack of async TLB flush support), that is not an issue. It might see the state from before the unmap, or after the unmap, or some intermediate state and all of those would be okay. We still get the constraint that the TLB flush happens between the VM unmap and later CS and hence the unmap is certainly visible to them. So you basically just want to set the sync mode in amdgpu_vm_update_range() to AMDGPU_SYNC_EXPLICIT even when it is an unmap? Yes, with the caveat that I want to do that only for DMA_RESV_USAGE_BOOKKEEP or higher (i.e. if we submit a CS with implicit sync we get the old implicit behavior, if we submit a CS with explicit sync we get the new explicit behavior). The rest of the series is basically just for enabling explicit sync submissions. That part won't work at all and would cause additional synchronization problems. First of all for implicit synced CS we should use READ, not BOOKKEEP. Because BOOKKEEP would incorrectly be ignored by OpenGL importers. I've fixed that this causes memory corruption, but it is still nice to avoid. BOOKKEEP can only be used by VM updates themselves. So that they don't interfere with CS. Then the second problem is that the VM IOCTL has absolutely no idea what the CS IOCTL would be doing. That's why we have added the EXPLICIT sync flag on the BO. Regards, Christian. That should be doable, but then you don't need any of the other changes. Regards, Christian. #1 Is rather easy to fix, you just need to copy all dma_fences from the page table dma_resv object over to the BOs dma_resv object in the gem close handler. E.g. exactly what you suggested with the dma_resv_copy function. #2 is a nightmare. We can't move the TLB flush at the end of the unmap operation because on async TLB flushes are either a bit complicated (double flushes etc..) or don't even work at all because of hw bugs. So to have a reliable TLB flush we must make sure that nothing else is ongoing and that means CS->VM->CS barrier. We try very hard to circumvent that already on maps by (for example) using a completely new VMID for CS after the VM map operation. But for the unmap operation we would need some kind special dma_fence implementation which would not only wait for all existing dma_fence but also for the one added until the unmap operation is completed. Cause otherwise our operation we do at #1 would simply not catch all dma_fences which have access to the memory. That's certainly doable, but I think just using the drm_exec stuff I already came up with is easier. When we can grab locks for all the BOs involved amdgpu_vm_clear_freed() goes away and we
Re: [RFC PATCH 3/5] drm/amdgpu: Allow explicit sync for VM ops.
On Mon, Jun 6, 2022 at 12:15 PM Christian König wrote: > > > > Am 03.06.22 um 21:11 schrieb Bas Nieuwenhuizen: > > On Fri, Jun 3, 2022 at 8:41 PM Christian König > > wrote: > >> Am 03.06.22 um 19:50 schrieb Bas Nieuwenhuizen: > >>> [SNIP] > >> Yeah, but that's exactly the bubble we try to avoid. Isn't it? > > For this series, not really. To clarify there are two sides for > > getting GPU bubbles and no overlap: > > > > (1) VM operations implicitly wait for earlier CS submissions > > (2) CS submissions implicitly wait for earlier VM operations > > > > Together, these combine to ensure that you get a (potentially small) > > bubble any time VM work happens. > > > > Your series (and further ideas) tackles (2), and is a worthwhile thing > > to do. However, while writing the userspace for this I noticed this > > isn't enough to get rid of all our GPU bubbles. In particular when > > doing a non-sparse map of a new BO, that tends to need to be waited on > > for the next CS anyway for API semantics. Due to VM operations > > happening on a single timeline that means this high priority map can > > end up being blocked by earlier sparse maps and hence the bubble in > > that case still exists. > > > > So in this series I try to tackle (1) instead. Since GPU work > > typically lags behind CPU submissions and VM operations aren't that > > slow, we can typically execute VM operations early enough that any > > implicit syncs from (2) are less/no issue. > Ok, once more since you don't seem to understand what I want to say: It > isn't possible to fix #1 before you have fixed #2. > > The VM unmap operation here is a barrier which divides the CS operations > in a before and after. This is intentional design. > >>> Why is that barrier needed? The two barriers I got and understood and > >>> I think we can deal with: > >>> > >>> 1) the VM unmap is a barrier between prior CS and later memory free. > >>> 2) The TLB flush need to happen between a VM unmap and later CS. > >>> > >>> But why do we need the VM unmap to be a strict barrier between prior > >>> CS and later CS? > >> Exactly because of the two reasons you mentioned. > > This is the part I'm not seeing. I get that removing #2 is a > > nightmare, which is why I did something that doesn't violate that > > constraint. > > > > Like if an explicit CS that was running before the VM operation runs > > till after the VM operation (and hence possibly till after the TLB > > flush, or otherwise have the TLB flush not apply due to lack of async > > TLB flush support), that is not an issue. It might see the state from > > before the unmap, or after the unmap, or some intermediate state and > > all of those would be okay. > > > > We still get the constraint that the TLB flush happens between the VM > > unmap and later CS and hence the unmap is certainly visible to them. > > So you basically just want to set the sync mode in > amdgpu_vm_update_range() to AMDGPU_SYNC_EXPLICIT even when it is an unmap? Yes, with the caveat that I want to do that only for DMA_RESV_USAGE_BOOKKEEP or higher (i.e. if we submit a CS with implicit sync we get the old implicit behavior, if we submit a CS with explicit sync we get the new explicit behavior). The rest of the series is basically just for enabling explicit sync submissions. > That should be doable, but then you don't need any of the other changes. > > Regards, > Christian. > > > > >> #1 Is rather easy to fix, you just need to copy all dma_fences from the > >> page table dma_resv object over to the BOs dma_resv object in the gem > >> close handler. E.g. exactly what you suggested with the dma_resv_copy > >> function. > >> > >> #2 is a nightmare. > >> > >> We can't move the TLB flush at the end of the unmap operation because on > >> async TLB flushes are either a bit complicated (double flushes etc..) or > >> don't even work at all because of hw bugs. So to have a reliable TLB > >> flush we must make sure that nothing else is ongoing and that means > >> CS->VM->CS barrier. > >> > >> We try very hard to circumvent that already on maps by (for example) > >> using a completely new VMID for CS after the VM map operation. > >> > >> But for the unmap operation we would need some kind special dma_fence > >> implementation which would not only wait for all existing dma_fence but > >> also for the one added until the unmap operation is completed. Cause > >> otherwise our operation we do at #1 would simply not catch all > >> dma_fences which have access to the memory. > >> > >> That's certainly doable, but I think just using the drm_exec stuff I > >> already came up with is easier. > >> > >> When we can grab locks for all the BOs involved amdgpu_vm_clear_freed() > >> goes away and we can keep track of the unmap operations in the bo_va > >> structure. > >> > >> With that done you can make the explicit sync you noted in the bo_va > >>
Re: [PATCH] drm/ttm: fix missing NULL check in ttm_device_swapout
Am 04.06.22 um 00:44 schrieb Felix Kuehling:
[+amd-gfx]
On 2022-06-03 15:37, Felix Kuehling wrote:
On 2022-06-03 06:46, Christian König wrote:
Resources about to be destructed are not tied to BOs any more.
I've been seeing a backtrace in that area with a patch series I'm
working on, but didn't have enough time to track it down yet. I'll
try if this patch fixes it.
The patch doesn't apply on amd-staging-drm-next. I made the following
change instead, which fixes my problem (and I do see the pr_err being
triggered):
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -157,6 +157,10 @@ int ttm_device_swapout(struct ttm_device *bdev,
struct ttm_operation_ctx *ctx,
list_for_each_entry(bo, >lru[j], lru) {
uint32_t num_pages =
PFN_UP(bo->base.size);
+ if (!bo->resource) {
+ pr_err("### bo->resource is
NULL\n");
+ continue;
+ }
Yeah, that should be functional identical.
Can I get an rb for that? Going to provide backports to older kernels as
well then.
Regards,
Christian.
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the
lru_lock */
if (!ret)
Regards,
Felix
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_device.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c
b/drivers/gpu/drm/ttm/ttm_device.c
index a0562ab386f5..e7147e304637 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -156,8 +156,12 @@ int ttm_device_swapout(struct ttm_device *bdev,
struct ttm_operation_ctx *ctx,
ttm_resource_manager_for_each_res(man, , res) {
struct ttm_buffer_object *bo = res->bo;
- uint32_t num_pages = PFN_UP(bo->base.size);
+ uint32_t num_pages;
+ if (!bo)
+ continue;
+
+ num_pages = PFN_UP(bo->base.size);
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the lru_lock */
if (!ret)
Re: [PATCH v2] drm/sun4i: Enable output signal premultiplication for DE2/DE3
Hi,
вс, 5 июн. 2022 г. в 23:23, Jernej Škrabec :
>
> Dne nedelja, 05. junij 2022 ob 11:40:18 CEST je Roman Stratiienko napisal(a):
> > Otherwise alpha value is discarded, resulting incorrect pixel
> > apperance on the display.
> >
> > This also fixes missing transparency for the most bottom layer.
>
> Can you explain that a bit more?
Well... I would recommend reading Bartosz Ciechanowski's blog
https://ciechanow.ski/alpha-compositing/ or the Porter-Duff's 1984
whitepaper itself.
HINT: That magic numbers from sun8i_mixer.h ( 0x03010301 ) corresponds
to SOURCE OVER mode.
As you can see from the blending equation it outputs both pixel value
and alpha value (non-premultiplied data mode).
Also single-layer non-premultiplied buffers may have for example
(R255,G255,B255,A2) pixel value, which should be sent as {R2, G2, B2}
through the physical display interface.
When OUTCTL.PREMULTI disabled pixel, the RGB values passes as is, and
even 100% transparent data {R255, G255, B255, A0} will appear as 100%
opaque white.
>Also, BSP driver never enables this bit. What are we doing differently?
Are you sure the BSP does not have an issues with presenting
transparent buffers?
Does the sunxi even have a customer-feedback mechanism and publicly
available stable BSP with all the fixes?
Regards,
Roman
>
> >
> > Test applications and videos w/ w/o this patch are available at [1].
> >
> > [1]: https://github.com/GloDroid/glodroid_tests/issues/1
>
> As stated in other emails, commit messages should not contain external links
> (per patch rules).
>
> Best regards,
> Jernej
>
> > Signed-off-by: Roman Stratiienko
> >
> > ---
> > Changelog:
> >
> > V2: Added code hunk missing in v1
> > ---
> > drivers/gpu/drm/sun4i/sun8i_mixer.c | 5 +++--
> > drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 +
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 6b1711a9a71f..ba2932aaed08
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -320,8 +320,9 @@ static void sun8i_mixer_mode_set(struct sunxi_engine
> > *engine, else
> > val = 0;
> >
> > - regmap_update_bits(engine->regs,
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> val);
> > + val |= SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY;
> > +
> > + regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> val);
> >
> > DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> >interlaced ? "on" : "off");
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..bc12c95af6f3
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > @@ -65,6 +65,7 @@
> > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n) (0xf << ((n) << 2))
> > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2)
> >
> > +#define SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY BIT(0)
> > #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED BIT(1)
> >
> > #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch)
>
>
>
>
Re: [RFC PATCH 3/5] drm/amdgpu: Allow explicit sync for VM ops.
Am 03.06.22 um 21:11 schrieb Bas Nieuwenhuizen: On Fri, Jun 3, 2022 at 8:41 PM Christian König wrote: Am 03.06.22 um 19:50 schrieb Bas Nieuwenhuizen: [SNIP] Yeah, but that's exactly the bubble we try to avoid. Isn't it? For this series, not really. To clarify there are two sides for getting GPU bubbles and no overlap: (1) VM operations implicitly wait for earlier CS submissions (2) CS submissions implicitly wait for earlier VM operations Together, these combine to ensure that you get a (potentially small) bubble any time VM work happens. Your series (and further ideas) tackles (2), and is a worthwhile thing to do. However, while writing the userspace for this I noticed this isn't enough to get rid of all our GPU bubbles. In particular when doing a non-sparse map of a new BO, that tends to need to be waited on for the next CS anyway for API semantics. Due to VM operations happening on a single timeline that means this high priority map can end up being blocked by earlier sparse maps and hence the bubble in that case still exists. So in this series I try to tackle (1) instead. Since GPU work typically lags behind CPU submissions and VM operations aren't that slow, we can typically execute VM operations early enough that any implicit syncs from (2) are less/no issue. Ok, once more since you don't seem to understand what I want to say: It isn't possible to fix #1 before you have fixed #2. The VM unmap operation here is a barrier which divides the CS operations in a before and after. This is intentional design. Why is that barrier needed? The two barriers I got and understood and I think we can deal with: 1) the VM unmap is a barrier between prior CS and later memory free. 2) The TLB flush need to happen between a VM unmap and later CS. But why do we need the VM unmap to be a strict barrier between prior CS and later CS? Exactly because of the two reasons you mentioned. This is the part I'm not seeing. I get that removing #2 is a nightmare, which is why I did something that doesn't violate that constraint. Like if an explicit CS that was running before the VM operation runs till after the VM operation (and hence possibly till after the TLB flush, or otherwise have the TLB flush not apply due to lack of async TLB flush support), that is not an issue. It might see the state from before the unmap, or after the unmap, or some intermediate state and all of those would be okay. We still get the constraint that the TLB flush happens between the VM unmap and later CS and hence the unmap is certainly visible to them. So you basically just want to set the sync mode in amdgpu_vm_update_range() to AMDGPU_SYNC_EXPLICIT even when it is an unmap? That should be doable, but then you don't need any of the other changes. Regards, Christian. #1 Is rather easy to fix, you just need to copy all dma_fences from the page table dma_resv object over to the BOs dma_resv object in the gem close handler. E.g. exactly what you suggested with the dma_resv_copy function. #2 is a nightmare. We can't move the TLB flush at the end of the unmap operation because on async TLB flushes are either a bit complicated (double flushes etc..) or don't even work at all because of hw bugs. So to have a reliable TLB flush we must make sure that nothing else is ongoing and that means CS->VM->CS barrier. We try very hard to circumvent that already on maps by (for example) using a completely new VMID for CS after the VM map operation. But for the unmap operation we would need some kind special dma_fence implementation which would not only wait for all existing dma_fence but also for the one added until the unmap operation is completed. Cause otherwise our operation we do at #1 would simply not catch all dma_fences which have access to the memory. That's certainly doable, but I think just using the drm_exec stuff I already came up with is easier. When we can grab locks for all the BOs involved amdgpu_vm_clear_freed() goes away and we can keep track of the unmap operations in the bo_va structure. With that done you can make the explicit sync you noted in the bo_va structure and implicit sync when the bo_va structure goes away. Then the only reason I can see why we would need a CS->VM dependency is implicit synchronization, and that's what we are trying to avoid here in the first place. Regards, Christian. To get rid of this barrier you must first fix the part where CS submissions wait for the VM operation to complete, e.g. the necessity of the barrier. I'm working on this for a couple of years now and I'm really running out of idea how to explain this restriction. Regards, Christian.
[PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Test the conversion from XRGB to RGB332.
What is tested?
- Different values for the X in XRGB to make sure it is ignored
- Different clip values: Single pixel and full and partial buffer
- Well known colors: White, black, red, green, blue, magenta, yellow
and cyan
- Other colors: Randomly picked
- Destination pitch
How to run the tests?
$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \
--kconfig_add CONFIG_VIRTIO_UML=y \
--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
Suggested-by: Javier Martinez Canillas
Signed-off-by: José Expósito
---
RFC -> v1:
https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposit...@gmail.com/T/
- Add .kunitconfig (Maxime Ripard)
- Fix memory leak (Daniel Latypov)
- Make config option generic (Javier Martinez Canillas):
DRM_FORMAR_HELPER_TEST -> DRM_KUNIT_TEST
- Remove DISABLE_STRUCTLEAK_PLUGIN (Daniel Latypov)
---
drivers/gpu/drm/.kunitconfig | 3 +
drivers/gpu/drm/Kconfig | 16 +++
drivers/gpu/drm/Makefile | 2 +
drivers/gpu/drm/drm_format_helper_test.c | 166 +++
4 files changed, 187 insertions(+)
create mode 100644 drivers/gpu/drm/.kunitconfig
create mode 100644 drivers/gpu/drm/drm_format_helper_test.c
diff --git a/drivers/gpu/drm/.kunitconfig b/drivers/gpu/drm/.kunitconfig
new file mode 100644
index ..6ec04b4c979d
--- /dev/null
+++ b/drivers/gpu/drm/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_DRM=y
+CONFIG_DRM_KUNIT_TEST=y
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e88c497fa010..3c0b1faba439 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -70,6 +70,22 @@ config DRM_DEBUG_SELFTEST
If in doubt, say "N".
+config DRM_KUNIT_TEST
+ tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
+ depends on DRM && KUNIT=y
+ select DRM_KMS_HELPER
+ default KUNIT_ALL_TESTS
+ help
+ This builds unit tests for DRM. This option is not useful for
+ distributions or general kernels, but only for kernel
+ developers working on DRM and associated drivers.
+
+ For more information on KUnit and unit tests in general,
+ please refer to the KUnit documentation in
+ Documentation/dev-tools/kunit/.
+
+ If in doubt, say "N".
+
config DRM_KMS_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 15fe3163f822..6549471f09c7 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -76,6 +76,8 @@ obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
#
obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
+obj-$(CONFIG_DRM_KUNIT_TEST) += drm_kms_helper.o \
+ drm_format_helper_test.o
obj-$(CONFIG_DRM_MIPI_DBI) += drm_mipi_dbi.o
obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
diff --git a/drivers/gpu/drm/drm_format_helper_test.c
b/drivers/gpu/drm/drm_format_helper_test.c
new file mode 100644
index ..e9302219f3f9
--- /dev/null
+++ b/drivers/gpu/drm/drm_format_helper_test.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "drm_crtc_internal.h"
+
+#define TEST_BUF_SIZE 50
+#define CLIP(x, y, w, h) { (x), (y), (x) + (w), (y) + (h) }
+
+struct xrgb_to_rgb332_case {
+ const char *name;
+ unsigned int pitch;
+ unsigned int dst_pitch;
+ struct drm_rect clip;
+ const u32 xrgb[TEST_BUF_SIZE];
+ const u8 expected[4 * TEST_BUF_SIZE];
+};
+
+static struct xrgb_to_rgb332_case xrgb_to_rgb332_cases[] = {
+ {
+ .name = "Single pixel source",
+ .pitch = 1 * 4,
+ .dst_pitch = 0,
+ .clip = CLIP(0, 0, 1, 1),
+ .xrgb = { 0x01FF },
+ .expected = { 0xE0 },
+ },
+ {
+ .name = "Single pixel clip",
+ .pitch = 2 * 4,
+ .dst_pitch = 0,
+ .clip = CLIP(1, 1, 1, 1),
+ .xrgb = {
+ 0x, 0x,
+ 0x, 0x10FF,
+ },
+ .expected = { 0xE0 },
+ },
+ {
+ .name = "White, black, red, green, blue, magenta, yellow, cyan",
+ .pitch = 4 * 4,
+ .dst_pitch = 0,
+ .clip = CLIP(1, 1, 2, 4),
+ .xrgb = {
+ 0x, 0x, 0x, 0x,
+ 0x, 0x11FF, 0x2200, 0x,
+ 0x, 0x33FF, 0x4400FF00, 0x,
+ 0x, 0x55FF, 0x66FF00FF, 0x,
+ 0x, 0x7700, 0x8800, 0x,
+ },
+ .expected = {
+
[PATCH 0/1] KUnit tests for drm_format_helper
Hello everyone, Recently Javier added a new task in the ToDo list [1] to create KUnit tests for the functions present in "drm_format_helper". This patch includes the changes suggested in the RFC version [2]. Best wishes, José Expósito [1] https://cgit.freedesktop.org/drm/drm-misc/commit/?id=596c35b1440e [2] https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposit...@gmail.com/T/ José Expósito (1): drm/format-helper: Add KUnit tests for drm_fb_xrgb_to_rgb332() drivers/gpu/drm/.kunitconfig | 3 + drivers/gpu/drm/Kconfig | 16 +++ drivers/gpu/drm/Makefile | 2 + drivers/gpu/drm/drm_format_helper_test.c | 166 +++ 4 files changed, 187 insertions(+) create mode 100644 drivers/gpu/drm/.kunitconfig create mode 100644 drivers/gpu/drm/drm_format_helper_test.c -- 2.25.1
Re: [RFC PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Hello everyone, On Thu, Jun 02, 2022 at 07:21:28PM +0200, Javier Martinez Canillas wrote: > Hello David, > > On 6/2/22 19:07, David Gow wrote: > > On Thu, Jun 2, 2022 at 9:27 AM Javier Martinez Canillas > > [snip] > > >> > >> And doing that will also allow you to get rid of this, since just selecting > >> CONFIG_DRM_KUNIT_TEST=y would be enough for the tests built and run by > >> KUnit. > >> > > > > This is definitely something other KUnit tests (apparmor, elf, etc) > > are doing, and it's generally fine. You do lose the ability to build > > the tests as a separate module, though. (This is not usually a big > > problem, but there are some cases where it's useful.) > > > > That being said, I don't think 'select' is enough of a problem that > > you should feel the need to refactor in this way just to avoid it. > > Oh, yes I didn't want to imply that this was the main reason but just > pointed out that wouldn't even be needed if done that way. And it is > something that we want to do anyway IMO, since as mentioned it would > allow to test the static functions, which are the majority the format > helpers in that file. Conversion functions alway call drm_fb_xfrm()/drm_fb_xfrm_toio() and their *_line function. For example, drm_fb_xrgb_to_rgb332() calls drm_fb_xfrm() and drm_fb_xrgb_to_rgb332_line(). The current tests already check that the *_line() function works as expected. I'd like to test the high-level functions first and, if required, go into more detail in the future. The refactor is pretty easy, so I'd prefer to keep it as it is for the moment. About the other changes suggested, I applied all of them over the weekend. I'll send v1 of the patch to the mailing list including them so we have an up to date code to comment on. Thanks a lot for all of your comments and help, José Expósito
Re: [PATCH v3 8/8] drm/mediatek: Config orientation property if panel provides it
Il 06/06/22 06:47, Hsin-Yi Wang ha scritto: Panel orientation property should be set before drm_dev_register(). Mediatek drm driver calls drm_dev_register() in .bind(). However, most panels sets orientation property relatively late, mostly in .get_modes() callback, since this is when they are able to get the connector and binds the orientation property to it, though the value should be known when the panel is probed. Let the drm driver check if the remote end point is a panel and if it contains the orientation property. If it does, set it before drm_dev_register() is called. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede Reviewed-by: AngeloGioacchino Del Regno
Re: (subset) [PATCH 3/3] ARM: dts: exynos: add panel and backlight to p4note
On Mon, 16 May 2022 21:37:09 +0200, Martin Jücker wrote: > Add configuration for the LTL101AL01 panel and a pwm backlight to drive > the display in the p4note devices. > > Applied, thanks! [3/3] ARM: dts: exynos: add panel and backlight to p4note https://git.kernel.org/krzk/linux/c/6c52573bf4c3a0f6e7142264fb36b31ae2c3707a Best regards, -- Krzysztof Kozlowski
Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
On Mon, Jun 06, 2022 at 11:20:06AM +0200, Maxime Ripard wrote: > On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote: > > Hello Jernej, > > > > Thank you for having a look. > > > > вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec : > > > > > > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko > > > napisal(a): > > > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes: > > > > "None", "Pre-multiplied", "Coverage" > > > > > > > > Add the blend mode property and route it to the appropriate registers. > > > > > > > > Note: > > > > "force_premulti" parameter was added to handle multi-overlay channel > > > > cases in future changes. It must be set to true for cases when more > > > > than 1 overlay layer is used within a channel and at least one of the > > > > overlay layers within a group uses premultiplied blending mode. > > > > > > Please remove this parameter. It's nothing special, so it can be easily > > > added > > > once it's actually needed. For now, it only complicates code. > > > > I would prefer keeping it if you do not have any strong opinion against it. > > > > I am working now on exposing all overlays, so it will be needed soon anyway. > > KMS assumes pre-multiplied alpha anyway, so we shouldn't need a > parameter to force it: we're always going to force it. My bad, I got confused with your other patch. Still, I agree with Jernej, if it's not needed it only complicates the code for no particular reason. If you need it at some point in the future, add it then. Otherwise, it's hard to reason about, since we don't know what are the expectations that those future patches will bring. Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote: > Hello Jernej, > > Thank you for having a look. > > вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec : > > > > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko > > napisal(a): > > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes: > > > "None", "Pre-multiplied", "Coverage" > > > > > > Add the blend mode property and route it to the appropriate registers. > > > > > > Note: > > > "force_premulti" parameter was added to handle multi-overlay channel > > > cases in future changes. It must be set to true for cases when more > > > than 1 overlay layer is used within a channel and at least one of the > > > overlay layers within a group uses premultiplied blending mode. > > > > Please remove this parameter. It's nothing special, so it can be easily > > added > > once it's actually needed. For now, it only complicates code. > > I would prefer keeping it if you do not have any strong opinion against it. > > I am working now on exposing all overlays, so it will be needed soon anyway. KMS assumes pre-multiplied alpha anyway, so we shouldn't need a parameter to force it: we're always going to force it. > Also it helps to better understand the COV2PREMULT mode which has not > the best description in the datasheet. Only after testing this > register using devmem I became confident on its purpose. Sounds like a good job for a comment in the source code. Maxime signature.asc Description: PGP signature
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Fri, 3 Jun 2022 14:38:54 + Zack Rusin wrote: > > On Jun 3, 2022, at 10:32 AM, Simon Ser wrote: > > > > ⚠ External Email > > > > On Friday, June 3rd, 2022 at 16:27, Zack Rusin wrote: > > > >>> In particular: since the driver will ignore the KMS cursor plane > >>> position set by user-space, I don't think it's okay to just expose > >>> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP). > >>> > >>> cc wayland-devel and Pekka for user-space feedback. > >> > >> I think Thomas expressed our concerns and reasons why those wouldn’t > >> work for us back then. Is there something else you’d like to add? > > > > I disagreed, and I don't understand Thomas' reasoning. > > > > KMS clients will need an update to work correctly. Adding a new prop > > without a cap leaves existing KMS clients broken. Adding a cap allows > > both existing KMS clients and updated KMS clients to work correctly. > > I’m not sure what you mean here. They are broken right now. That’s > what we’re fixing. That’s the reason why the virtualized drivers are > on deny-lists for all atomic kms. So nothing needs to be updated. If Hi Zack, please, stop removing all email quoting level indicators, you have been doing that a lot in your more recent replies. It makes reading the emails really difficult, and I had to stop trying to make sense of the latest emails. It is really unfortunate that display servers have driver deny-lists indeed. All the bug reports they got from being broken should have been denied and forwarded to the respective drivers instead for breaking the KMS UAPI. OTOH, I understand that some userspace projects do not want to stop because of few broken but popular drivers. > you have a kms client that was using virtualized drivers with atomic > kms then mouse clicks never worked correctly. So, yes, clients need > to set cursor hotspot if they want mouse to work correctly with > virtualized drivers. I'm very much agreeing with Simon. He has made similar questions and comments that occurred to me. Reading as much of the discussion between Simon and Zack as I could, it seems every time Simon gets to the point, Zack hops to a completely different use case to make Simon's argument no longer apply. Like, root-window-less per-window remoting through KMS? How is that even possible? > >>> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin z...@kde.org wrote: > >>> > - all userspace code needs to hardcore a list of drivers which require > hotspots because there's no way to query from drm "does this driver > require hotspot" > >>> > >>> Can you elaborate? I'm not sure I understand what you mean here. > >> > >> Only some drivers require informations about cursor hotspot, user space > >> currently has no way of figuring out which ones are those, i.e. amdgpu > >> doesn’t care about hotspots, qxl does so when running on qxl without > >> properly set cursor hotspot atomic kms will result in a desktop where > >> mouse clicks have incorrect coordinates. > >> > >> There’s no way to differentiate between drivers that do not care about > >> cursor hotspots and ones that absolutely require it. > > > > Only VM drivers should expose the hotspot properties. Real drivers like > > amdgpu must not expose it. > > Yes, that’s what I said. There’s no way to differentiate between > amdgpu that doesn’t have those properties and virtio_gpu driver from > a kernel before hotspot properties were added. Why would userspace want to tell the difference between a driver that truly does not need those properties, and a driver that did not add those properties *yet*? Thanks, pq pgpVkyvtF0gw0.pgp Description: OpenPGP digital signature
Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
Hello Jernej,
Thank you for having a look.
вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec :
>
> Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > "None", "Pre-multiplied", "Coverage"
> >
> > Add the blend mode property and route it to the appropriate registers.
> >
> > Note:
> > "force_premulti" parameter was added to handle multi-overlay channel
> > cases in future changes. It must be set to true for cases when more
> > than 1 overlay layer is used within a channel and at least one of the
> > overlay layers within a group uses premultiplied blending mode.
>
> Please remove this parameter. It's nothing special, so it can be easily added
> once it's actually needed. For now, it only complicates code.
I would prefer keeping it if you do not have any strong opinion against it.
I am working now on exposing all overlays, so it will be needed soon anyway.
Also it helps to better understand the COV2PREMULT mode which has not
the best description in the datasheet. Only after testing this
register using devmem I became confident on its purpose.
>
> >
> > Test:
> > Manually tested all the modes using kmsxx python wrapper with and
> > without 'force_premulti' flag enabled.
> >
> > Signed-off-by: Roman Stratiienko
> > ---
> > drivers/gpu/drm/sun4i/sun8i_mixer.h| 2 ++
> > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 -
> > drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++
> > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++
> > drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++
> > 5 files changed, 94 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > @@ -65,6 +65,8 @@
> > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n) (0xf << ((n) << 2))
> > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2)
> >
> > +#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe) BIT(pipe)
> > +
> > #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED BIT(1)
> >
> > #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch)
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 6ccbbca3176d..29c0d9cca19a
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -58,24 +58,46 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer
> > *mixer, int channel, }
> >
> > static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int
> > channel, -int overlay, struct
> drm_plane *plane)
> > + int overlay, struct
> drm_plane *plane,
> > + unsigned int zpos,
> bool force_premulti)
> > {
> > - u32 mask, val, ch_base;
> > + u32 mask, val, ch_base, bld_base;
> > + bool in_premulti, out_premulti;
> >
> > + bld_base = sun8i_blender_base(mixer);
> > ch_base = sun8i_channel_base(mixer, channel);
> >
> > + in_premulti = plane->state->pixel_blend_mode ==
> DRM_MODE_BLEND_PREMULTI;
> > + out_premulti = force_premulti || in_premulti;
> > +
> > mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
> > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK;
> >
> > val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >>
> 8);
> >
> > - val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> > + if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
> > + } else {
> > + val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE)
> ?
> > +
> SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> > +
> SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> > +
> > + if (in_premulti)
> > + val |=
> SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
> > + }
> > +
> > + if (!in_premulti && out_premulti)
> > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM;
> >
> > regmap_update_bits(mixer->engine.regs,
> > SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base,
> overlay),
> > mask, val);
> > +
> > + regmap_update_bits(
> > + mixer->engine.regs,
> SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
> > + SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
> > + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
> 0);
> > }
> >
> > static int
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Sunday, June 5th, 2022 at 20:16, Zack Rusin wrote: > > At any rate, I consider broken any driver which exposes a cursor plane, > > then doesn't display it exactly at the CRTC_X/CRTC_Y > > But we do… The cursor is at crtc_x, crtc_y. How do you show the cursor on the host side then? Pretty sure you use the host X11/Wayland cursor? In which case nothing guarantees that the host cursor position matches CRTC_X/CRTC_Y.
[PATCH] drm/bridge: it6505: Power off downstream device in .atomic_enable
Power off the downstream device in .atomic_enable callback, so the
external display shows up again after changing resolution.
Fixes: 46ca7da7f1e8 ("drm/bridge: it6505: Send DPCD SET_POWER to downstream")
Signed-off-by: Pin-Yen Lin
---
drivers/gpu/drm/bridge/ite-it6505.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 4b673c4792d7..e5626035f311 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2945,6 +2945,9 @@ static void it6505_bridge_atomic_enable(struct drm_bridge
*bridge,
if (ret)
dev_err(dev, "Failed to setup AVI infoframe: %d", ret);
+ it6505_drm_dp_link_set_power(>aux, >link,
+DP_SET_POWER_D0);
+
it6505_update_video_parameter(it6505, mode);
ret = it6505_send_video_infoframe(it6505, );
--
2.36.1.255.ge46751e96f-goog
