Re: [PATCH 01/10] Revert "drm/msm: Add missing check and destroy for alloc_ordered_workqueue"
On Wed, Mar 08, 2023 at 10:10:24AM +0800, Jiasheng Jiang wrote: > On Mon, 06 Mar 2023 18:07:13 +0800, Johan Hovold wrote: > > This reverts commit 643b7d0869cc7f1f7a5ac7ca6bd25d88f54e31d0. > > The commit not only adds the allocation sanity check, but also adds the > destroy_workqueue to release the allocated priv->wq. > Therefore, revert the commit will cause memory leak. No, reverting this commit does not cause any memory leaks (look at at diff again). The original patch called msm_drm_uninit() in some early error paths, but that was just completely broken as that function must not be called before the subcomponents have been bound and also triggered a bunch of other NULL-pointer dereferences. That bit was however removed when the change was merged with a second branch that also touched these error paths. In the end, the leaked wq is still there and this commit only added broken error handling for the wq allocation failing (as it does not free the drm device). > > A recent patch that tried to fix up the msm_drm_init() paths with > > respect to the workqueue but only ended up making things worse: > > > > First, the newly added calls to msm_drm_uninit() on early errors would > > trigger NULL-pointer dereferences, for example, as the kms pointer would > > not have been initialised. (Note that these paths were also modified by > > a second broken error handling patch which in effect cancelled out this > > part when merged.) > > There is a check for the kms pointer to avoid NULL-pointer dereference in > the msm_drm_uninit(). No, there were further places in msm_drm_uninit() which did not have any such checks when you submitted your patch. Some of the missing checks were added by a separate patch, but that would not in itself have been sufficient as with your patch you'd still end up trying to unbind the subcomponents before they are bound, which will lead to further crashes. > > Second, the newly added allocation sanity check would still leak the > > previously allocated drm device. > > The ddev is allocated by drm_dev_alloc which support automatic cleanup. We don't have automatic garbage collection in the kernel. You still need to release the reference to the device for it to be freed. Johan
[PATCH 11/13] backlight: qcom-wled: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/qcom-wled.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/qcom-wled.c
b/drivers/video/backlight/qcom-wled.c
index 527210e85795..8d88f36d8ff3 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1717,7 +1717,7 @@ static int wled_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(bl);
};
-static int wled_remove(struct platform_device *pdev)
+static void wled_remove(struct platform_device *pdev)
{
struct wled *wled = platform_get_drvdata(pdev);
@@ -1725,8 +1725,6 @@ static int wled_remove(struct platform_device *pdev)
cancel_delayed_work_sync(>ovp_work);
disable_irq(wled->short_irq);
disable_irq(wled->ovp_irq);
-
- return 0;
}
static const struct of_device_id wled_match_table[] = {
@@ -1742,7 +1740,7 @@ MODULE_DEVICE_TABLE(of, wled_match_table);
static struct platform_driver wled_driver = {
.probe = wled_probe,
- .remove = wled_remove,
+ .remove_new = wled_remove,
.driver = {
.name = "qcom,wled",
.of_match_table = wled_match_table,
--
2.39.1
[PATCH 09/13] backlight: mt6370-backlight: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/mt6370-backlight.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/mt6370-backlight.c
b/drivers/video/backlight/mt6370-backlight.c
index 623d4f2baca2..94422c956453 100644
--- a/drivers/video/backlight/mt6370-backlight.c
+++ b/drivers/video/backlight/mt6370-backlight.c
@@ -318,15 +318,13 @@ static int mt6370_bl_probe(struct platform_device *pdev)
return 0;
}
-static int mt6370_bl_remove(struct platform_device *pdev)
+static void mt6370_bl_remove(struct platform_device *pdev)
{
struct mt6370_priv *priv = platform_get_drvdata(pdev);
struct backlight_device *bl_dev = priv->bl;
bl_dev->props.brightness = 0;
backlight_update_status(priv->bl);
-
- return 0;
}
static const struct of_device_id mt6370_bl_of_match[] = {
@@ -342,7 +340,7 @@ static struct platform_driver mt6370_bl_driver = {
.of_match_table = mt6370_bl_of_match,
},
.probe = mt6370_bl_probe,
- .remove = mt6370_bl_remove,
+ .remove_new = mt6370_bl_remove,
};
module_platform_driver(mt6370_bl_driver);
--
2.39.1
[PATCH 00/13] backlight: Convert to platform remove callback returning void
Hello,
this patch series adapts the platform drivers below drivers/video/backlight to
use the .remove_new() callback. Compared to the traditional .remove() callback
.remove_new() returns no value. This is a good thing because the driver core
doesn't (and cannot) cope for errors during remove. The only effect of a
non-zero return value in .remove() is that the driver core emits a warning. The
device is removed anyhow and an early return from .remove() usually yields a
resource leak.
By changing the remove callback to return void driver authors cannot
reasonably assume any more that there is some kind of cleanup later.
All drivers in drivers/video/backlight returned zero unconditionally in their
remove callback, so they could all be converted trivially to .remove_new().
Note that this series depends on commit 5c5a7680e67b ("platform: Provide
a remove callback that returns no value") which is included in v6.3-rc1.
Best regards
Uwe
Uwe Kleine-König (13):
backlight: aat2870_bl: Convert to platform remove callback returning
void
backlight: adp5520_bl: Convert to platform remove callback returning
void
backlight: cr_bllcd: Convert to platform remove callback returning
void
backlight: da9052_bl: Convert to platform remove callback returning
void
backlight: hp680_bl: Convert to platform remove callback returning
void
backlight: led_bl: Convert to platform remove callback returning void
backlight: lm3533_bl: Convert to platform remove callback returning
void
backlight: lp8788_bl: Convert to platform remove callback returning
void
backlight: mt6370-backlight: Convert to platform remove callback
returning void
backlight: pwm_bl: Convert to platform remove callback returning void
backlight: qcom-wled: Convert to platform remove callback returning
void
backlight: rt4831-backlight: Convert to platform remove callback
returning void
backlight: sky81452-backlight: Convert to platform remove callback
returning void
drivers/video/backlight/aat2870_bl.c | 6 ++
drivers/video/backlight/adp5520_bl.c | 6 ++
drivers/video/backlight/cr_bllcd.c | 6 ++
drivers/video/backlight/da9052_bl.c | 6 ++
drivers/video/backlight/hp680_bl.c | 6 ++
drivers/video/backlight/led_bl.c | 6 ++
drivers/video/backlight/lm3533_bl.c | 6 ++
drivers/video/backlight/lp8788_bl.c | 6 ++
drivers/video/backlight/mt6370-backlight.c | 6 ++
drivers/video/backlight/pwm_bl.c | 6 ++
drivers/video/backlight/qcom-wled.c | 6 ++
drivers/video/backlight/rt4831-backlight.c | 6 ++
drivers/video/backlight/sky81452-backlight.c | 6 ++
13 files changed, 26 insertions(+), 52 deletions(-)
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
--
2.39.1
[PATCH 04/13] backlight: da9052_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/da9052_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/da9052_bl.c
b/drivers/video/backlight/da9052_bl.c
index 882359dd288c..1cdc8543310b 100644
--- a/drivers/video/backlight/da9052_bl.c
+++ b/drivers/video/backlight/da9052_bl.c
@@ -135,7 +135,7 @@ static int da9052_backlight_probe(struct platform_device
*pdev)
return da9052_adjust_wled_brightness(wleds);
}
-static int da9052_backlight_remove(struct platform_device *pdev)
+static void da9052_backlight_remove(struct platform_device *pdev)
{
struct backlight_device *bl = platform_get_drvdata(pdev);
struct da9052_bl *wleds = bl_get_data(bl);
@@ -143,8 +143,6 @@ static int da9052_backlight_remove(struct platform_device
*pdev)
wleds->brightness = 0;
wleds->state = DA9052_WLEDS_OFF;
da9052_adjust_wled_brightness(wleds);
-
- return 0;
}
static const struct platform_device_id da9052_wled_ids[] = {
@@ -166,7 +164,7 @@ MODULE_DEVICE_TABLE(platform, da9052_wled_ids);
static struct platform_driver da9052_wled_driver = {
.probe = da9052_backlight_probe,
- .remove = da9052_backlight_remove,
+ .remove_new = da9052_backlight_remove,
.id_table = da9052_wled_ids,
.driver = {
.name = "da9052-wled",
--
2.39.1
[PATCH 10/13] backlight: pwm_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/pwm_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index fb388148d98f..fce412234d10 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -625,7 +625,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
return ret;
}
-static int pwm_backlight_remove(struct platform_device *pdev)
+static void pwm_backlight_remove(struct platform_device *pdev)
{
struct backlight_device *bl = platform_get_drvdata(pdev);
struct pwm_bl_data *pb = bl_get_data(bl);
@@ -635,8 +635,6 @@ static int pwm_backlight_remove(struct platform_device
*pdev)
if (pb->exit)
pb->exit(>dev);
-
- return 0;
}
static void pwm_backlight_shutdown(struct platform_device *pdev)
@@ -690,7 +688,7 @@ static struct platform_driver pwm_backlight_driver = {
.of_match_table = of_match_ptr(pwm_backlight_of_match),
},
.probe = pwm_backlight_probe,
- .remove = pwm_backlight_remove,
+ .remove_new = pwm_backlight_remove,
.shutdown = pwm_backlight_shutdown,
};
--
2.39.1
[PATCH 08/13] backlight: lp8788_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/lp8788_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/lp8788_bl.c
b/drivers/video/backlight/lp8788_bl.c
index ba42f3fe0c73..d1a14b0db265 100644
--- a/drivers/video/backlight/lp8788_bl.c
+++ b/drivers/video/backlight/lp8788_bl.c
@@ -298,7 +298,7 @@ static int lp8788_backlight_probe(struct platform_device
*pdev)
return ret;
}
-static int lp8788_backlight_remove(struct platform_device *pdev)
+static void lp8788_backlight_remove(struct platform_device *pdev)
{
struct lp8788_bl *bl = platform_get_drvdata(pdev);
struct backlight_device *bl_dev = bl->bl_dev;
@@ -307,13 +307,11 @@ static int lp8788_backlight_remove(struct platform_device
*pdev)
backlight_update_status(bl_dev);
sysfs_remove_group(>dev.kobj, _attr_group);
lp8788_backlight_unregister(bl);
-
- return 0;
}
static struct platform_driver lp8788_bl_driver = {
.probe = lp8788_backlight_probe,
- .remove = lp8788_backlight_remove,
+ .remove_new = lp8788_backlight_remove,
.driver = {
.name = LP8788_DEV_BACKLIGHT,
},
--
2.39.1
[PATCH 02/13] backlight: adp5520_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/adp5520_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/adp5520_bl.c
b/drivers/video/backlight/adp5520_bl.c
index 686988c3df3a..8e0e9cfe5fe9 100644
--- a/drivers/video/backlight/adp5520_bl.c
+++ b/drivers/video/backlight/adp5520_bl.c
@@ -337,7 +337,7 @@ static int adp5520_bl_probe(struct platform_device *pdev)
return 0;
}
-static int adp5520_bl_remove(struct platform_device *pdev)
+static void adp5520_bl_remove(struct platform_device *pdev)
{
struct backlight_device *bl = platform_get_drvdata(pdev);
struct adp5520_bl *data = bl_get_data(bl);
@@ -347,8 +347,6 @@ static int adp5520_bl_remove(struct platform_device *pdev)
if (data->pdata->en_ambl_sens)
sysfs_remove_group(>dev.kobj,
_bl_attr_group);
-
- return 0;
}
#ifdef CONFIG_PM_SLEEP
@@ -377,7 +375,7 @@ static struct platform_driver adp5520_bl_driver = {
.pm = _bl_pm_ops,
},
.probe = adp5520_bl_probe,
- .remove = adp5520_bl_remove,
+ .remove_new = adp5520_bl_remove,
};
module_platform_driver(adp5520_bl_driver);
--
2.39.1
[PATCH 12/13] backlight: rt4831-backlight: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/rt4831-backlight.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/rt4831-backlight.c
b/drivers/video/backlight/rt4831-backlight.c
index eb8c59e8713f..7d1af4c2ca67 100644
--- a/drivers/video/backlight/rt4831-backlight.c
+++ b/drivers/video/backlight/rt4831-backlight.c
@@ -203,15 +203,13 @@ static int rt4831_bl_probe(struct platform_device *pdev)
return 0;
}
-static int rt4831_bl_remove(struct platform_device *pdev)
+static void rt4831_bl_remove(struct platform_device *pdev)
{
struct rt4831_priv *priv = platform_get_drvdata(pdev);
struct backlight_device *bl_dev = priv->bl;
bl_dev->props.brightness = 0;
backlight_update_status(priv->bl);
-
- return 0;
}
static const struct of_device_id __maybe_unused rt4831_bl_of_match[] = {
@@ -226,7 +224,7 @@ static struct platform_driver rt4831_bl_driver = {
.of_match_table = rt4831_bl_of_match,
},
.probe = rt4831_bl_probe,
- .remove = rt4831_bl_remove,
+ .remove_new = rt4831_bl_remove,
};
module_platform_driver(rt4831_bl_driver);
--
2.39.1
[PATCH 13/13] backlight: sky81452-backlight: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/sky81452-backlight.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/sky81452-backlight.c
b/drivers/video/backlight/sky81452-backlight.c
index 0172438c38ce..eb18c6eb0ff0 100644
--- a/drivers/video/backlight/sky81452-backlight.c
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -311,7 +311,7 @@ static int sky81452_bl_probe(struct platform_device *pdev)
return ret;
}
-static int sky81452_bl_remove(struct platform_device *pdev)
+static void sky81452_bl_remove(struct platform_device *pdev)
{
const struct sky81452_bl_platform_data *pdata =
dev_get_platdata(>dev);
@@ -325,8 +325,6 @@ static int sky81452_bl_remove(struct platform_device *pdev)
if (pdata->gpiod_enable)
gpiod_set_value_cansleep(pdata->gpiod_enable, 0);
-
- return 0;
}
#ifdef CONFIG_OF
@@ -343,7 +341,7 @@ static struct platform_driver sky81452_bl_driver = {
.of_match_table = of_match_ptr(sky81452_bl_of_match),
},
.probe = sky81452_bl_probe,
- .remove = sky81452_bl_remove,
+ .remove_new = sky81452_bl_remove,
};
module_platform_driver(sky81452_bl_driver);
--
2.39.1
[PATCH 06/13] backlight: led_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/led_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index f54d256e2d54..a1b6a2ad73a0 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -217,7 +217,7 @@ static int led_bl_probe(struct platform_device *pdev)
return 0;
}
-static int led_bl_remove(struct platform_device *pdev)
+static void led_bl_remove(struct platform_device *pdev)
{
struct led_bl_data *priv = platform_get_drvdata(pdev);
struct backlight_device *bl = priv->bl_dev;
@@ -228,8 +228,6 @@ static int led_bl_remove(struct platform_device *pdev)
led_bl_power_off(priv);
for (i = 0; i < priv->nb_leds; i++)
led_sysfs_enable(priv->leds[i]);
-
- return 0;
}
static const struct of_device_id led_bl_of_match[] = {
@@ -245,7 +243,7 @@ static struct platform_driver led_bl_driver = {
.of_match_table = of_match_ptr(led_bl_of_match),
},
.probe = led_bl_probe,
- .remove = led_bl_remove,
+ .remove_new = led_bl_remove,
};
module_platform_driver(led_bl_driver);
--
2.39.1
[PATCH 01/13] backlight: aat2870_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/aat2870_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/aat2870_bl.c
b/drivers/video/backlight/aat2870_bl.c
index 1cbb303e9c88..81fde3abb92c 100644
--- a/drivers/video/backlight/aat2870_bl.c
+++ b/drivers/video/backlight/aat2870_bl.c
@@ -178,7 +178,7 @@ static int aat2870_bl_probe(struct platform_device *pdev)
return ret;
}
-static int aat2870_bl_remove(struct platform_device *pdev)
+static void aat2870_bl_remove(struct platform_device *pdev)
{
struct aat2870_bl_driver_data *aat2870_bl = platform_get_drvdata(pdev);
struct backlight_device *bd = aat2870_bl->bd;
@@ -186,8 +186,6 @@ static int aat2870_bl_remove(struct platform_device *pdev)
bd->props.power = FB_BLANK_POWERDOWN;
bd->props.brightness = 0;
backlight_update_status(bd);
-
- return 0;
}
static struct platform_driver aat2870_bl_driver = {
@@ -195,7 +193,7 @@ static struct platform_driver aat2870_bl_driver = {
.name = "aat2870-backlight",
},
.probe = aat2870_bl_probe,
- .remove = aat2870_bl_remove,
+ .remove_new = aat2870_bl_remove,
};
static int __init aat2870_bl_init(void)
--
2.39.1
[PATCH 05/13] backlight: hp680_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/hp680_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/hp680_bl.c
b/drivers/video/backlight/hp680_bl.c
index 9123c33def05..ddb7ab4df77e 100644
--- a/drivers/video/backlight/hp680_bl.c
+++ b/drivers/video/backlight/hp680_bl.c
@@ -119,20 +119,18 @@ static int hp680bl_probe(struct platform_device *pdev)
return 0;
}
-static int hp680bl_remove(struct platform_device *pdev)
+static void hp680bl_remove(struct platform_device *pdev)
{
struct backlight_device *bd = platform_get_drvdata(pdev);
bd->props.brightness = 0;
bd->props.power = 0;
hp680bl_send_intensity(bd);
-
- return 0;
}
static struct platform_driver hp680bl_driver = {
.probe = hp680bl_probe,
- .remove = hp680bl_remove,
+ .remove_new = hp680bl_remove,
.driver = {
.name = "hp680-bl",
.pm = _pm_ops,
--
2.39.1
[PATCH 07/13] backlight: lm3533_bl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/lm3533_bl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/lm3533_bl.c
b/drivers/video/backlight/lm3533_bl.c
index 1df1b6643c0b..3e10d480cb7f 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -337,7 +337,7 @@ static int lm3533_bl_probe(struct platform_device *pdev)
return ret;
}
-static int lm3533_bl_remove(struct platform_device *pdev)
+static void lm3533_bl_remove(struct platform_device *pdev)
{
struct lm3533_bl *bl = platform_get_drvdata(pdev);
struct backlight_device *bd = bl->bd;
@@ -349,8 +349,6 @@ static int lm3533_bl_remove(struct platform_device *pdev)
lm3533_ctrlbank_disable(>cb);
sysfs_remove_group(>dev.kobj, _bl_attribute_group);
-
- return 0;
}
#ifdef CONFIG_PM_SLEEP
@@ -390,7 +388,7 @@ static struct platform_driver lm3533_bl_driver = {
.pm = _bl_pm_ops,
},
.probe = lm3533_bl_probe,
- .remove = lm3533_bl_remove,
+ .remove_new = lm3533_bl_remove,
.shutdown = lm3533_bl_shutdown,
};
module_platform_driver(lm3533_bl_driver);
--
2.39.1
[PATCH 03/13] backlight: cr_bllcd: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/video/backlight/cr_bllcd.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/cr_bllcd.c
b/drivers/video/backlight/cr_bllcd.c
index 4ad0a72531fe..781aeecc451d 100644
--- a/drivers/video/backlight/cr_bllcd.c
+++ b/drivers/video/backlight/cr_bllcd.c
@@ -210,7 +210,7 @@ static int cr_backlight_probe(struct platform_device *pdev)
return 0;
}
-static int cr_backlight_remove(struct platform_device *pdev)
+static void cr_backlight_remove(struct platform_device *pdev)
{
struct cr_panel *crp = platform_get_drvdata(pdev);
@@ -220,13 +220,11 @@ static int cr_backlight_remove(struct platform_device
*pdev)
cr_backlight_set_intensity(crp->cr_backlight_device);
cr_lcd_set_power(crp->cr_lcd_device, FB_BLANK_POWERDOWN);
pci_dev_put(lpc_dev);
-
- return 0;
}
static struct platform_driver cr_backlight_driver = {
.probe = cr_backlight_probe,
- .remove = cr_backlight_remove,
+ .remove_new = cr_backlight_remove,
.driver = {
.name = "cr_backlight",
},
--
2.39.1
Re: [PATCH v13 01/10] device property: Add remote endpoint to devcon matcher
On Fri, Mar 03, 2023 at 10:33:41PM +0800, Pin-yen Lin wrote:
> From: Prashant Malani
>
> When searching the device graph for device matches, check the
> remote-endpoint itself for a match.
>
> Some drivers register devices for individual endpoints. This allows
> the matcher code to evaluate those for a match too, instead
> of only looking at the remote parent devices. This is required when a
> device supports two mode switches in its endpoints, so we can't simply
> register the mode switch with the parent node.
>
> Signed-off-by: Prashant Malani
> Signed-off-by: Pin-yen Lin
Reviewed-by: Heikki Krogerus
> ---
>
> Changes in v13:
> - Update the kernel doc of fwnode_connection_find_match
>
> Changes in v12:
> - Check the availability of the device node in fwnode_graph_devcon_matches
> - Ensured valid access to "matches" in fwnode_graph_devcon_matches
> - Updated the documentation in fwnode_connection_find_match(es)
> - Dropped collected tags due to the new changes
>
> Changes in v11:
> - Added missing fwnode_handle_put in drivers/base/property.c
>
> Changes in v10:
> - Collected Reviewed-by and Tested-by tags
>
> Changes in v6:
> - New in v6
>
> drivers/base/property.c | 31 ++-
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 083a95791d3b..4426ac2b16ca 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1243,6 +1243,23 @@ static unsigned int fwnode_graph_devcon_matches(const
> struct fwnode_handle *fwno
> continue;
> }
>
> + ret = match(node, con_id, data);
> + fwnode_handle_put(node);
> + if (ret) {
> + if (matches)
> + matches[count] = ret;
> + count++;
> +
> + if (matches && count >= matches_len)
> + break;
> + }
> +
> + /*
> + * Some drivers may register devices for endpoints. Check
> + * the remote-endpoints for matches in addition to the remote
> + * port parent.
> + */
> + node = fwnode_graph_get_remote_endpoint(ep);
> ret = match(node, con_id, data);
> fwnode_handle_put(node);
> if (ret) {
> @@ -1293,8 +1310,11 @@ static unsigned int fwnode_devcon_matches(const struct
> fwnode_handle *fwnode,
> * @match: Function to check and convert the connection description
> *
> * Find a connection with unique identifier @con_id between @fwnode and
> another
> - * device node. @match will be used to convert the connection description to
> - * data the caller is expecting to be returned.
> + * device node. For fwnode graph connections, the graph endpoints are also
> + * checked. @match will be used to convert the connection description to data
> + * the caller is expecting to be returned.
> + *
> + * Return: The pointer to the matched node, or NULL on error.
> */
> void *fwnode_connection_find_match(const struct fwnode_handle *fwnode,
> const char *con_id, void *data,
> @@ -1325,9 +1345,10 @@ EXPORT_SYMBOL_GPL(fwnode_connection_find_match);
> * @matches_len: Length of @matches
> *
> * Find up to @matches_len connections with unique identifier @con_id between
> - * @fwnode and other device nodes. @match will be used to convert the
> - * connection description to data the caller is expecting to be returned
> - * through the @matches array.
> + * @fwnode and other device nodes. For fwnode graph connections, the graph
> + * endpoints are also checked. @match will be used to convert the connection
> + * description to data the caller is expecting to be returned through the
> + * @matches array.
> * If @matches is NULL @matches_len is ignored and the total number of
> resolved
> * matches is returned.
> *
> --
> 2.40.0.rc0.216.gc4246ad0f0-goog
--
heikki
[PATCH] MAINTAINERS: orphan SIS FRAMEBUFFER DRIVER
This was triggered by the fact that the webpage: http://www.winischhofer.net/linuxsisvga.shtml cannot be reached anymore. Thomas Winischhofer is still reachable at the given email address, but he has not been active since 2005. Mark the SIS FRAMEBUFFER DRIVER as orphan to reflect the current state. Signed-off-by: Lukas Bulwahn --- MAINTAINERS | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5d8f46f35aa4..354577534aef 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19191,9 +19191,7 @@ W: http://www.brownhat.org/sis900.html F: drivers/net/ethernet/sis/sis900.* SIS FRAMEBUFFER DRIVER -M: Thomas Winischhofer -S: Maintained -W: http://www.winischhofer.net/linuxsisvga.shtml +S: Orphan F: Documentation/fb/sisfb.rst F: drivers/video/fbdev/sis/ F: include/video/sisfb.h -- 2.17.1
[PATCH] fbdev: Fix incorrect page mapping clearance at fb_deferred_io_release()
The recent fix for the deferred I/O by the commit
3efc61d95259 ("fbdev: Fix invalid page access after closing deferred I/O
devices")
caused a regression when the same fb device is opened/closed while
it's being used. It resulted in a frozen screen even if something
is redrawn there after the close. The breakage is because the patch
was made under a wrong assumption of a single open; in the current
code, fb_deferred_io_release() cleans up the page mapping of the
pageref list and it calls cancel_delayed_work_sync() unconditionally,
where both are no correct behavior for multiple opens.
This patch adds a refcount for the opens of the device, and applies
the cleanup only when all files get closed.
Fixes: 3efc61d95259 ("fbdev: Fix invalid page access after closing deferred I/O
devices")
Cc:
Signed-off-by: Takashi Iwai
---
drivers/video/fbdev/core/fb_defio.c | 16 +---
include/linux/fb.h | 1 +
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c
b/drivers/video/fbdev/core/fb_defio.c
index aa5f059d0222..9dcec9e020b6 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -305,17 +305,19 @@ void fb_deferred_io_open(struct fb_info *info,
struct inode *inode,
struct file *file)
{
+ struct fb_deferred_io *fbdefio = info->fbdefio;
+
file->f_mapping->a_ops = _deferred_io_aops;
+ fbdefio->opens++;
}
EXPORT_SYMBOL_GPL(fb_deferred_io_open);
-void fb_deferred_io_release(struct fb_info *info)
+static void fb_deferred_io_release_internal(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
struct page *page;
int i;
- BUG_ON(!fbdefio);
cancel_delayed_work_sync(>deferred_work);
/* clear out the mapping that we setup */
@@ -324,13 +326,21 @@ void fb_deferred_io_release(struct fb_info *info)
page->mapping = NULL;
}
}
+
+void fb_deferred_io_release(struct fb_info *info)
+{
+ struct fb_deferred_io *fbdefio = info->fbdefio;
+
+ if (!--fbdefio->opens)
+ fb_deferred_io_release_internal(info);
+}
EXPORT_SYMBOL_GPL(fb_deferred_io_release);
void fb_deferred_io_cleanup(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
- fb_deferred_io_release(info);
+ fb_deferred_io_release_internal(info);
kvfree(info->pagerefs);
mutex_destroy(>lock);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index d8d20514ea05..29674a29d1c4 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -212,6 +212,7 @@ struct fb_deferred_io {
/* delay between mkwrite and deferred handler */
unsigned long delay;
bool sort_pagereflist; /* sort pagelist by offset */
+ int opens; /* number of opened files */
struct mutex lock; /* mutex that protects the pageref list */
struct list_head pagereflist; /* list of pagerefs for touched pages */
/* callback */
--
2.35.3
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
Am 08.03.23 um 03:26 schrieb Steven Rostedt:
On Tue, 7 Mar 2023 21:22:23 -0500
Steven Rostedt wrote:
Looks like there was a lock possibly used after free. But as commit
9bff18d13473a9fdf81d5158248472a9d8ecf2bd ("drm/ttm: use per BO cleanup
workers") changed a lot of this code, I figured it may be the culprit.
If I bothered to look at the second warning after this one (I usually stop
after the first), it appears to state there was a use after free issue.
Yeah, that looks like the reference count was somehow messed up.
What test case/environment do you run to trigger this?
Thanks for the notice,
Christian.
[ 206.692285] [ cut here ]
[ 206.706333] refcount_t: underflow; use-after-free.
[ 206.720577] WARNING: CPU: 0 PID: 332 at lib/refcount.c:28
refcount_warn_saturate+0xb6/0xfc
[ 206.735810] Modules linked in:
[ 206.749493] CPU: 0 PID: 332 Comm: kworker/0:13H Tainted: GW
6.3.0-rc1-test-1-ga98bd42762ed-dirty #965
[ 206.765833] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.16.0-debian-1.16.0-5 04/01/2014
[ 206.781767] Workqueue: ttm ttm_bo_delayed_delete
[ 206.796500] EIP: refcount_warn_saturate+0xb6/0xfc
[ 206.811121] Code: 68 50 1c 0d cf e8 66 b3 a9 ff 0f 0b 58 c9 c3 90 80 3d 57 c6 38
cf 00 75 8a c6 05 57 c6 38 cf 01 68 7c 1c 0d cf e8 46 b3 a9 ff <0f> 0b 59 c9 c3
80 3d 55 c6 38 cf 00 0f 85 67 ff ff ff c6 05 55 c6
[ 206.844560] EAX: 0026 EBX: c2d5f150 ECX: c3ae5e40 EDX: 0002
[ 206.862109] ESI: c2d5f0bc EDI: f6f91200 EBP: c3ae5f18 ESP: c3ae5f14
[ 206.878773] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246
[ 206.895665] CR0: 80050033 CR2: ff9ff000 CR3: 0f512000 CR4: 00150ef0
[ 206.912303] Call Trace:
[ 206.927940] ttm_bo_delayed_delete+0x8c/0x94
[ 206.944179] process_one_work+0x21a/0x538
[ 206.960605] worker_thread+0x146/0x398
[ 206.976839] kthread+0xea/0x10c
[ 206.992696] ? process_one_work+0x538/0x538
[ 207.008827] ? kthread_complete_and_exit+0x1c/0x1c
[ 207.025150] ret_from_fork+0x1c/0x28
[ 207.041307] irq event stamp: 4219
[ 207.056883] hardirqs last enabled at (4219): []
_raw_spin_unlock_irqrestore+0x2d/0x58
[ 207.074298] hardirqs last disabled at (4218): []
kvfree_call_rcu+0x155/0x2ec
[ 207.091461] softirqs last enabled at (3570): []
__do_softirq+0x2f3/0x48b
[ 207.107979] softirqs last disabled at (3565): []
call_on_stack+0x45/0x4c
[ 207.123827] ---[ end trace ]---
-- Steve
Re: [PATCH 3/3] drm/i915/pmu: Use common freq functions with sysfs
On 3/7/2023 9:33 PM, Ashutosh Dixit wrote:
Using common freq functions with sysfs in PMU (but without taking
forcewake) solves the following issues (a) missing support for MTL (b)
For the requested_freq, we read it only if actual_freq is zero below
(meaning, GT is in C6). So then what is the point of reading it without
a force wake? It will also be zero, correct?
Thanks,
Vinay.
missing support for older generation (prior to Gen6) (c) missing support
for slpc when freq sampling has to fall back to requested freq. It also
makes the PMU code future proof where sometimes code has been updated for
sysfs and PMU has been missed.
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/gt/intel_rps.c | 10 --
drivers/gpu/drm/i915/gt/intel_rps.h | 1 -
drivers/gpu/drm/i915/i915_pmu.c | 10 --
3 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 49df31927c0e..b03bfbe7ee23 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2046,16 +2046,6 @@ void intel_rps_sanitize(struct intel_rps *rps)
rps_disable_interrupts(rps);
}
-u32 intel_rps_read_rpstat_fw(struct intel_rps *rps)
-{
- struct drm_i915_private *i915 = rps_to_i915(rps);
- i915_reg_t rpstat;
-
- rpstat = (GRAPHICS_VER(i915) >= 12) ? GEN12_RPSTAT1 : GEN6_RPSTAT1;
-
- return intel_uncore_read_fw(rps_to_gt(rps)->uncore, rpstat);
-}
-
u32 intel_rps_read_rpstat(struct intel_rps *rps)
{
struct drm_i915_private *i915 = rps_to_i915(rps);
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index a990f985ab23..60ae27679011 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -53,7 +53,6 @@ u32 intel_rps_get_rp1_frequency(struct intel_rps *rps);
u32 intel_rps_get_rpn_frequency(struct intel_rps *rps);
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
u32 intel_rps_read_rpstat(struct intel_rps *rps);
-u32 intel_rps_read_rpstat_fw(struct intel_rps *rps);
void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps
*caps);
void intel_rps_raise_unslice(struct intel_rps *rps);
void intel_rps_lower_unslice(struct intel_rps *rps);
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index a76c5ce9513d..1a4c9fed257c 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -392,14 +392,12 @@ frequency_sample(struct intel_gt *gt, unsigned int
period_ns)
* case we assume the system is running at the intended
* frequency. Fortunately, the read should rarely fail!
*/
- val = intel_rps_read_rpstat_fw(rps);
- if (val)
- val = intel_rps_get_cagf(rps, val);
- else
- val = rps->cur_freq;
+ val = intel_rps_read_actual_frequency_fw(rps);
+ if (!val)
+ val = intel_rps_get_requested_frequency_fw(rps),
add_sample_mult(>sample[__I915_SAMPLE_FREQ_ACT],
- intel_gpu_freq(rps, val), period_ns / 1000);
+ val, period_ns / 1000);
}
if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) {
[PATCH -next] fbdev/clps711x-fb: Use devm_platform_get_and_ioremap_resource()
According to commit 890cc39a8799 ("drivers: provide
devm_platform_get_and_ioremap_resource()"), convert
platform_get_resource(), devm_ioremap_resource() to a single
call to devm_platform_get_and_ioremap_resource(), as this is exactly
what this function does.
Signed-off-by: Yang Li
---
drivers/video/fbdev/clps711x-fb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/clps711x-fb.c
b/drivers/video/fbdev/clps711x-fb.c
index 45c75ff01eca..c8bfc608bd9c 100644
--- a/drivers/video/fbdev/clps711x-fb.c
+++ b/drivers/video/fbdev/clps711x-fb.c
@@ -238,8 +238,7 @@ static int clps711x_fb_probe(struct platform_device *pdev)
info->fix.mmio_start = res->start;
info->fix.mmio_len = resource_size(res);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- info->screen_base = devm_ioremap_resource(dev, res);
+ info->screen_base = devm_platform_get_and_ioremap_resource(pdev, 1,
);
if (IS_ERR(info->screen_base)) {
ret = PTR_ERR(info->screen_base);
goto out_fb_release;
--
2.20.1.7.g153144c
Re: [PATCH 2/2] drm/i915/pmu: Use correct requested freq for SLPC
On Mon, 06 Mar 2023 03:10:24 -0800, Tvrtko Ursulin wrote:
>
Hi Tvrtko,
> On 04/03/2023 01:27, Ashutosh Dixit wrote:
> > SLPC does not use 'struct intel_rps'. Use UNSLICE_RATIO bits from
>
> Would it be more accurate to say 'SLPC does not use rps->cur_freq' rather
> than it not using struct intel_rps?
No actually SLPC maintains a separate 'struct intel_guc_slpc' and does not
use 'struct intel_rps' at all so all of 'struct intel_rps' is 0.
> Fixes: / stable ? CI chances of catching this?
Same issue as Patch 1, I have answered this there.
> > GEN6_RPNSWREQ for SLPC. See intel_rps_get_requested_frequency.
> >
> > Bspec: 52745
> >
> > Signed-off-by: Ashutosh Dixit
> > ---
> > drivers/gpu/drm/i915/i915_pmu.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c
> > b/drivers/gpu/drm/i915/i915_pmu.c
> > index f0a1e36915b8..5ee836610801 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -394,8 +394,13 @@ frequency_sample(struct intel_gt *gt, unsigned int
> > period_ns)
> > * frequency. Fortunately, the read should rarely fail!
> > */
> > val = intel_rps_get_cagf(rps, intel_rps_read_rpstat_fw(rps));
> > - if (!val)
> > - val = rps->cur_freq;
> > + if (!val) {
> > + if (intel_uc_uses_guc_slpc(>uc))
> > + val = intel_rps_read_punit_req(rps) >>
> > + GEN9_SW_REQ_UNSLICE_RATIO_SHIFT;
> > + else
> > + val = rps->cur_freq;
> > + }
>
> That's a bunch of duplication from intel_rps.c so perhaps the appropriate
> helpers should be exported (some way) from there.
This is also addressed in the new series:
https://patchwork.freedesktop.org/series/114814/
> > add_sample_mult(>sample[__I915_SAMPLE_FREQ_ACT],
> > intel_gpu_freq(rps, val), period_ns / 1000);
Thanks.
--
Ashutosh
Re: [PATCH 1/2] drm/i915/pmu: Use only freq bits for falling back to requested freq
On Mon, 06 Mar 2023 03:04:40 -0800, Tvrtko Ursulin wrote: > Hi Tvrtko, > On 04/03/2023 01:27, Ashutosh Dixit wrote: > > On newer generations, the GEN12_RPSTAT1 register contains more than freq > > information, e.g. see GEN12_VOLTAGE_MASK. Therefore use only the freq bits > > to decide whether to fall back to requested freq. > > CI is not catching the problem? This is because as we know PMU freq sampling happens only when gt is unparked (actively processing requests) so it is highly unlikely that gt will be in rc6 when it might have to fall back to requested freq (I checked this and it seems it is only at the end of the workload that we see it entering the fallback code path). Deleting the fallback path completely will not make much difference to the output and is an option too. Anyway I have retained it for now. > Could you find an appropriate Fixes: tag please? If it can affects a > platform out of force probe then cc: stable to. Cc stable is anyway not needed because affected platforms (DG1 onwards) are under force probe. Also because the issue does not affect real metrics (as mentioned above) as well as because it is a really a missing patch rather than a broken previous patch I am skipping the Fixes tag. > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/i915/i915_pmu.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c > > b/drivers/gpu/drm/i915/i915_pmu.c > > index 52531ab28c5f..f0a1e36915b8 100644 > > --- a/drivers/gpu/drm/i915/i915_pmu.c > > +++ b/drivers/gpu/drm/i915/i915_pmu.c > > @@ -393,10 +393,8 @@ frequency_sample(struct intel_gt *gt, unsigned int > > period_ns) > > * case we assume the system is running at the intended > > * frequency. Fortunately, the read should rarely fail! > > */ > > - val = intel_rps_read_rpstat_fw(rps); > > - if (val) > > - val = intel_rps_get_cagf(rps, val); > > - else > > + val = intel_rps_get_cagf(rps, intel_rps_read_rpstat_fw(rps)); > > Will this work with gen5_invert_freq as called by intel_rps_get_cagf? PMU has ever only supported Gen6+. See intel_rps_read_rpstat_fw (Gen5 does not have a GEN6_RPSTAT1 register) as well as 01b8c2e60e96. More importantly PMU was missing support for MTL. It is to avoid these kinds of issues I have submitted a new series with a different approach which should now take care of both MTL+ as well as Gen5-: https://patchwork.freedesktop.org/series/114814/ > > + if (!val) > > val = rps->cur_freq; > > add_sample_mult(>sample[__I915_SAMPLE_FREQ_ACT], Thanks. -- Ashutosh
[PATCH 0/3] drm/i915/pmu: Use common freq functions with sysfs
Using common freq functions with sysfs in PMU (but without taking forcewake) solves the following issues (a) missing support for MTL (b) missing support for older generations (prior to Gen6) (c) missing support for slpc when freq sampling has to fall back to requested freq. It also makes the PMU code future proof where sometimes code has been updated for sysfs and PMU has been missed. Ashutosh Dixit (3): drm/i915/rps: Expose read_actual_frequency_fw for PMU drm/i915/rps: Expose get_requested_frequency_fw for PMU drm/i915/pmu: Use common freq functions with sysfs drivers/gpu/drm/i915/gt/intel_rps.c | 68 + drivers/gpu/drm/i915/gt/intel_rps.h | 4 +- drivers/gpu/drm/i915/i915_pmu.c | 10 ++--- 3 files changed, 56 insertions(+), 26 deletions(-) -- 2.38.0
[PATCH 2/3] drm/i915/rps: Expose get_requested_frequency_fw for PMU
Expose intel_rps_get_requested_frequency_fw to read the requested freq
without taking forcewake. This is done for use by PMU which does not take
forcewake when reading freq. The code is refactored to use a common set of
functions across sysfs and PMU. It also allows PMU to support both host
turbo (rps) and slpc which was previously missed due to the non-use of
common functions across sysfs and PMU.
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/gt/intel_rps.c | 22 +++---
drivers/gpu/drm/i915/gt/intel_rps.h | 2 +-
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 0a8e24bcb874..49df31927c0e 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2154,7 +2154,7 @@ u32 intel_rps_read_actual_frequency_fw(struct intel_rps
*rps)
return freq;
}
-u32 intel_rps_read_punit_req(struct intel_rps *rps)
+static u32 intel_rps_read_punit_req(struct intel_rps *rps, bool take_fw)
{
struct intel_uncore *uncore = rps_to_uncore(rps);
struct intel_runtime_pm *rpm = rps_to_uncore(rps)->rpm;
@@ -2162,7 +2162,8 @@ u32 intel_rps_read_punit_req(struct intel_rps *rps)
u32 freq = 0;
with_intel_runtime_pm_if_in_use(rpm, wakeref)
- freq = intel_uncore_read(uncore, GEN6_RPNSWREQ);
+ freq = take_fw ? intel_uncore_read(uncore, GEN6_RPNSWREQ) :
+ intel_uncore_read_fw(uncore, GEN6_RPNSWREQ);
return freq;
}
@@ -2176,7 +2177,7 @@ static u32 intel_rps_get_req(u32 pureq)
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps)
{
- u32 freq = intel_rps_get_req(intel_rps_read_punit_req(rps));
+ u32 freq = intel_rps_get_req(intel_rps_read_punit_req(rps, true));
return intel_gpu_freq(rps, freq);
}
@@ -2189,6 +2190,21 @@ u32 intel_rps_get_requested_frequency(struct intel_rps
*rps)
return intel_gpu_freq(rps, rps->cur_freq);
}
+static u32 intel_rps_read_punit_req_frequency_fw(struct intel_rps *rps)
+{
+ u32 freq = intel_rps_get_req(intel_rps_read_punit_req(rps, false));
+
+ return intel_gpu_freq(rps, freq);
+}
+
+u32 intel_rps_get_requested_frequency_fw(struct intel_rps *rps)
+{
+ if (rps_uses_slpc(rps))
+ return intel_rps_read_punit_req_frequency_fw(rps);
+ else
+ return intel_gpu_freq(rps, rps->cur_freq);
+}
+
u32 intel_rps_get_max_frequency(struct intel_rps *rps)
{
struct intel_guc_slpc *slpc = rps_to_slpc(rps);
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index 63511b826a97..a990f985ab23 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -41,6 +41,7 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat1);
u32 intel_rps_read_actual_frequency(struct intel_rps *rps);
u32 intel_rps_read_actual_frequency_fw(struct intel_rps *rps);
u32 intel_rps_get_requested_frequency(struct intel_rps *rps);
+u32 intel_rps_get_requested_frequency_fw(struct intel_rps *rps);
u32 intel_rps_get_min_frequency(struct intel_rps *rps);
u32 intel_rps_get_min_raw_freq(struct intel_rps *rps);
int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val);
@@ -50,7 +51,6 @@ int intel_rps_set_max_frequency(struct intel_rps *rps, u32
val);
u32 intel_rps_get_rp0_frequency(struct intel_rps *rps);
u32 intel_rps_get_rp1_frequency(struct intel_rps *rps);
u32 intel_rps_get_rpn_frequency(struct intel_rps *rps);
-u32 intel_rps_read_punit_req(struct intel_rps *rps);
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
u32 intel_rps_read_rpstat(struct intel_rps *rps);
u32 intel_rps_read_rpstat_fw(struct intel_rps *rps);
--
2.38.0
[PATCH 3/3] drm/i915/pmu: Use common freq functions with sysfs
Using common freq functions with sysfs in PMU (but without taking
forcewake) solves the following issues (a) missing support for MTL (b)
missing support for older generation (prior to Gen6) (c) missing support
for slpc when freq sampling has to fall back to requested freq. It also
makes the PMU code future proof where sometimes code has been updated for
sysfs and PMU has been missed.
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/gt/intel_rps.c | 10 --
drivers/gpu/drm/i915/gt/intel_rps.h | 1 -
drivers/gpu/drm/i915/i915_pmu.c | 10 --
3 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 49df31927c0e..b03bfbe7ee23 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2046,16 +2046,6 @@ void intel_rps_sanitize(struct intel_rps *rps)
rps_disable_interrupts(rps);
}
-u32 intel_rps_read_rpstat_fw(struct intel_rps *rps)
-{
- struct drm_i915_private *i915 = rps_to_i915(rps);
- i915_reg_t rpstat;
-
- rpstat = (GRAPHICS_VER(i915) >= 12) ? GEN12_RPSTAT1 : GEN6_RPSTAT1;
-
- return intel_uncore_read_fw(rps_to_gt(rps)->uncore, rpstat);
-}
-
u32 intel_rps_read_rpstat(struct intel_rps *rps)
{
struct drm_i915_private *i915 = rps_to_i915(rps);
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index a990f985ab23..60ae27679011 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -53,7 +53,6 @@ u32 intel_rps_get_rp1_frequency(struct intel_rps *rps);
u32 intel_rps_get_rpn_frequency(struct intel_rps *rps);
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
u32 intel_rps_read_rpstat(struct intel_rps *rps);
-u32 intel_rps_read_rpstat_fw(struct intel_rps *rps);
void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps
*caps);
void intel_rps_raise_unslice(struct intel_rps *rps);
void intel_rps_lower_unslice(struct intel_rps *rps);
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index a76c5ce9513d..1a4c9fed257c 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -392,14 +392,12 @@ frequency_sample(struct intel_gt *gt, unsigned int
period_ns)
* case we assume the system is running at the intended
* frequency. Fortunately, the read should rarely fail!
*/
- val = intel_rps_read_rpstat_fw(rps);
- if (val)
- val = intel_rps_get_cagf(rps, val);
- else
- val = rps->cur_freq;
+ val = intel_rps_read_actual_frequency_fw(rps);
+ if (!val)
+ val = intel_rps_get_requested_frequency_fw(rps),
add_sample_mult(>sample[__I915_SAMPLE_FREQ_ACT],
- intel_gpu_freq(rps, val), period_ns / 1000);
+ val, period_ns / 1000);
}
if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) {
--
2.38.0
[PATCH 1/3] drm/i915/rps: Expose read_actual_frequency_fw for PMU
Expose intel_rps_read_actual_frequency_fw to read the actual/granted freq
without taking forcewake. This is done for use by PMU which does not take
forcewake when reading freq. The code is refactored to use a common set of
functions across sysfs and PMU. It also allows PMU to support MTL as well
as older generations (before Gen6) which were previously missed due to the
non-use of common functions across sysfs and PMU.
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/gt/intel_rps.c | 36 +
drivers/gpu/drm/i915/gt/intel_rps.h | 1 +
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 4d0dc9de23f9..0a8e24bcb874 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2089,10 +2089,11 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32
rpstat)
return cagf;
}
-static u32 read_cagf(struct intel_rps *rps)
+static u32 __read_cagf(struct intel_rps *rps, bool take_fw)
{
struct drm_i915_private *i915 = rps_to_i915(rps);
struct intel_uncore *uncore = rps_to_uncore(rps);
+ i915_reg_t r = INVALID_MMIO_REG;
u32 freq;
/*
@@ -2100,22 +2101,30 @@ static u32 read_cagf(struct intel_rps *rps)
* registers will return 0 freq when GT is in RC6
*/
if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) {
- freq = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
+ r = MTL_MIRROR_TARGET_WP1;
} else if (GRAPHICS_VER(i915) >= 12) {
- freq = intel_uncore_read(uncore, GEN12_RPSTAT1);
+ r = GEN12_RPSTAT1;
} else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
vlv_punit_get(i915);
freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
vlv_punit_put(i915);
+ goto exit;
} else if (GRAPHICS_VER(i915) >= 6) {
- freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
+ r = GEN6_RPSTAT1;
} else {
- freq = intel_uncore_read(uncore, MEMSTAT_ILK);
+ r = MEMSTAT_ILK;
}
+ freq = take_fw ? intel_uncore_read(uncore, r) :
intel_uncore_read_fw(uncore, r);
+exit:
return intel_rps_get_cagf(rps, freq);
}
+static u32 read_cagf(struct intel_rps *rps)
+{
+ return __read_cagf(rps, true);
+}
+
u32 intel_rps_read_actual_frequency(struct intel_rps *rps)
{
struct intel_runtime_pm *rpm = rps_to_uncore(rps)->rpm;
@@ -2128,6 +2137,23 @@ u32 intel_rps_read_actual_frequency(struct intel_rps
*rps)
return freq;
}
+static u32 read_cagf_fw(struct intel_rps *rps)
+{
+ return __read_cagf(rps, false);
+}
+
+u32 intel_rps_read_actual_frequency_fw(struct intel_rps *rps)
+{
+ struct intel_runtime_pm *rpm = rps_to_uncore(rps)->rpm;
+ intel_wakeref_t wakeref;
+ u32 freq = 0;
+
+ with_intel_runtime_pm_if_in_use(rpm, wakeref)
+ freq = intel_gpu_freq(rps, read_cagf_fw(rps));
+
+ return freq;
+}
+
u32 intel_rps_read_punit_req(struct intel_rps *rps)
{
struct intel_uncore *uncore = rps_to_uncore(rps);
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index c622962c6bef..63511b826a97 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -39,6 +39,7 @@ int intel_gpu_freq(struct intel_rps *rps, int val);
int intel_freq_opcode(struct intel_rps *rps, int val);
u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat1);
u32 intel_rps_read_actual_frequency(struct intel_rps *rps);
+u32 intel_rps_read_actual_frequency_fw(struct intel_rps *rps);
u32 intel_rps_get_requested_frequency(struct intel_rps *rps);
u32 intel_rps_get_min_frequency(struct intel_rps *rps);
u32 intel_rps_get_min_raw_freq(struct intel_rps *rps);
--
2.38.0
Re: [PATCH 9/9] drm: move ttm_execbuf_util into vmwgfx
On Tue, 2023-02-28 at 09:34 +0100, Christian König wrote: > VMWGFX is the only remaining user of this and should probably moved over > to drm_exec when it starts using GEM as well. Is this because vmwgfx piggybacks buffer-id relocations on top of ttm validations or did you just find it too hard to port it over? I'd prefer to avoid ttm moves to vmwgfx and at least have a clear idea of what we need to do to port. z
[PATCH v3 2/2] drm/panel: Add driver for Novatek NT36523
Add a driver for panels using the Novatek NT36523 display driver IC.
Signed-off-by: Jianhua Lu
---
Changes in v3:
- Refactor source code
Changes in v2:
- Refactor and clean up source code
MAINTAINERS | 7 +
drivers/gpu/drm/panel/Kconfig | 10 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-novatek-nt36523.c | 770 ++
4 files changed, 788 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt36523.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 5383af5d3b45..3586248bb05d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6537,6 +6537,13 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
F: drivers/gpu/drm/panel/panel-novatek-nt35560.c
+DRM DRIVER FOR NOVATEK NT36523 PANELS
+M: Jianhua Lu
+S: Maintained
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/panel/novatek,nt36523.yaml
+F: drivers/gpu/drm/panel/panel-novatek-nt36523.c
+
DRM DRIVER FOR NOVATEK NT36672A PANELS
M: Sumit Semwal
S: Maintained
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 871c..268508743b5c 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -377,6 +377,16 @@ config DRM_PANEL_NOVATEK_NT35950
Sharp panels used in Sony Xperia Z5 Premium and XZ Premium
mobile phones.
+config DRM_PANEL_NOVATEK_NT36523
+ tristate "Novatek NT36523 panel driver"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for the panels built
+ around the Novatek NT36523 display controller, such as some
+ Boe panels used in Xiaomi Mi Pad 5 and 5 Pro tablets.
+
config DRM_PANEL_NOVATEK_NT36672A
tristate "Novatek NT36672A DSI panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index c05aa9e23907..570eab8bf2b2 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) +=
panel-newvision-nv3052c.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35560) += panel-novatek-nt35560.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35950) += panel-novatek-nt35950.o
+obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36523) += panel-novatek-nt36523.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36672A) += panel-novatek-nt36672a.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36523.c
b/drivers/gpu/drm/panel/panel-novatek-nt36523.c
new file mode 100644
index ..0df41ef9690c
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36523.c
@@ -0,0 +1,770 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Novatek NT36523 DriverIC panels driver
+ *
+ * Copyright (c) 2022, 2023 Jianhua Lu
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#define DSI_NUM_MIN 1
+
+/* Macro modified from mipi_dual_dsi_dcs_write_seq */
+#define mipi_dual_dsi_dcs_write_seq(dsi, cmd, seq...) \
+ do { \
+ static const u8 d[] = { cmd, seq };\
+ int i, ret;\
+ for (i = 0; i < ARRAY_SIZE(dsi); i++) {\
+ ret = mipi_dsi_dcs_write_buffer(dsi[i], d,
ARRAY_SIZE(d));\
+ if (ret < 0) {
\
+ dev_err_ratelimited(
\
+ [i]->dev, "sending command %#02x
failed: %d\n", \
+ cmd, ret);
\
+ return ret;
\
+ }
\
+ }
\
+ } while (0)
+
+struct panel_info {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ const struct panel_desc *desc;
+
+ struct gpio_desc *reset_gpio;
+ struct backlight_device *backlight;
+ struct regulator *vddio;
+
+ bool prepared;
+};
+
+struct panel_desc {
+ unsigned int width_mm;
+ unsigned int height_mm;
+
+ unsigned int bpc;
+ unsigned int lanes;
+ unsigned long mode_flags;
+ enum mipi_dsi_pixel_format
[PATCH v3 1/2] dt-bindings: display: panel: Add Novatek NT36523 bindings
Novatek NT36523 is a display driver IC used to drive DSI panels.
Signed-off-by: Jianhua Lu
Reviewed-by: Krzysztof Kozlowski
---
Changes in v3:
- pick up Krzysztof's R-b
Changes in v2:
- Drop unnecessary description
- dsi0 -> dsi
- Correct indentation
.../display/panel/novatek,nt36523.yaml| 85 +++
1 file changed, 85 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/novatek,nt36523.yaml
diff --git
a/Documentation/devicetree/bindings/display/panel/novatek,nt36523.yaml
b/Documentation/devicetree/bindings/display/panel/novatek,nt36523.yaml
new file mode 100644
index ..0039561ef04c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/novatek,nt36523.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/novatek,nt36523.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Novatek NT36523 based DSI display Panels
+
+maintainers:
+ - Jianhua Lu
+
+description: |
+ The Novatek NT36523 is a generic DSI Panel IC used to drive dsi
+ panels. Support video mode panels from China Star Optoelectronics
+ Technology (CSOT) and BOE Technology.
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+items:
+ - enum:
+ - xiaomi,elish-boe-nt36523
+ - xiaomi,elish-csot-nt36523
+ - const: novatek,nt36523
+
+ reset-gpios:
+maxItems: 1
+description: phandle of gpio for reset line - This should be 8mA
+
+ vddio-supply:
+description: regulator that supplies the I/O voltage
+
+ reg: true
+ ports: true
+ backlight: true
+
+required:
+ - compatible
+ - reg
+ - vddio-supply
+ - reset-gpios
+ - ports
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "xiaomi,elish-csot-nt36523", "novatek,nt36523";
+reg = <0>;
+
+vddio-supply = <_l14a_1p88>;
+reset-gpios = < 75 GPIO_ACTIVE_LOW>;
+backlight = <>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+panel_in_0: endpoint {
+remote-endpoint = <_out>;
+};
+};
+
+port@1{
+reg = <1>;
+panel_in_1: endpoint {
+remote-endpoint = <_out>;
+};
+};
+};
+};
+};
+
+...
--
2.39.2
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
On Tue, 7 Mar 2023 21:22:23 -0500
Steven Rostedt wrote:
> Looks like there was a lock possibly used after free. But as commit
> 9bff18d13473a9fdf81d5158248472a9d8ecf2bd ("drm/ttm: use per BO cleanup
> workers") changed a lot of this code, I figured it may be the culprit.
If I bothered to look at the second warning after this one (I usually stop
after the first), it appears to state there was a use after free issue.
[ 206.692285] [ cut here ]
[ 206.706333] refcount_t: underflow; use-after-free.
[ 206.720577] WARNING: CPU: 0 PID: 332 at lib/refcount.c:28
refcount_warn_saturate+0xb6/0xfc
[ 206.735810] Modules linked in:
[ 206.749493] CPU: 0 PID: 332 Comm: kworker/0:13H Tainted: GW
6.3.0-rc1-test-1-ga98bd42762ed-dirty #965
[ 206.765833] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.16.0-debian-1.16.0-5 04/01/2014
[ 206.781767] Workqueue: ttm ttm_bo_delayed_delete
[ 206.796500] EIP: refcount_warn_saturate+0xb6/0xfc
[ 206.811121] Code: 68 50 1c 0d cf e8 66 b3 a9 ff 0f 0b 58 c9 c3 90 80 3d 57
c6 38 cf 00 75 8a c6 05 57 c6 38 cf 01 68 7c 1c 0d cf e8 46 b3 a9 ff <0f> 0b 59
c9 c3 80 3d 55 c6 38 cf 00 0f 85 67 ff ff ff c6 05 55 c6
[ 206.844560] EAX: 0026 EBX: c2d5f150 ECX: c3ae5e40 EDX: 0002
[ 206.862109] ESI: c2d5f0bc EDI: f6f91200 EBP: c3ae5f18 ESP: c3ae5f14
[ 206.878773] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246
[ 206.895665] CR0: 80050033 CR2: ff9ff000 CR3: 0f512000 CR4: 00150ef0
[ 206.912303] Call Trace:
[ 206.927940] ttm_bo_delayed_delete+0x8c/0x94
[ 206.944179] process_one_work+0x21a/0x538
[ 206.960605] worker_thread+0x146/0x398
[ 206.976839] kthread+0xea/0x10c
[ 206.992696] ? process_one_work+0x538/0x538
[ 207.008827] ? kthread_complete_and_exit+0x1c/0x1c
[ 207.025150] ret_from_fork+0x1c/0x28
[ 207.041307] irq event stamp: 4219
[ 207.056883] hardirqs last enabled at (4219): []
_raw_spin_unlock_irqrestore+0x2d/0x58
[ 207.074298] hardirqs last disabled at (4218): []
kvfree_call_rcu+0x155/0x2ec
[ 207.091461] softirqs last enabled at (3570): []
__do_softirq+0x2f3/0x48b
[ 207.107979] softirqs last disabled at (3565): []
call_on_stack+0x45/0x4c
[ 207.123827] ---[ end trace ]---
-- Steve
[BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
In a report for a regression in my code, I tried to run v6.3-rc1 through my
tests. It crashed at boot up on my first test (my start up tests do take a
long time, hence the 206 seconds of boot!).
[ 206.238782] [ cut here ]
[ 206.277786] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
[ 206.277946] WARNING: CPU: 0 PID: 332 at kernel/locking/mutex.c:582
__ww_mutex_lock.constprop.0+0x566/0xfec
[ 206.313338] Modules linked in:
[ 206.324732] CPU: 0 PID: 332 Comm: kworker/0:13H Not tainted
6.3.0-rc1-test-1-ga98bd42762ed-dirty #965
[ 206.338273] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.16.0-debian-1.16.0-5 04/01/2014
[ 206.353596] Workqueue: ttm ttm_bo_delayed_delete
[ 206.370520] EIP: __ww_mutex_lock.constprop.0+0x566/0xfec
[ 206.382855] Code: e8 ab 59 95 ff 85 c0 0f 84 25 fb ff ff 8b 0d 58 c0 3b cf
85 c9 0f 85 17 fb ff ff 68 e0 8d 07 cf 68 2b ac 05 cf e8 e6 e6 3f ff <0f> 0b 58
5a e9 ff fa ff ff e8 78 59 95 ff 85 c0 74 0e 8b 0d 58 c0
[ 206.411247] EAX: 0028 EBX: ECX: c3ae5dd8 EDX: 0002
[ 206.425193] ESI: EDI: c2d5f0bc EBP: c3ae5f00 ESP: c3ae5eac
[ 206.439236] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246
[ 206.453597] CR0: 80050033 CR2: ff9ff000 CR3: 0f512000 CR4: 00150ef0
[ 206.467841] Call Trace:
[ 206.481059] ? ttm_bo_delayed_delete+0x30/0x94
[ 206.494980] ww_mutex_lock+0x32/0x94
[ 206.508699] ttm_bo_delayed_delete+0x30/0x94
[ 206.522371] process_one_work+0x21a/0x538
[ 206.536306] worker_thread+0x146/0x398
[ 206.549860] kthread+0xea/0x10c
[ 206.563141] ? process_one_work+0x538/0x538
[ 206.576835] ? kthread_complete_and_exit+0x1c/0x1c
[ 206.590652] ret_from_fork+0x1c/0x28
[ 206.604522] irq event stamp: 4219
[ 206.617852] hardirqs last enabled at (4219): []
_raw_spin_unlock_irqrestore+0x2d/0x58
[ 206.633077] hardirqs last disabled at (4218): []
kvfree_call_rcu+0x155/0x2ec
[ 206.648161] softirqs last enabled at (3570): []
__do_softirq+0x2f3/0x48b
[ 206.663025] softirqs last disabled at (3565): []
call_on_stack+0x45/0x4c
[ 206.678065] ---[ end trace ]---
Looks like there was a lock possibly used after free. But as commit
9bff18d13473a9fdf81d5158248472a9d8ecf2bd ("drm/ttm: use per BO cleanup
workers") changed a lot of this code, I figured it may be the culprit.
-- Steve
Re: [PATCH 01/10] Revert "drm/msm: Add missing check and destroy for alloc_ordered_workqueue"
On Mon, 06 Mar 2023 18:07:13 +0800, Johan Hovold wrote: > This reverts commit 643b7d0869cc7f1f7a5ac7ca6bd25d88f54e31d0. The commit not only adds the allocation sanity check, but also adds the destroy_workqueue to release the allocated priv->wq. Therefore, revert the commit will cause memory leak. > A recent patch that tried to fix up the msm_drm_init() paths with > respect to the workqueue but only ended up making things worse: > > First, the newly added calls to msm_drm_uninit() on early errors would > trigger NULL-pointer dereferences, for example, as the kms pointer would > not have been initialised. (Note that these paths were also modified by > a second broken error handling patch which in effect cancelled out this > part when merged.) There is a check for the kms pointer to avoid NULL-pointer dereference in the msm_drm_uninit(). > Second, the newly added allocation sanity check would still leak the > previously allocated drm device. The ddev is allocated by drm_dev_alloc which support automatic cleanup. Thanks, Jiang
Re: [PATCH v2 2/2] drm/panel: Add driver for Novatek NT36523
On Tue, Mar 07, 2023 at 11:34:55PM +0100, Linus Walleij wrote:
> Hi Jianhua,
>
> thanks for your patch!
>
> It appears Konrad is working on a very similar driver, so I suggest merging
> them into one Novatek NT36523 driver.
>
> Possibly we can fix this up first and then add Konrads Lenovo-panel with
> a patch on top.
>
> On Mon, Feb 20, 2023 at 1:13 PM Jianhua Lu wrote:
>
> > Add a driver for panels using the Novatek NT36523 display driver IC.
> >
> > Signed-off-by: Jianhua Lu
>
> (...)
>
> I like how you abstract the panel with init commands in the panel info.
>
> > +enum dsi_cmd_type {
> > + INIT_DCS_CMD,
> > + DELAY_CMD,
> > +};
> > +
> > +struct panel_init_cmd {
> > + enum dsi_cmd_type type;
> > + size_t len;
> > + const char *data;
> > +};
> > +
> > +#define _INIT_DCS_CMD(...) { \
> > + .type = INIT_DCS_CMD, \
> > + .len = sizeof((char[]){__VA_ARGS__}), \
> > + .data = (char[]){__VA_ARGS__} }
> > +
> > +#define _INIT_DELAY_CMD(...) { \
> > + .type = DELAY_CMD,\
> > + .len = sizeof((char[]){__VA_ARGS__}), \
> > + .data = (char[]){__VA_ARGS__} }
>
> I have seen this type of reinvented wheels a few times now. Don't do this.
>
> Look into other recently merged drivers and look how they do it, for example
> drivers/gpu/drm/panel/panel-himax-hx8394.c
>
> For example:
>
> - Use mipi_dsi_dcs_write_seq()
>
> - If the delay is just used at one point in the sequence, do not invent
> a command language like above for it, open code the delay instead
>
> - Try to decode as much magic as possible, if you look in Konrads
> driver you clearly see some standard MIPI commands, I bet you have
> some too.
>
> - Maybe use callbacks to send sequences instead of tables, like in
> the himax driver?
Maybe I should create a wrapper of mipi_dsi_dcs_write_seq() for sync dual dsi
mode.
>
> Other than that it seems like something that could also handle the Lenovo
> display, or the other way around, I don't know which driver is the best
> starting point, but this one has the right Novatek name at least.
>
> Yours,
> Linus Walleij
Re: [PATCH v2 2/2] drm/panel: Add driver for Novatek NT36523
On Tue, Mar 07, 2023 at 11:34:55PM +0100, Linus Walleij wrote:
> Hi Jianhua,
>
> thanks for your patch!
>
> It appears Konrad is working on a very similar driver, so I suggest merging
> them into one Novatek NT36523 driver.
>
> Possibly we can fix this up first and then add Konrads Lenovo-panel with
> a patch on top.
>
> On Mon, Feb 20, 2023 at 1:13 PM Jianhua Lu wrote:
>
> > Add a driver for panels using the Novatek NT36523 display driver IC.
> >
> > Signed-off-by: Jianhua Lu
>
> (...)
>
> I like how you abstract the panel with init commands in the panel info.
>
> > +enum dsi_cmd_type {
> > + INIT_DCS_CMD,
> > + DELAY_CMD,
> > +};
> > +
> > +struct panel_init_cmd {
> > + enum dsi_cmd_type type;
> > + size_t len;
> > + const char *data;
> > +};
> > +
> > +#define _INIT_DCS_CMD(...) { \
> > + .type = INIT_DCS_CMD, \
> > + .len = sizeof((char[]){__VA_ARGS__}), \
> > + .data = (char[]){__VA_ARGS__} }
> > +
> > +#define _INIT_DELAY_CMD(...) { \
> > + .type = DELAY_CMD,\
> > + .len = sizeof((char[]){__VA_ARGS__}), \
> > + .data = (char[]){__VA_ARGS__} }
>
> I have seen this type of reinvented wheels a few times now. Don't do this.
>
> Look into other recently merged drivers and look how they do it, for example
> drivers/gpu/drm/panel/panel-himax-hx8394.c
>
> For example:
>
> - Use mipi_dsi_dcs_write_seq()
>
> - If the delay is just used at one point in the sequence, do not invent
> a command language like above for it, open code the delay instead
>
> - Try to decode as much magic as possible, if you look in Konrads
> driver you clearly see some standard MIPI commands, I bet you have
> some too.
>
> - Maybe use callbacks to send sequences instead of tables, like in
> the himax driver?
I use init cmd tables in order to send init sequences to dsi0 and dsi1 at the
same time (sync dual dsi mode). I don't found a convenient way to use
mipi_dsi_dcs_write_seq() for sync dual dsi mode.
>
> Other than that it seems like something that could also handle the Lenovo
> display, or the other way around, I don't know which driver is the best
> starting point, but this one has the right Novatek name at least.
>
> Yours,
> Linus Walleij
Re: [PATCH 1/2] dt-bindings: display/bridge: toshiba,tc358764: convert to dtschema
On Sat, 25 Feb 2023 17:02:51 +0100, Krzysztof Kozlowski wrote: > Convert the Toshiba TC358764 bridge bindings to DT schema. > > Signed-off-by: Krzysztof Kozlowski > --- > .../display/bridge/toshiba,tc358764.txt | 35 > .../display/bridge/toshiba,tc358764.yaml | 89 +++ > 2 files changed, 89 insertions(+), 35 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt > create mode 100644 > Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.yaml > Applied, thanks!
Re: [PATCH v2 1/2] dt-bindings: display: panel: Add Novatek NT36523 bindings
On Tue, Mar 07, 2023 at 11:22:35PM +0100, Linus Walleij wrote: > On Mon, Feb 20, 2023 at 1:13 PM Jianhua Lu wrote: > > > Novatek NT36523 is a display driver IC used to drive DSI panels. > > > > Signed-off-by: Jianhua Lu > > --- > > Changes in v2: > > - Drop unnecessary description > > - dsi0 -> dsi > > - Correct indentation > > I'd like Konrad and Neil to look at this before we merge it. > > > +required: > > + - compatible > > + - reg > > + - vddio-supply > > + - vddpos-supply > > + - vddneg-supply > > It appears vddpos and vddneg are not necessary on > all variants, can they be made non-required? It can be non-required, this panel's positive regulator and negative regulator are supplied by backlight IC, the both regulator of Konrad's NT36523W panel are supplied by secure firmware layer. > > It is also possible to do some - if -construction of course > based on the compatible, if we want to be fancy. > > Yours, > Linus Walleij
Re: [PATCH] dt-bindings: display: bridge: parade,ps8622: convert to dtschema
On Tue, 21 Feb 2023 18:09:55 +0100, Krzysztof Kozlowski wrote: > Convert the Parade PS8622/PS8625 DisplayPort to LVDS Converter bindings > to DT schema. Changes during conversion: add missing vdd12-supply, used > by Linux driver. > > Signed-off-by: Krzysztof Kozlowski > --- > .../display/bridge/parade,ps8622.yaml | 115 ++ > .../bindings/display/bridge/ps8622.txt| 31 - > 2 files changed, 115 insertions(+), 31 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/parade,ps8622.yaml > delete mode 100644 > Documentation/devicetree/bindings/display/bridge/ps8622.txt > Applied, thanks!
[PATCH] drm/amdkfd: fix a potential double free in pqm_create_queue
Set *q to NULL on errors, otherwise pqm_create_queue would free it again. Signed-off-by: Chia-I Wu --- drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c index 5137476ec18e6..4236539d9f932 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c @@ -218,8 +218,8 @@ static int init_user_queue(struct process_queue_manager *pqm, return 0; cleanup: - if (dev->shared_resources.enable_mes) - uninit_queue(*q); + uninit_queue(*q); + *q = NULL; return retval; } -- 2.40.0.rc0.216.gc4246ad0f0-goog
Re: [PATCH] drm/amd/display: remove legacy fields of dc_plane_cap struct
On 3/7/23 15:53, David Tadokoro wrote:
The fields blends_with_above and blends_with_below of struct
dc_plane_cap (defined in dc/dc.h) are boolean and set to true by
default. All instances of a dc_plane_cap maintain the default values of
both. Also, there is only one if statement that checks those fields and
there would be the same effect if it was deleted (assuming that those
fields are always going to be true).
For this reason, considering both fields as legacy ones, this commit
removes them and the aforementioned if statement.
Signed-off-by: David Tadokoro
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ---
drivers/gpu/drm/amd/display/dc/dc.h | 2 --
drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 3 ---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn201/dcn201_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c | 2 --
17 files changed, 36 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b472931cb7ca..fdcb375e908a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4354,9 +4354,6 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
if (plane->type != DC_PLANE_TYPE_DCN_UNIVERSAL)
continue;
- if (!plane->blends_with_above || !plane->blends_with_below)
- continue;
-
if (!plane->pixel_format_support.argb)
continue;
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index f0a1934ebf8c..ccc27d482640 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -82,8 +82,6 @@ enum det_size {
struct dc_plane_cap {
enum dc_plane_type type;
- uint32_t blends_with_above : 1;
- uint32_t blends_with_below : 1;
uint32_t per_pixel_alpha : 1;
struct {
uint32_t argb : 1;
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
index f808315b2835..a4a45a6ce61e 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
@@ -401,8 +401,6 @@ static const struct resource_caps stoney_resource_cap = {
static const struct dc_plane_cap plane_cap = {
.type = DC_PLANE_TYPE_DCE_RGB,
- .blends_with_below = true,
- .blends_with_above = true,
.per_pixel_alpha = 1,
.pixel_format_support = {
@@ -428,7 +426,6 @@ static const struct dc_plane_cap plane_cap = {
static const struct dc_plane_cap underlay_plane_cap = {
.type = DC_PLANE_TYPE_DCE_UNDERLAY,
- .blends_with_above = true,
.per_pixel_alpha = 1,
.pixel_format_support = {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index 6bfac8088ab0..2bb8e11f26e0 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -504,8 +504,6 @@ static const struct resource_caps rv2_res_cap = {
static const struct dc_plane_cap plane_cap = {
.type = DC_PLANE_TYPE_DCN_UNIVERSAL,
- .blends_with_above = true,
- .blends_with_below = true,
.per_pixel_alpha = true,
.pixel_format_support = {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 3af24ef9cb2d..00668df0938e 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -670,8 +670,6 @@ static const struct resource_caps res_cap_nv10 = {
static const struct dc_plane_cap plane_cap = {
.type = DC_PLANE_TYPE_DCN_UNIVERSAL,
- .blends_with_above = true,
- .blends_with_below = true,
.per_pixel_alpha = true,
.pixel_format_support = {
diff --git
Re: [PATCH v4 1/2] dt-bindings: display/panel: Add Sony Tama TD4353 JDI display panel
On Thu, Jan 19, 2023 at 5:32 PM Konrad Dybcio wrote: > From: Konrad Dybcio > > Add bindings for the display panel used on some Sony Xperia XZ2 and XZ2 > Compact smartphones. > > Signed-off-by: Konrad Dybcio > Signed-off-by: Konrad Dybcio > Reviewed-by: Krzysztof Kozlowski Patch applied to drm-misc-next. Yours, Linus Walleij
Re: [PATCH v4 2/2] gpu/drm/panel: Add Sony TD4353 JDI panel driver
On Thu, Jan 19, 2023 at 5:32 PM Konrad Dybcio wrote: > From: Konrad Dybcio > > Add support for the Sony TD4353 JDI 2160x1080 display panel used in > some Sony Xperia XZ2 and XZ2 Compact smartphones. Due to the specifics > of smartphone manufacturing, it is impossible to retrieve a better name > for this panel. > > This revision adds support for the default 60 Hz configuration, however > there could possibly be some room for expansion, as the display panels > used on Sony devices have historically been capable of >2x refresh rate > overclocking. > > Signed-off-by: Konrad Dybcio > Signed-off-by: Konrad Dybcio > Reviewed-by: Marijn Suijten Looks good, so patch applied to drm-misc-next. Yours, Linus Walleij
Re: [PATCH 1/2] drm/i915/gsc: flush the GSC worker before wedging on unload
On Thu, 2023-02-23 at 09:21 -0800, Ceraolo Spurio, Daniele wrote: > If we unload the driver and wedge before the GSC worker is complete, > the worker will hit an error on its submission to the GSC engine and > then exit. This is hard to hit for a user, but it is reproducible > with skipping selftests. The error is handled gracefully by the > worker, so there are no functional issues, but we still end up with > an error message in dmesg, which is something we want to avoid as > this is a supported scenario. We could modify the worker to better > handle a wedging occurring during its execution, but that gets > complicated for a couple of reasons: > - We do want the error on runtime wedging, because there are > implications for subsystems outside of GT (i.e., PXP, HDCP), it's > only the error on driver unload that we want to silence. > - The worker is responsible for multiple submissions (GSC FW load, > HuC auth, SW proxy), so all of those will have to be adapted to > handle the wedged_on_fini scenario. > Therefore, it's much simpler to just wait for the worker to be done > before wedging on driver removal, also considering that the worker > will likely already be idle in the great majority of non-selftest > scenarios. > > Signed-off-by: Daniele Ceraolo Spurio > alan:snip > > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -784,6 +784,29 @@ void intel_gt_driver_unregister(struct intel_gt *gt) > intel_rps_driver_unregister(>rps); > intel_gsc_fini(>gsc); > > + /* > + * If we unload the driver and wedge before the GSC worker is complete, alan:snip > + * scenarios. > + */ > + intel_gsc_uc_flush_work(>uc.gsc); alan: nit: from a code readiblity, having intel_gsc_fini before intel_gsc_uc_flush_work almost reads as if code should have been inverted eventhough we know that the former is for the old mei-comp based framework and the latter is based on the new gsc-cs based framework. i cant think of how else to resolve this other unintrusively other than adding a comment. Other than that, also considering we've had offline testing already verify this and the next patch too, LGTM: Reviewed-by: Alan Previn
Re: [PATCH 2/2] drm/nouveau/clk: avoid usage of list iterator after loop
On Wed, 2023-03-01 at 18:25 +0100, Jakob Koschel wrote: > + } > } > > + BUG_ON(!pstate); > nvkm_debug(subdev, "setting performance state %d\n", pstatei); > clk->pstate = pstatei; We should probably also replace this with if (WARN_ON(!pstate) return -EINVAL; -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH 1/2] drm/nouveau/device: avoid usage of list iterator after loop
On Wed, 2023-03-01 at 18:25 +0100, Jakob Koschel wrote:
> If potentially no valid element is found, 'pstate' would contain an
> invalid pointer past the iterator loop. To ensure 'pstate' is always
> valid, we only set it if the correct element was found. That allows
> adding a BUG_ON in case the code works incorrectly, exposing currently
> undetectable potential bugs.
>
> Additionally, Linus proposed to avoid any use of the list iterator
> variable after the loop, in the attempt to move the list iterator
> variable declaration into the marcro to avoid any potential misuse after
> the loop [1].
>
> Link:
> https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=ehreask5sqxpwr9y7k9sa6cwx...@mail.gmail.com/
> [1]
> Signed-off-by: Jakob Koschel
> ---
> drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c
> index ce774579c89d..7c9dd91e98ee 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c
> @@ -72,7 +72,7 @@ nvkm_control_mthd_pstate_attr(struct nvkm_control *ctrl,
> void *data, u32 size)
> } *args = data;
> struct nvkm_clk *clk = ctrl->device->clk;
> const struct nvkm_domain *domain;
> - struct nvkm_pstate *pstate;
> + struct nvkm_pstate *pstate = NULL, *iter;
> struct nvkm_cstate *cstate;
> int i = 0, j = -1;
> u32 lo, hi;
> @@ -103,11 +103,14 @@ nvkm_control_mthd_pstate_attr(struct nvkm_control
> *ctrl, void *data, u32 size)
> return -EINVAL;
>
> if (args->v0.state != NVIF_CONTROL_PSTATE_ATTR_V0_STATE_CURRENT) {
> - list_for_each_entry(pstate, >states, head) {
> - if (i++ == args->v0.state)
> + list_for_each_entry(iter, >states, head) {
> + if (i++ == args->v0.state) {
> + pstate = iter;
> break;
> + }
> }
>
> + BUG_ON(!pstate);
Let's replace this with
if (WARN_ON_ONCE(!pstate))
return -EINVAL;
> lo = pstate->base.domain[domain->name];
> hi = lo;
> list_for_each_entry(cstate, >list, head) {
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH 0/2] drm/nouveau: avoid usage of list iterator after loop
Actually hm, dim is warning me about this and making me realize there's probably a better way to handle this, going to revoke the previous r-b I gave and respond inline On Tue, 2023-03-07 at 17:43 -0500, Lyude Paul wrote: > Reviewed-by: Lyude Paul > > Will push upstream in just a moment > > On Wed, 2023-03-01 at 18:25 +0100, Jakob Koschel wrote: > > This patch set includes two instances where the list iterator variable > > 'pstate' is implicitly assumed to be valid after the iterator loop. > > While in pratice that is most likely the case (if > > 'pstatei'/'args->v0.state' is <= the elements in clk->states), we should > > explicitly only allow 'pstate' to always point to correct 'nvkm_pstate' > > structs. > > > > That allows catching potential bugs with BUG_ON(!pstate) that otherwise > > would be completely undetectable. > > > > It also helps the greater mission to hopefully move the list iterator > > variable into the iterating macro directly [1]. > > > > Link: > > https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=ehreask5sqxpwr9y7k9sa6cwx...@mail.gmail.com/ > > [1] > > Signed-off-by: Jakob Koschel > > --- > > Jakob Koschel (2): > > drm/nouveau/device: avoid usage of list iterator after loop > > drm/nouveau/clk: avoid usage of list iterator after loop > > > > drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++--- > > drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c| 9 ++--- > > 2 files changed, 12 insertions(+), 6 deletions(-) > > --- > > base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9 > > change-id: 20230301-drm-nouveau-avoid-iter-after-loop-4bff97166efa > > > > Best regards, > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH drm-next v2 05/16] drm: manager to keep track of GPUs VA mappings
* Danilo Krummrich [230306 10:46]: > On 3/2/23 03:38, Liam R. Howlett wrote: > > * Danilo Krummrich [230227 08:17]: > > > > ... > > > > > Would this variant be significantly more efficient? > > > > > > > > Well, what you are doing is walking the tree to see if there's anything > > > > there... then re-walking the tree to store it. So, yes, it's much more > > > > efficient.. However, writing is heavier. How much of the time is spent > > > > walking vs writing depends on the size of the tree, but it's rather easy > > > > to do this in a single walk of the tree so why wouldn't you? > > > > > > I will, I was just curious about how much of an impact it has. > > > > > > > > > > > > > > > > > Also, would this also work while already walking the tree? > > > > > > > > Yes, to an extent. If you are at the correct location in the tree, you > > > > can write to that location. If you are not in the correct location and > > > > try to write to the tree then things will go poorly.. In this scenario, > > > > we are very much walking the tree and writing to it in two steps. > > > > > > > > > > > > > > To remove an entry while walking the tree I have a separate function > > > > > drm_gpuva_iter_remove(). Would I need something similar for inserting > > > > > entries? > > > > > > > > I saw that. Your remove function uses the erase operation which is > > > > implemented as a walk to that location and a store of a null over the > > > > range that is returned. You do not need a function to insert an entry > > > > if the maple state is at the correct location, and that doesn't just > > > > mean setting mas.index/mas.last to the correct value. There is a node & > > > > offset saved in the maple state that needs to be in the correct > > > > location. If you store to that node then the node may be replaced, so > > > > other iterators that you have may become stale, but the one you used > > > > execute the store operation will now point to the new node with the new > > > > entry. > > > > > > > > > > > > > > I already provided this example in a separate mail thread, but it may > > > > > makes > > > > > sense to move this to the mailing list: > > > > > > > > > > In __drm_gpuva_sm_map() we're iterating a given range of the tree, > > > > > where the > > > > > given range is the size of the newly requested mapping. > > > > > __drm_gpuva_sm_map() > > > > > invokes a callback for each sub-operation that needs to be taken in > > > > > order to > > > > > fulfill this mapping request. In most cases such a callback just > > > > > creates a > > > > > drm_gpuva_op object and stores it in a list. > > > > > > > > > > However, drivers can also implement the callback, such that they > > > > > directly > > > > > execute this operation within the callback. > > > > > > > > > > Let's have a look at the following example: > > > > > > > > > >0 a 2 > > > > > old: |---| (bo_offset=n) > > > > > > > > > > 1 b 3 > > > > > req: |---| (bo_offset=m) > > > > > > > > > >0 a' 1 b 3 > > > > > new: |-|---| (a.bo_offset=n,b.bo_offset=m) > > > > > > > > > > This would result in the following operations. > > > > > > > > > > __drm_gpuva_sm_map() finds entry "a" and calls back into the driver > > > > > suggesting to re-map "a" with the new size. The driver removes entry > > > > > "a" > > > > > from the tree and adds "a'" > > > > > > > > What you have here won't work. The driver will cause your iterators > > > > maple state to point to memory that is freed. You will either need to > > > > pass through your iterator so that the modifications can occur with that > > > > maple state so it remains valid, or you will need to invalidate the > > > > iterator on every modification by the driver. > > > > > > > > I'm sure the first idea you have will be to invalidate the iterator, but > > > > that is probably not the way to proceed. Even ignoring the unclear > > > > locking of two maple states trying to modify the tree, this is rather > > > > inefficient - each invalidation means a re-walk of the tree. You may as > > > > well not use an iterator in this case. > > > > > > > > Depending on how/when the lookups occur, you could still iterate over > > > > the tree and let the driver modify the ending of "a", but leave the tree > > > > alone and just store b over whatever - but the failure scenarios may > > > > cause you grief. > > > > > > > > If you pass the iterator through, then you can just use it to do your > > > > writes and keep iterating as if nothing changed. > > > > > > Passing through the iterater clearly seems to be the way to go. > > > > > > I assume that if the entry to insert isn't at the location of the iterator > > > (as in the following example) we can just keep walking to this location my > > > changing the index of the mas and calling mas_walk()? > > > > no. You have to mas_set() to the value and walk from the top of the > >
Re: [PATCH 0/2] drm/nouveau: avoid usage of list iterator after loop
Reviewed-by: Lyude Paul Will push upstream in just a moment On Wed, 2023-03-01 at 18:25 +0100, Jakob Koschel wrote: > This patch set includes two instances where the list iterator variable > 'pstate' is implicitly assumed to be valid after the iterator loop. > While in pratice that is most likely the case (if > 'pstatei'/'args->v0.state' is <= the elements in clk->states), we should > explicitly only allow 'pstate' to always point to correct 'nvkm_pstate' > structs. > > That allows catching potential bugs with BUG_ON(!pstate) that otherwise > would be completely undetectable. > > It also helps the greater mission to hopefully move the list iterator > variable into the iterating macro directly [1]. > > Link: > https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=ehreask5sqxpwr9y7k9sa6cwx...@mail.gmail.com/ > [1] > Signed-off-by: Jakob Koschel > --- > Jakob Koschel (2): > drm/nouveau/device: avoid usage of list iterator after loop > drm/nouveau/clk: avoid usage of list iterator after loop > > drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++--- > drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c| 9 ++--- > 2 files changed, 12 insertions(+), 6 deletions(-) > --- > base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9 > change-id: 20230301-drm-nouveau-avoid-iter-after-loop-4bff97166efa > > Best regards, -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH v2 2/2] drm/panel: Add driver for Novatek NT36523
Hi Jianhua,
thanks for your patch!
It appears Konrad is working on a very similar driver, so I suggest merging
them into one Novatek NT36523 driver.
Possibly we can fix this up first and then add Konrads Lenovo-panel with
a patch on top.
On Mon, Feb 20, 2023 at 1:13 PM Jianhua Lu wrote:
> Add a driver for panels using the Novatek NT36523 display driver IC.
>
> Signed-off-by: Jianhua Lu
(...)
I like how you abstract the panel with init commands in the panel info.
> +enum dsi_cmd_type {
> + INIT_DCS_CMD,
> + DELAY_CMD,
> +};
> +
> +struct panel_init_cmd {
> + enum dsi_cmd_type type;
> + size_t len;
> + const char *data;
> +};
> +
> +#define _INIT_DCS_CMD(...) { \
> + .type = INIT_DCS_CMD, \
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +#define _INIT_DELAY_CMD(...) { \
> + .type = DELAY_CMD,\
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
I have seen this type of reinvented wheels a few times now. Don't do this.
Look into other recently merged drivers and look how they do it, for example
drivers/gpu/drm/panel/panel-himax-hx8394.c
For example:
- Use mipi_dsi_dcs_write_seq()
- If the delay is just used at one point in the sequence, do not invent
a command language like above for it, open code the delay instead
- Try to decode as much magic as possible, if you look in Konrads
driver you clearly see some standard MIPI commands, I bet you have
some too.
- Maybe use callbacks to send sequences instead of tables, like in
the himax driver?
Other than that it seems like something that could also handle the Lenovo
display, or the other way around, I don't know which driver is the best
starting point, but this one has the right Novatek name at least.
Yours,
Linus Walleij
Re: [PATCH v2 1/2] dt-bindings: display: panel: Add Novatek NT36523 bindings
On Mon, Feb 20, 2023 at 1:13 PM Jianhua Lu wrote: > Novatek NT36523 is a display driver IC used to drive DSI panels. > > Signed-off-by: Jianhua Lu > --- > Changes in v2: > - Drop unnecessary description > - dsi0 -> dsi > - Correct indentation I'd like Konrad and Neil to look at this before we merge it. > +required: > + - compatible > + - reg > + - vddio-supply > + - vddpos-supply > + - vddneg-supply It appears vddpos and vddneg are not necessary on all variants, can they be made non-required? It is also possible to do some - if -construction of course based on the compatible, if we want to be fancy. Yours, Linus Walleij
Re: [PATCH v6 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities
Hi Sascha,
Am Donnerstag, 16. Februar 2023, 11:24:44 CET schrieb Sascha Hauer:
> The different VOP variants support different maximum resolutions. Reject
> resolutions that are not supported by a specific variant.
>
> This hasn't been a problem in the upstream driver so far as 1920x1080
> has been the maximum resolution supported by the HDMI driver and that
> resolution is supported by all VOP variants. Now with higher resolutions
> supported in the HDMI driver we have to limit the resolutions to the
> ones supported by the VOP.
>
> The actual maximum resolutions are taken from the Rockchip downstream
> Kernel.
>
> Signed-off-by: Sascha Hauer
> ---
>
> Notes:
> Changes since v5:
> - fix wrong check height vs. width
>
> Changes since v4:
> - Use struct vop_rect for storing resolution
>
> Changes since v3:
> - new patch
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 +++
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 6 ++
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 5 -
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++
> 4 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fa1f4ee6d1950..40c688529d44e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc
> *crtc)
> spin_unlock_irqrestore(>irq_lock, flags);
> }
>
> +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> + const struct drm_display_mode
> *mode)
> +{
> + struct vop *vop = to_vop(crtc);
> +
> + if (vop->data->max_output.width && mode->hdisplay >
> vop->data->max_output.width)
> + return MODE_BAD_HVALUE;
> +
> + if (vop->data->max_output.height && mode->vdisplay >
> vop->data->max_output.height)
> + return MODE_BAD_VVALUE;
> +
> + return MODE_OK;
> +}
I'm very much in favor of codifying the possible resolutions. Hopefully
this will also enable better vop-selection down the road.
But ...
The above does break the px30-minievb display.
While the px30 TRM does say it supports a 1920x1080 resolution only, the
px30-minievb comes with a 720x1280 DSI display and normally runs just
fine with it.
Looking at the vendor-code [0], it seems they only seem to check for the
hvalue. Looking deeper, the height-check was present in the beginning [1],
but then was removed later on.
Looking a bit more, I find [2] which says that
"Actually vop hardware has no output height limit"
I re-checked this on both px30+dsi and rock64+1080p-hdmi and with
> + if (vop->data->max_output.height && mode->vdisplay >
> vop->data->max_output.height)
> + return MODE_BAD_VVALUE;
line gone, rock64 is still happy and the px30 works correctly again.
So, do you see an issue with removing the output-height check?
Heiko
[0]
https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#L2446
[1]
https://github.com/rockchip-linux/kernel/commit/7e3e0c5e2eb16901ab5dce1cb981e1ac58fe42c6
[2]
https://github.com/rockchip-linux/kernel/commit/28c41da2693fe448aeda7c03070c376290b93805
Re: [PATCH v2 2/2] gpu/drm/panel: Add Lenovo NT36523W BOE panel
On Tue, Mar 7, 2023 at 2:26 PM Konrad Dybcio wrote: > Introduce support for the BOE panel with a NT36523W touch/driver IC > found on some Lenovo Tab P11 devices. It's a 2000x1200, 24bit RGB > MIPI DSI panel with integrated DCS-controlled backlight (that expects > big-endian communication). > > Reviewed-by: Neil Armstrong > Signed-off-by: Konrad Dybcio I will think this is some variant of the Novatek NT36523 display controller packaged up with Lenovo electronics until proven how wrong I am. I will listen to reason if it can be demonstrated that NT36523 and NT36523W are considerably different and need very different drivers, but I seriously doubt it. (For reasons see below.) > drivers/gpu/drm/panel/panel-lenovo-nt36523w-boe.c | 747 > ++ We usually share code with different displays using the same display controller, so panel-novatek-nt36523.c should be used as name. > +config DRM_PANEL_LENOVO_NT36523W_BOE > + tristate "Lenovo NT36523W BOE panel" Name it after the display controller like the other examples in the Kconfig, DRM_PANEL_NOVATEK_NT36523 > + mipi_dsi_dcs_write_seq(dsi, 0xff, 0x20); > + mipi_dsi_dcs_write_seq(dsi, 0xfb, 0x01); > + mipi_dsi_dcs_write_seq(dsi, 0x05, 0xd9); > + mipi_dsi_dcs_write_seq(dsi, 0x07, 0x78); > + mipi_dsi_dcs_write_seq(dsi, 0x08, 0x5a); > + mipi_dsi_dcs_write_seq(dsi, 0x0d, 0x63); > + mipi_dsi_dcs_write_seq(dsi, 0x0e, 0x91); > + mipi_dsi_dcs_write_seq(dsi, 0x0f, 0x73); > + mipi_dsi_dcs_write_seq(dsi, 0x95, 0xeb); > + mipi_dsi_dcs_write_seq(dsi, 0x96, 0xeb); > + mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_PARTIAL_ROWS, 0x11); I think it looks very similar to Jianhua:s driver: https://lore.kernel.org/lkml/20230220121258.10727-1-lujianhua...@gmail.com/T/ Can't you just add this special magic sequence into that driver instead? Would it help if we merge Jianhua's driver first so you can patch on top of it? Yours, Linus Walleij
Re: [PATCH v2 1/2] dt-bindings: display/panel: Add Lenovo NT36523W BOE panel
On Tue, Mar 7, 2023 at 2:26 PM Konrad Dybcio wrote: > Add bindings for the 2000x1200px IPS panel found on Lenovo Tab P11/ > XiaoXin Pad devices. > > Reviewed-by: Rob Herring > Signed-off-by: Konrad Dybcio (...) > +$id: > http://devicetree.org/schemas/display/panel/lenovo,nt36523w-boe-j606.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NT36523W BOE panel found on Lenovo J606 devices It's a Novatek NT36523 display controller-based device isn't it? I would reflect that in the title or at least the description. > + > +maintainers: > + - Konrad Dybcio > + > +allOf: > + - $ref: panel-common.yaml# > + > +properties: > + compatible: > +const: lenovo,nt36523w-boe-j606 > + > + reg: > +maxItems: 1 > +description: DSI virtual channel > + > + vddio-supply: true > + reset-gpios: true > + rotation: true > + port: true This is clearly (as can be seen from the magic in the driver) a Novatek NT36523 display controller, just configured differently. https://lore.kernel.org/lkml/20230220121258.10727-1-lujianhua...@gmail.com/T/ Why can't you just modify the existing nt36523 binding from Jianhua Lu by e.g. making these two non-required: - vddpos-supply - vddneg-supply It would not be helpful for driver writers to have two different bindings for similar hardware hand having to write code to handle different properties depending on which binding is used, so please unify into one binding by cooperating with Jianhua. Would it help if we merged Jianhua's binding so you can build on top? Yours, Linus Walleij
[PULL] drm-intel-next
Hi Dave and Daniel, Here goes our first pull request towards 6.3. drm-intel-next-2023-03-07: Cross-subsystem Changes: - MEI patches to fix suspend/resume issues with the i915's PXP. (Alexander) Driver Changes: - Registers helpers and clean-ups. (Lucas) - PXP fixes and clean-ups. (Alan, Alexander) - CDCLK related fixes and w/a (Chaitanya, Stanislav) - Move display code to use RMW whenever possible (Andrzej) - PSR fixes (Jouni, Ville) - Implement async_flip mode per plane tracking (Andrzej) - Remove pre-production Workarounds (Matt) - HDMI related fixes (Ankit) - LVDS cleanup (Ville) - Watermark fixes and cleanups (Ville, Jani, Stanilav) - DMC code related fixes, cleanups and improvements (Jani) - Implement fb_dirty for PSR,FBC,DRRS fixes (Jouni) - Initial DSB improvements targeting LUTs loading (Ville) - HWMON related fixes (Ashutosh) - PCI ID updates (Jonathan, Matt Roper) - Fix leak in scatterlist (Matt Atwood) - Fix eDP+DSI dual panel systems (Ville) - Cast iomem to avoid sparese warnings (Jani) - Set default backlight controller index (Jani) - More MTL enabling (RK) - Conversion of display dev_priv towards i915 (Nirmoy) - Improvements in log/debug messages (Ville) - Increase slice_height for DP VDSC (Suraj) - VBT ports improvements (Ville) - Fix platforms without Display (Imre) - Other generic display code clean-ups (Ville, Jani, Rodrigo) - Add RPL-U sub platform (Chaitanya) - Add inverted backlight quirk for HP 14-r206nv (Mavroudis) - Transcoder timing improvements (Ville) - Track audio state per-transcoder (Ville) - Error/underrun interrupt fixes (Ville) - Update combo PHY init sequence (Matt Roper) - Get HDR DPCD refresh timeout (Ville) - Vblank improvements (Ville) - DSS fixes and cleanups (Jani) - PM code cleanup (Jani) - Split display parts related to RPS (Jani) Thanks, Rodrigo. The following changes since commit d3eb347da1148fdb1c2462ae83090a4553d3f46f: drm/i915/mtl: Apply Wa_14013475917 for all MTL steppings (2023-01-26 13:54:05 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2023-03-07 for you to fetch changes up to 4b736ed40583631e0cf32c55dbc1e5ec0434a74b: drm/i915: Get rid of the gm45 HPD live state nonsense (2023-03-07 19:09:20 +0200) Cross-subsystem Changes: - MEI patches to fix suspend/resume issues with the i915's PXP. (Alexander) Driver Changes: - Registers helpers and clean-ups. (Lucas) - PXP fixes and clean-ups. (Alan, Alexander) - CDCLK related fixes and w/a (Chaitanya, Stanislav) - Move display code to use RMW whenever possible (Andrzej) - PSR fixes (Jouni, Ville) - Implement async_flip mode per plane tracking (Andrzej) - Remove pre-production Workarounds (Matt) - HDMI related fixes (Ankit) - LVDS cleanup (Ville) - Watermark fixes and cleanups (Ville, Jani, Stanilav) - DMC code related fixes, cleanups and improvements (Jani) - Implement fb_dirty for PSR,FBC,DRRS fixes (Jouni) - Initial DSB improvements targeting LUTs loading (Ville) - HWMON related fixes (Ashutosh) - PCI ID updates (Jonathan, Matt Roper) - Fix leak in scatterlist (Matt Atwood) - Fix eDP+DSI dual panel systems (Ville) - Cast iomem to avoid sparese warnings (Jani) - Set default backlight controller index (Jani) - More MTL enabling (RK) - Conversion of display dev_priv towards i915 (Nirmoy) - Improvements in log/debug messages (Ville) - Increase slice_height for DP VDSC (Suraj) - VBT ports improvements (Ville) - Fix platforms without Display (Imre) - Other generic display code clean-ups (Ville, Jani, Rodrigo) - Add RPL-U sub platform (Chaitanya) - Add inverted backlight quirk for HP 14-r206nv (Mavroudis) - Transcoder timing improvements (Ville) - Track audio state per-transcoder (Ville) - Error/underrun interrupt fixes (Ville) - Update combo PHY init sequence (Matt Roper) - Get HDR DPCD refresh timeout (Ville) - Vblank improvements (Ville) - DSS fixes and cleanups (Jani) - PM code cleanup (Jani) - Split display parts related to RPS (Jani) Alan Previn (3): drm/i915/pxp: Invalidate all PXP fw sessions during teardown drm/i915/pxp: Trigger the global teardown for before suspending drm/i915/pxp: Pxp hw init should be in resume_complete Alexander Usyskin (3): mei: mei-me: resume device in prepare drm/i915/pxp: add device link between i915 and mei_pxp mei: clean pending read with vtag on bus Andrzej Hajda (14): drm/i915/display/fdi: use intel_de_rmw if possible drm/i915/display/vlv: fix pixel overlap register update drm/i915/display/vlv: use intel_de_rmw if possible drm/i915/display/dsi: use intel_de_rmw if possible drm/i915: implement async_flip mode per plane tracking drm/i915/display/core: use intel_de_rmw if possible drm/i915/display/dpll: use intel_de_rmw if possible drm/i915/display/phys: use intel_de_rmw if possible
Re: [PATCH] drm/format_helper: Add Kunit tests for drm_fb_xrgb8888_to_mono()
Javier Martinez Canillas writes:
[...]
>
>> +static size_t conversion_buf_size_mono(unsigned int dst_pitch, const struct
>> drm_rect *clip)
>> +{
>> +if (!dst_pitch) {
>> +unsigned int linepixels = drm_rect_width(clip) * 1;
>> +
>> +dst_pitch = DIV_ROUND_UP(linepixels, 8);
>> +}
>> +
>> +return dst_pitch * drm_rect_height(clip);
>> +}
>> +
>
> I don't think you need a new helper only for this. There are other
> formats that have sub-byte pixels, so you may want to instead make
> the existing conversion_buf_size() function more general.
>
> Could you please base on the following patch that I just posted?
>
> https://lists.freedesktop.org/archives/dri-devel/2023-March/394466.html
>
I've posted a v2 since the kernel robot found a build warning on v1:
https://lists.freedesktop.org/archives/dri-devel/2023-March/394473.html
This time I have also tested your patch rebased on top of my v2, and
your tests are passing too:
[22:46:16] == drm_test_fb_xrgb_to_mono ===
[22:46:16] [PASSED] single_pixel_source_buffer
[22:46:16] [PASSED] single_pixel_clip_rectangle
[22:46:16] [PASSED] well_known_colors
[22:46:16] [PASSED] destination_pitch
The version of your patch I that tested is the following:
>From def1b2c04c812d53ebcda17c2d34d16f311ad406 Mon Sep 17 00:00:00 2001
From: Arthur Grillo
Date: Thu, 2 Mar 2023 17:01:31 -0300
Subject: [PATCH] drm/format_helper: Add Kunit tests for
drm_fb_xrgb_to_mono()
Extend the existing test cases to test the conversion from XRGB to
monochromatic.
Signed-off-by: Arthur Grillo
Reviewed-by: Javier Martinez Canillas
---
.../gpu/drm/tests/drm_format_helper_test.c| 63 +++
1 file changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 84b5cc29c8fc..c9cd8a7741ee 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -67,6 +67,11 @@ struct convert_to_argb2101010_result {
const u32 expected[TEST_BUF_SIZE];
};
+struct convert_to_mono_result {
+ unsigned int dst_pitch;
+ const u8 expected[TEST_BUF_SIZE];
+};
+
struct convert_xrgb_case {
const char *name;
unsigned int pitch;
@@ -82,6 +87,7 @@ struct convert_xrgb_case {
struct convert_to_argb_result argb_result;
struct convert_to_xrgb2101010_result xrgb2101010_result;
struct convert_to_argb2101010_result argb2101010_result;
+ struct convert_to_mono_result mono_result;
};
static struct convert_xrgb_case convert_xrgb_cases[] = {
@@ -131,6 +137,10 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = 0,
.expected = { 0xFFF0 },
},
+ .mono_result = {
+ .dst_pitch = 0,
+ .expected = { 0x00 },
+ },
},
{
.name = "single_pixel_clip_rectangle",
@@ -181,6 +191,10 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = 0,
.expected = { 0xFFF0 },
},
+ .mono_result = {
+ .dst_pitch = 0,
+ .expected = { 0x00 },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -293,6 +307,15 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0xFC00, 0xC00F,
},
},
+ .mono_result = {
+ .dst_pitch = 0,
+ .expected = {
+ 0x01,
+ 0x02,
+ 0x00,
+ 0x03,
+ },
+ },
},
{
/* Randomly picked colors. Full buffer within the clip area. */
@@ -392,6 +415,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0xEA20300C, 0xDB1705CD, 0xC3844672, 0x,
0x,
},
},
+ .mono_result = {
+ .dst_pitch = 0,
+ .expected = {
+ 0x00,
+ 0x00,
+ 0x00,
+ },
+ },
},
};
@@ -792,6 +823,37 @@ static void drm_test_fb_xrgb_to_argb2101010(struct
kunit *test)
KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}
+static void
[PATCH v2] drm/format-helper: Make conversion_buf_size() support sub-byte pixel fmts
There are DRM fourcc formats that have pixels smaller than a byte, but the
conversion_buf_size() function assumes that pixels are a multiple of bytes
and use the struct drm_format_info .cpp field to calculate the dst_pitch.
Instead, calculate it by using the bits per pixel (bpp) and divide it by 8
to account for formats that have sub-byte pixels.
Signed-off-by: Javier Martinez Canillas
---
Tested by making sure that the following command still succeeds:
./tools/testing/kunit/kunit.py run \
--kunitconfig=drivers/gpu/drm/tests/.kunitconfig
Changes in v2:
- Drop an unused variable, that was pointed out by the kernel robot.
drivers/gpu/drm/tests/drm_format_helper_test.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 9536829c6e3a..84b5cc29c8fc 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -409,12 +409,15 @@ static size_t conversion_buf_size(u32 dst_format,
unsigned int dst_pitch,
const struct drm_rect *clip)
{
const struct drm_format_info *dst_fi = drm_format_info(dst_format);
+ unsigned int bpp;
if (!dst_fi)
return -EINVAL;
- if (!dst_pitch)
- dst_pitch = drm_rect_width(clip) * dst_fi->cpp[0];
+ if (!dst_pitch) {
+ bpp = drm_format_info_bpp(dst_fi, 0);
+ dst_pitch = DIV_ROUND_UP(drm_rect_width(clip) * bpp, 8);
+ }
return dst_pitch * drm_rect_height(clip);
}
--
2.39.2
Re: [PATCH 2/2] drm/i915/gsc: Fix race between HW init and GSC FW load
On Thu, 2023-02-23 at 09:21 -0800, Ceraolo Spurio, Daniele wrote: > The GSC FW load is a slow process (up to 250 ms), so we defer it to a > dedicated worker to avoid stalling the init flow for that long. However, > we currently start this worker before the HW init is complete, so there > is a chance that the GSC loading code submits to the HW before the > engine initialization has completed. We can easily fix this by starting > the thread later in the gt_resume flow. > From this later spot, the GSC code can still race with the default > submission code; we functionally don't care who wins the race (the GSC > load doesn't need any state), but since the whole point of the separate > worker is to make the main thread faster, we prefer the default > submission code to run first. Therefore, make an exception for driver > probe and only and start the gsc load from uc_init_late. > > Signed-off-by: Daniele Ceraolo Spurio > Cc: Alan Previn > --- alan:snip Simple fix... LGTM: Reviewed-by: Alan Previn
Re: [PATCH] drm/format-helper: Make conversion_buf_size() support sub-byte pixel fmts
Hi Javier,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.3-rc1 next-20230307]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-format-helper-Make-conversion_buf_size-support-sub-byte-pixel-fmts/20230308-033619
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20230307193457.331360-1-javierm%40redhat.com
patch subject: [PATCH] drm/format-helper: Make conversion_buf_size() support
sub-byte pixel fmts
config: m68k-allyesconfig
(https://download.01.org/0day-ci/archive/20230308/202303080420.t1vvhvxo-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.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/5f0f9d30de18661bdabebde361180893b8ddba27
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Javier-Martinez-Canillas/drm-format-helper-Make-conversion_buf_size-support-sub-byte-pixel-fmts/20230308-033619
git checkout 5f0f9d30de18661bdabebde361180893b8ddba27
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=m68k SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Link:
https://lore.kernel.org/oe-kbuild-all/202303080420.t1vvhvxo-...@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/tests/drm_format_helper_test.c: In function
'conversion_buf_size':
>> drivers/gpu/drm/tests/drm_format_helper_test.c:412:27: warning: variable
>> 'cpp' set but not used [-Wunused-but-set-variable]
412 | unsigned int bpp, cpp;
| ^~~
vim +/cpp +412 drivers/gpu/drm/tests/drm_format_helper_test.c
397
398 /*
399 * conversion_buf_size - Return the destination buffer size required to
convert
400 * between formats.
401 * @dst_format: destination buffer pixel format (DRM_FORMAT_*)
402 * @dst_pitch: Number of bytes between two consecutive scanlines within
dst
403 * @clip: Clip rectangle area to convert
404 *
405 * Returns:
406 * The size of the destination buffer or negative value on error.
407 */
408 static size_t conversion_buf_size(u32 dst_format, unsigned int
dst_pitch,
409const struct drm_rect *clip)
410 {
411 const struct drm_format_info *dst_fi =
drm_format_info(dst_format);
> 412 unsigned int bpp, cpp;
413
414 if (!dst_fi)
415 return -EINVAL;
416
417 if (!dst_pitch) {
418 bpp = drm_format_info_bpp(dst_fi, 0);
419 cpp = DIV_ROUND_UP(bpp, 8);
420 dst_pitch = DIV_ROUND_UP(drm_rect_width(clip) * bpp, 8);
421 }
422
423 return dst_pitch * drm_rect_height(clip);
424 }
425
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
[PATCH] habanalabs: Drop redundant pci_enable_pcie_error_reporting()
From: Bjorn Helgaas
pci_enable_pcie_error_reporting() enables the device to send ERR_*
Messages. Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is
native"), the PCI core does this for all devices during enumeration, so the
driver doesn't need to do it itself.
Remove the redundant pci_enable_pcie_error_reporting() call from the
driver. Also remove the corresponding pci_disable_pcie_error_reporting()
from the driver .remove() path.
Note that this only controls ERR_* Messages from the device. An ERR_*
Message may cause the Root Port to generate an interrupt, depending on the
AER Root Error Command register managed by the AER service driver.
Signed-off-by: Bjorn Helgaas
---
drivers/accel/habanalabs/common/habanalabs_drv.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/accel/habanalabs/common/habanalabs_drv.c
b/drivers/accel/habanalabs/common/habanalabs_drv.c
index 03dae57dc838..26f65aa21079 100644
--- a/drivers/accel/habanalabs/common/habanalabs_drv.c
+++ b/drivers/accel/habanalabs/common/habanalabs_drv.c
@@ -12,7 +12,6 @@
#include "../include/hw_ip/pci/pci_general.h"
#include
-#include
#include
#define CREATE_TRACE_POINTS
@@ -550,8 +549,6 @@ static int hl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
pci_set_drvdata(pdev, hdev);
- pci_enable_pcie_error_reporting(pdev);
-
rc = hl_device_init(hdev, hl_class);
if (rc) {
dev_err(>dev, "Fatal error during habanalabs device
init\n");
@@ -562,7 +559,6 @@ static int hl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
return 0;
disable_device:
- pci_disable_pcie_error_reporting(pdev);
pci_set_drvdata(pdev, NULL);
destroy_hdev(hdev);
@@ -585,7 +581,6 @@ static void hl_pci_remove(struct pci_dev *pdev)
return;
hl_device_fini(hdev);
- pci_disable_pcie_error_reporting(pdev);
pci_set_drvdata(pdev, NULL);
destroy_hdev(hdev);
}
--
2.25.1
[PATCH] drm/amdgpu: Drop redundant pci_enable_pcie_error_reporting()
From: Bjorn Helgaas
pci_enable_pcie_error_reporting() enables the device to send ERR_*
Messages. Since f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is
native"), the PCI core does this for all devices during enumeration, so the
driver doesn't need to do it itself.
Remove the redundant pci_enable_pcie_error_reporting() call from the
driver.
Note that this only controls ERR_* Messages from the device. An ERR_*
Message may cause the Root Port to generate an interrupt, depending on the
AER Root Error Command register managed by the AER service driver.
Signed-off-by: Bjorn Helgaas
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 --
2 files changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 164141bc8b4a..208cebb40232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,7 +50,6 @@
#include
#include
#include
-#include
#include
#include
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c4a4e2fe6681..a5151e83a3f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3773,8 +3773,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
}
}
- pci_enable_pcie_error_reporting(adev->pdev);
-
/* Post card if necessary */
if (amdgpu_device_need_post(adev)) {
if (!adev->bios) {
--
2.25.1
Re: [Intel-gfx] [PATCH v2 6/7] drm/ttm: Reduce the number of used allocation orders for TTM pages
Hi Thomas, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm-intel/for-linux-next] [cannot apply to drm-tip/drm-tip] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Hellstr-m/drm-ttm-Fix-a-NULL-pointer-dereference/20230307-224931 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230307144621.10748-7-thomas.hellstrom%40linux.intel.com patch subject: [Intel-gfx] [PATCH v2 6/7] drm/ttm: Reduce the number of used allocation orders for TTM pages config: powerpc-randconfig-r006-20230306 (https://download.01.org/0day-ci/archive/20230308/202303080352.azyewwwt-...@intel.com/config) compiler: powerpc-linux-gcc (GCC) 12.1.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/0eee47dba298051fc49965d56cb17dd113ff0236 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Thomas-Hellstr-m/drm-ttm-Fix-a-NULL-pointer-dereference/20230307-224931 git checkout 0eee47dba298051fc49965d56cb17dd113ff0236 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/gpu/drm/ttm/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202303080352.azyewwwt-...@intel.com/ All error/warnings (new ones prefixed by >>): In function 'ttm_pool_type_init', inlined from 'ttm_pool_init' at drivers/gpu/drm/ttm/ttm_pool.c:557:5: >> drivers/gpu/drm/ttm/ttm_pool.c:264:18: warning: iteration 9 invokes >> undefined behavior [-Waggressive-loop-optimizations] 264 | pt->pool = pool; | ~^~ drivers/gpu/drm/ttm/ttm_pool.c: In function 'ttm_pool_init': drivers/gpu/drm/ttm/ttm_pool.c:556:39: note: within this loop 556 | for (j = 0; j < TTM_DIM_ORDER; ++j) | ^ In file included from : drivers/gpu/drm/ttm/ttm_pool.c: In function 'ttm_pool_mgr_init': >> include/linux/compiler_types.h:358:45: error: call to >> '__compiletime_assert_283' declared with attribute error: BUILD_BUG_ON >> failed: TTM_DIM_ORDER > MAX_ORDER 358 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^ include/linux/compiler_types.h:339:25: note: in definition of macro '__compiletime_assert' 339 | prefix ## suffix(); \ | ^~ include/linux/compiler_types.h:358:9: note: in expansion of macro '_compiletime_assert' 358 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) | ^~ include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG' 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) | ^~~~ drivers/gpu/drm/ttm/ttm_pool.c:744:9: note: in expansion of macro 'BUILD_BUG_ON' 744 | BUILD_BUG_ON(TTM_DIM_ORDER > MAX_ORDER); | ^~~~ vim +/__compiletime_assert_283 +358 include/linux/compiler_types.h eb5c2d4b45e3d2 Will Deacon 2020-07-21 344 eb5c2d4b45e3d2 Will Deacon 2020-07-21 345 #define _compiletime_assert(condition, msg, prefix, suffix) \ eb5c2d4b45e3d2 Will Deacon 2020-07-21 346 __compiletime_assert(condition, msg, prefix, suffix) eb5c2d4b45e3d2 Will Deacon 2020-07-21 347 eb5c2d4b45e3d2 Will Deacon 2020-07-21 348 /** eb5c2d4b45e3d2 Will Deacon 2020-07-21 349 * compiletime_assert - break build and emit msg if condition is false eb5c2d4b45e3d2 Will Deacon 2020-07-21 350 * @condition: a compile-time constant condition to check eb5c2d4b45e3d2 Will Deacon 2020-07-21 351 * @msg: a message to emit if condition is false eb5c2d4b45e3d2 Will Deacon 2020-07-21 352 * eb5c2d4b45e3d2 Will De
Re: [PATCH] drm/format_helper: Add Kunit tests for drm_fb_xrgb8888_to_mono()
Hello Arthur,
Thanks a lot for your patch!
Arthur Grillo writes:
> Extend the existing test cases to test the conversion from XRGB to
> monochromatic.
>
> Signed-off-by: Arthur Grillo
> ---
[...]
> +static size_t conversion_buf_size_mono(unsigned int dst_pitch, const struct
> drm_rect *clip)
> +{
> + if (!dst_pitch) {
> + unsigned int linepixels = drm_rect_width(clip) * 1;
> +
> + dst_pitch = DIV_ROUND_UP(linepixels, 8);
> + }
> +
> + return dst_pitch * drm_rect_height(clip);
> +}
> +
I don't think you need a new helper only for this. There are other
formats that have sub-byte pixels, so you may want to instead make
the existing conversion_buf_size() function more general.
Could you please base on the following patch that I just posted?
https://lists.freedesktop.org/archives/dri-devel/2023-March/394466.html
I believe with that you should be able to drop this format specific
helper and just use the fourcc DRM_FORMAT_C1 instead.
[...]
>
> +static void drm_test_fb_xrgb_to_mono(struct kunit *test)
> +{
> + const struct convert_xrgb_case *params = test->param_value;
> + const struct convert_to_mono_result *result = >mono_result;
> + size_t dst_size;
> + u8 *buf = NULL;
> + __le32 *xrgb = NULL;
> + struct iosys_map dst, src;
> +
> + struct drm_framebuffer fb = {
> + .format = drm_format_info(DRM_FORMAT_XRGB),
> + .pitches = { params->pitch, 0, 0 },
> + };
> +
> + dst_size = conversion_buf_size_mono(result->dst_pitch, >clip);
> +
Then here you could just do:
dst_size = conversion_buf_size(DRM_FORMAT_C1, result->dst_pitch,
>clip);
If you do that, feel free to add:
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
[PATCH] drm/format-helper: Make conversion_buf_size() support sub-byte pixel fmts
There are DRM fourcc formats that have pixels smaller than a byte, but the
conversion_buf_size() function assumes that pixels are a multiple of bytes
and use the struct drm_format_info .cpp field to calculate the dst_pitch.
Instead, calculate it by using the bits per pixel (bpp) and char per pixel
(cpp) to account for formats that have sub-byte pixels.
Signed-off-by: Javier Martinez Canillas
---
Tested by making sure that the following command still succeeds:
./tools/testing/kunit/kunit.py run \
--kunitconfig=drivers/gpu/drm/tests/.kunitconfig
drivers/gpu/drm/tests/drm_format_helper_test.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 9536829c6e3a..f200347a1db7 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -409,12 +409,16 @@ static size_t conversion_buf_size(u32 dst_format,
unsigned int dst_pitch,
const struct drm_rect *clip)
{
const struct drm_format_info *dst_fi = drm_format_info(dst_format);
+ unsigned int bpp, cpp;
if (!dst_fi)
return -EINVAL;
- if (!dst_pitch)
- dst_pitch = drm_rect_width(clip) * dst_fi->cpp[0];
+ if (!dst_pitch) {
+ bpp = drm_format_info_bpp(dst_fi, 0);
+ cpp = DIV_ROUND_UP(bpp, 8);
+ dst_pitch = DIV_ROUND_UP(drm_rect_width(clip) * bpp, 8);
+ }
return dst_pitch * drm_rect_height(clip);
}
--
2.39.2
Re: [Freedreno] [PATCH] drm/msm: fix PM_DEVFREQ kconfig dependency warning
On Tue, Mar 7, 2023 at 10:48 AM Fabio Estevam wrote:
>
> On Tue, Mar 7, 2023 at 2:46 PM Randy Dunlap wrote:
> >
> > Since DEVFREQ_GOV_SIMPLE_ONDEMAND depends on PM_DEVFREQ, the latter
> > should either be selected or DRM_MSM should depend on PM_DEVFREQ.
> > Since most drivers select PM_DEVFREQ instead of depending on it,
> > add a select here to satisfy kconfig.
> >
> > WARNING: unmet direct dependencies detected for DEVFREQ_GOV_SIMPLE_ONDEMAND
> > Depends on [n]: PM_DEVFREQ [=n]
> > Selected by [y]:
> > - DRM_MSM [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARCH_QCOM || SOC_IMX5 ||
> > COMPILE_TEST [=y]) && COMMON_CLK [=y] && IOMMU_SUPPORT [=y] && (QCOM_OCMEM
> > [=n] || QCOM_OCMEM [=n]=n) && (QCOM_LLCC [=n] || QCOM_LLCC [=n]=n) &&
> > (QCOM_COMMAND_DB [=y] || QCOM_COMMAND_DB [=y]=n)
> >
> > Fixes: 6563f60f14cb ("drm/msm/gpu: Add devfreq tuning debugfs")
> > Signed-off-by: Randy Dunlap
> > Reported-by: kernel test robot
> > Link: lore.kernel.org/r/202303071922.wjqdwqpe-...@intel.com
> > Cc: Rob Clark
> > Cc: Paul Gazzillo
> > Cc: Necip Fazil Yildiran
> > Cc: Chia-I Wu
> > Cc: Abhinav Kumar
> > Cc: Dmitry Baryshkov
> > Cc: linux-arm-...@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: freedr...@lists.freedesktop.org
>
> This fixes the warning after running 'make imx_v6_v7_defconfig', thanks:
>
> Tested-by: Fabio Estevam
https://patchwork.freedesktop.org/patch/523353 is the fix we actually
want.. I thought I'd already pulled that into msm-fixes but it seems
like it got lost somewhere.. I'll rectify that
BR,
-R
Re: [PATCH] drm/msm: fix PM_DEVFREQ kconfig dependency warning
On Tue, Mar 7, 2023 at 2:46 PM Randy Dunlap wrote:
>
> Since DEVFREQ_GOV_SIMPLE_ONDEMAND depends on PM_DEVFREQ, the latter
> should either be selected or DRM_MSM should depend on PM_DEVFREQ.
> Since most drivers select PM_DEVFREQ instead of depending on it,
> add a select here to satisfy kconfig.
>
> WARNING: unmet direct dependencies detected for DEVFREQ_GOV_SIMPLE_ONDEMAND
> Depends on [n]: PM_DEVFREQ [=n]
> Selected by [y]:
> - DRM_MSM [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARCH_QCOM || SOC_IMX5 ||
> COMPILE_TEST [=y]) && COMMON_CLK [=y] && IOMMU_SUPPORT [=y] && (QCOM_OCMEM
> [=n] || QCOM_OCMEM [=n]=n) && (QCOM_LLCC [=n] || QCOM_LLCC [=n]=n) &&
> (QCOM_COMMAND_DB [=y] || QCOM_COMMAND_DB [=y]=n)
>
> Fixes: 6563f60f14cb ("drm/msm/gpu: Add devfreq tuning debugfs")
> Signed-off-by: Randy Dunlap
> Reported-by: kernel test robot
> Link: lore.kernel.org/r/202303071922.wjqdwqpe-...@intel.com
> Cc: Rob Clark
> Cc: Paul Gazzillo
> Cc: Necip Fazil Yildiran
> Cc: Chia-I Wu
> Cc: Abhinav Kumar
> Cc: Dmitry Baryshkov
> Cc: linux-arm-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
This fixes the warning after running 'make imx_v6_v7_defconfig', thanks:
Tested-by: Fabio Estevam
Re: [PATCH v3 01/10] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible
On Tue, 07 Mar 2023 14:01:39 +0100, Konrad Dybcio wrote:
> The qcom, prefix was missed previously. Fix it.
>
> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible
> strings for every current SoC")
> Acked-by: Rob Herring
> Signed-off-by: Konrad Dybcio
> ---
> Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
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/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.example.dtb:
dsi@5e94000: compatible: 'oneOf' conditional failed, one must be fixed:
['qcom,dsi-ctrl-6g-qcm2290'] is too short
'qcom,dsi-ctrl-6g-qcm2290' is not one of ['qcom,apq8064-dsi-ctrl',
'qcom,msm8916-dsi-ctrl', 'qcom,msm8953-dsi-ctrl', 'qcom,msm8974-dsi-ctrl',
'qcom,msm8996-dsi-ctrl', 'qcom,msm8998-dsi-ctrl', 'qcom,qcm2290-dsi-ctrl',
'qcom,sc7180-dsi-ctrl', 'qcom,sc7280-dsi-ctrl', 'qcom,sdm660-dsi-ctrl',
'qcom,sdm845-dsi-ctrl', 'qcom,sm8150-dsi-ctrl', 'qcom,sm8250-dsi-ctrl',
'qcom,sm8350-dsi-ctrl', 'qcom,sm8450-dsi-ctrl', 'qcom,sm8550-dsi-ctrl']
From schema:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.example.dtb:
dsi@5e94000: Unevaluated properties are not allowed ('compatible' was
unexpected)
From schema:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.example.dtb:
dsi@5e94000: compatible: 'oneOf' conditional failed, one must be fixed:
['qcom,dsi-ctrl-6g-qcm2290'] is too short
'qcom,dsi-ctrl-6g-qcm2290' is not one of ['qcom,apq8064-dsi-ctrl',
'qcom,msm8916-dsi-ctrl', 'qcom,msm8953-dsi-ctrl', 'qcom,msm8974-dsi-ctrl',
'qcom,msm8996-dsi-ctrl', 'qcom,msm8998-dsi-ctrl', 'qcom,qcm2290-dsi-ctrl',
'qcom,sc7180-dsi-ctrl', 'qcom,sc7280-dsi-ctrl', 'qcom,sdm660-dsi-ctrl',
'qcom,sdm845-dsi-ctrl', 'qcom,sm8150-dsi-ctrl', 'qcom,sm8250-dsi-ctrl',
'qcom,sm8350-dsi-ctrl', 'qcom,sm8450-dsi-ctrl', 'qcom,sm8550-dsi-ctrl']
From schema:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.example.dtb:
dsi@5e94000: Unevaluated properties are not allowed ('compatible' was
unexpected)
From schema:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
doc reference errors (make refcheckdocs):
See
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230307-topic-dsi_qcm-v3-1-8bd7e1add...@linaro.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v12 10/11] drm/virtio: Support memory shrinking
On 3/7/23 21:25, Dmitry Osipenko wrote: >> Not really a problem with this patchset, but having such branches looks >> like a bug in the driver's GEM design. Whatever your GEM object needs or >> does, it should be hidden in the implementation. Why is virtio doing this? > There is another "VRAM" VirtIO-GPU BO type that doesn't implement the > pin/unpin callbacks. Perhaps another option was to add the callbacks. Although, the pin/unpin are optional. So yes, there was no need for the extra branch, good catch. -- Best regards, Dmitry
Re: [PATCH v2 39/50] drm/msm/dpu: drop duplicate vig_sblk instances
On 12/02/2023 01:12, Dmitry Baryshkov wrote:
After fixing scaler version we are sure that sm8450 and sc8280xp vig
sblk's are duplicates of sm8250_vig_sblk and thus can be dropped.
I will probably partially withdraw or rework this patch to fix the
scaler->version handling. The rest of the patches are still in the queue.
Signed-off-by: Dmitry Baryshkov
---
.../msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 8
.../drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 8
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 18 --
3 files changed, 8 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
index 094876b1019b..05b7ae3f0343 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
@@ -85,13 +85,13 @@ static const struct dpu_ctl_cfg sc8280xp_ctl[] = {
/* FIXME: check block length */
static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x328, VIG_SC7180_MASK,
-sc8280xp_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
+sm8250_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, 0x328, VIG_SC7180_MASK,
-sc8280xp_vig_sblk_1, 4, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
+sm8250_vig_sblk_1, 4, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, 0x328, VIG_SC7180_MASK,
-sc8280xp_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
+sm8250_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, 0x328, VIG_SC7180_MASK,
-sc8280xp_vig_sblk_3, 12, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
+sm8250_vig_sblk_3, 12, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x328, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x328, DMA_SDM845_MASK,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
index 3d95f2472e7a..d4f710481b78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
@@ -84,13 +84,13 @@ static const struct dpu_ctl_cfg sm8450_ctl[] = {
static const struct dpu_sspp_cfg sm8450_sspp[] = {
SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x328, VIG_SC7180_MASK,
- sm8450_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
+ sm8250_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, 0x328, VIG_SC7180_MASK,
- sm8450_vig_sblk_1, 4, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
+ sm8250_vig_sblk_1, 4, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, 0x328, VIG_SC7180_MASK,
- sm8450_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
+ sm8250_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, 0x328, VIG_SC7180_MASK,
- sm8450_vig_sblk_3, 12, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
+ sm8250_vig_sblk_3, 12, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x328, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x328, DMA_SDM845_MASK,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d84f8eb8f88a..5091cec30bfc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -411,15 +411,6 @@ static const struct dpu_sspp_sub_blks sm8250_vig_sblk_2 =
static const struct dpu_sspp_sub_blks sm8250_vig_sblk_3 =
_VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED4);
-static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
- _VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED4);
-static const struct dpu_sspp_sub_blks sm8450_vig_sblk_1 =
- _VIG_SBLK("1", 6, DPU_SSPP_SCALER_QSEED4);
-static const struct dpu_sspp_sub_blks sm8450_vig_sblk_2 =
- _VIG_SBLK("2", 7, DPU_SSPP_SCALER_QSEED4);
-static const struct dpu_sspp_sub_blks sm8450_vig_sblk_3 =
- _VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED4);
-
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
_VIG_SBLK("0", 7, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_1 =
@@ -431,15 +422,6 @@ static const struct dpu_sspp_sub_blks sm8550_vig_sblk_3 =
static const struct dpu_sspp_sub_blks
Re: [PATCH v12 10/11] drm/virtio: Support memory shrinking
On 3/7/23 13:42, Thomas Zimmermann wrote:
> Hi
>
> Am 05.03.23 um 23:10 schrieb Dmitry Osipenko:
> [...]
>> *bo_ptr = bo;
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> index 4c09e313bebc..3f21512ff153 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> @@ -238,20 +238,32 @@ static int virtio_gpu_plane_prepare_fb(struct
>> drm_plane *plane,
>> struct virtio_gpu_device *vgdev = dev->dev_private;
>> struct virtio_gpu_framebuffer *vgfb;
>> struct virtio_gpu_object *bo;
>> + int err;
>> 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 || (plane->type == DRM_PLANE_TYPE_PRIMARY &&
>> !bo->guest_blob))
>> +
>> + if (virtio_gpu_is_shmem(bo)) {
>
> Not really a problem with this patchset, but having such branches looks
> like a bug in the driver's GEM design. Whatever your GEM object needs or
> does, it should be hidden in the implementation. Why is virtio doing this?
There is another "VRAM" VirtIO-GPU BO type that doesn't implement the
pin/unpin callbacks. Perhaps another option was to add the callbacks.
>> + err = drm_gem_pin(>base.base);
>
> As the driver uses GEM SHMEM, please call drm_gem_shmem_object_pin()
> directly and remove patch [09/11]. Same goes for the _unpin call below.
>
> The problem with generic pinning interfaces is that there's no way of
> specifying where to pin to BO. The problem is most apparent with TTM,
> where hardware often has multiple locations were buffer can be placed
> (VRAM, GART, system memory). So it's really a detail between the driver
> and the memory manager.
>
> These generic internal GEM interfaces should only be used by DRM core
> and helpers. Drivers should use their memory manager's interface.
I'll switch to use drm_gem_shmem_object_pin() directly in v13, maybe add
virtio_gpu_gem_pin() helper for that. Thanks!
--
Best regards,
Dmitry
Re: [Freedreno] [PATCH v2 00/50] drm/msm/dpu: rework HW catalog
Hi Dmitry On 3/7/2023 10:02 AM, Dmitry Baryshkov wrote: On 12/02/2023 01:12, Dmitry Baryshkov wrote: This huge series attempts to restructure the DPU HW catalog into a manageable and reviewable data set. In order to ease review and testing I merged all the necessary fixes into this series. Also I cherry-picked & slightly fixed Konrad's patch adding size to the SSPP and INTF macros. First 12 patches are catalog fixes, which can be probably picked into the msm-fixes. Next 5 patches clean up the catalog a bit in order to make it more suitable for refactoring. Then the next batch of 13 + 5 patches split the hw catalog entries into per-SoC files. Next 8 patches rework catalog entries, mostly targeting QSEED cleanup and deduplication of data used by several platforms. At this moment only three pairs (out of 13 devices supported by DPU) are merged. However this part lays out the ground to ease adding support for new platforms, some of which use the same configuration as the existing platforms Last batch of 7 patches renames existing macros to ease using them while adding support for new devices. This pile of patches is submitted in a sinle batch to allow one to observe the final goal of the cleanup which otherwise might be hard to assess. Changes since v1: - Picked up Konrad's patch - Picked up dependencies into the main series - Moved qseed3lite vs qseed4 patches into the fixes part - Fixed sm6115 in a similar manner. Colleagues, could please take a look at this patchset? If nobody objects, I'd like to pick it after Rob merges Abhinav's msm-fixes pull request. Patches 1-13 are going through msm-fixes, patches 14-50 are pending. I will take a look at patches 14-50 by Friday or worst case early next week. This week, I plan to finish validating the wide planes on sc7280 and give my Tested-by, hence cant get to this before that. Thanks Abhinav Dmitry Baryshkov (49): drm/msm/dpu: set DPU_MDP_PERIPH_0_REMOVED for sc8280xp drm/msm/dpu: disable features unsupported by QCM2290 drm/msm/dpu: fix typo in in sm8550's dma_sblk_5 drm/msm/dpu: fix len of sc7180 ctl blocks drm/msm/dpu: fix sm6115 and qcm2290 mixer width limits drm/msm/dpu: correct sm8550 scaler drm/msm/dpu: correct sc8280xp scaler drm/msm/dpu: correct sm8450 scaler drm/msm/dpu: correct sm8250 and sm8350 scaler drm/msm/dpu: correct sm6115 scaler drm/msm/dpu: drop DPU_DIM_LAYER from MIXER_MSM8998_MASK drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks drm/msm/dpu: constify DSC data structures drm/msm/dpu: mark remaining pp data as const drm/msm/dpu: move UBWC/memory configuration to separate struct drm/msm/dpu: split SM8550 catalog entry to the separate file drm/msm/dpu: split SM8450 catalog entry to the separate file drm/msm/dpu: split SC8280XP catalog entry to the separate file drm/msm/dpu: split SC7280 catalog entry to the separate file drm/msm/dpu: split SM8350 catalog entry to the separate file drm/msm/dpu: split SM6115 catalog entry to the separate file drm/msm/dpu: split QCM2290 catalog entry to the separate file drm/msm/dpu: split SC7180 catalog entry to the separate file drm/msm/dpu: split SM8250 catalog entry to the separate file drm/msm/dpu: split SC8180X catalog entry to the separate file drm/msm/dpu: split SM8150 catalog entry to the separate file drm/msm/dpu: split MSM8998 catalog entry to the separate file drm/msm/dpu: split SDM845 catalog entry to the separate file drm/msm/dpu: duplicate sdm845 catalog entries drm/msm/dpu: duplicate sc7180 catalog entries drm/msm/dpu: duplicate sm8150 catalog entries drm/msm/dpu: duplicate sm8250 catalog entries drm/msm/dpu: duplicate sm8350 catalog entries drm/msm/dpu: use defined symbol for sc8280xp's maxwidth drm/msm/dpu: enable DPU_CTL_SPLIT_DISPLAY for sc8280xp drm/msm/dpu: enable DSPP_2/3 for LM_2/3 on sm8450 drm/msm/dpu: drop duplicate vig_sblk instances drm/msm/dpu: enable DSPP on sc8180x drm/msm/dpu: deduplicate sc8180x with sm8150 drm/msm/dpu: deduplicate sm6115 with qcm2290 drm/msm/dpu: deduplicate sc8280xp with sm8450 drm/msm/dpu: drop unused macros from hw catalog drm/msm/dpu: inline IRQ_n_MASK defines drm/msm/dpu: rename INTF_foo_MASK to contain major DPU version drm/msm/dpu: rename CTL_foo_MASK to contain major DPU version drm/msm/dpu: rename VIG and DMA_foo_MASK to contain major DPU version drm/msm/dpu: rename MIXER_foo_MASK to contain major DPU version drm/msm/dpu: rename MERGE_3D_foo_MASK to contain major DPU version Konrad Dybcio (1): drm/msm/dpu: Allow variable SSPP/INTF_BLK size .../msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 211 ++ .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 211 ++ .../msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 97 + .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 91 +
Re: [PATCH v2 00/50] drm/msm/dpu: rework HW catalog
On 12/02/2023 01:12, Dmitry Baryshkov wrote: This huge series attempts to restructure the DPU HW catalog into a manageable and reviewable data set. In order to ease review and testing I merged all the necessary fixes into this series. Also I cherry-picked & slightly fixed Konrad's patch adding size to the SSPP and INTF macros. First 12 patches are catalog fixes, which can be probably picked into the msm-fixes. Next 5 patches clean up the catalog a bit in order to make it more suitable for refactoring. Then the next batch of 13 + 5 patches split the hw catalog entries into per-SoC files. Next 8 patches rework catalog entries, mostly targeting QSEED cleanup and deduplication of data used by several platforms. At this moment only three pairs (out of 13 devices supported by DPU) are merged. However this part lays out the ground to ease adding support for new platforms, some of which use the same configuration as the existing platforms Last batch of 7 patches renames existing macros to ease using them while adding support for new devices. This pile of patches is submitted in a sinle batch to allow one to observe the final goal of the cleanup which otherwise might be hard to assess. Changes since v1: - Picked up Konrad's patch - Picked up dependencies into the main series - Moved qseed3lite vs qseed4 patches into the fixes part - Fixed sm6115 in a similar manner. Colleagues, could please take a look at this patchset? If nobody objects, I'd like to pick it after Rob merges Abhinav's msm-fixes pull request. Patches 1-13 are going through msm-fixes, patches 14-50 are pending. Dmitry Baryshkov (49): drm/msm/dpu: set DPU_MDP_PERIPH_0_REMOVED for sc8280xp drm/msm/dpu: disable features unsupported by QCM2290 drm/msm/dpu: fix typo in in sm8550's dma_sblk_5 drm/msm/dpu: fix len of sc7180 ctl blocks drm/msm/dpu: fix sm6115 and qcm2290 mixer width limits drm/msm/dpu: correct sm8550 scaler drm/msm/dpu: correct sc8280xp scaler drm/msm/dpu: correct sm8450 scaler drm/msm/dpu: correct sm8250 and sm8350 scaler drm/msm/dpu: correct sm6115 scaler drm/msm/dpu: drop DPU_DIM_LAYER from MIXER_MSM8998_MASK drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks drm/msm/dpu: constify DSC data structures drm/msm/dpu: mark remaining pp data as const drm/msm/dpu: move UBWC/memory configuration to separate struct drm/msm/dpu: split SM8550 catalog entry to the separate file drm/msm/dpu: split SM8450 catalog entry to the separate file drm/msm/dpu: split SC8280XP catalog entry to the separate file drm/msm/dpu: split SC7280 catalog entry to the separate file drm/msm/dpu: split SM8350 catalog entry to the separate file drm/msm/dpu: split SM6115 catalog entry to the separate file drm/msm/dpu: split QCM2290 catalog entry to the separate file drm/msm/dpu: split SC7180 catalog entry to the separate file drm/msm/dpu: split SM8250 catalog entry to the separate file drm/msm/dpu: split SC8180X catalog entry to the separate file drm/msm/dpu: split SM8150 catalog entry to the separate file drm/msm/dpu: split MSM8998 catalog entry to the separate file drm/msm/dpu: split SDM845 catalog entry to the separate file drm/msm/dpu: duplicate sdm845 catalog entries drm/msm/dpu: duplicate sc7180 catalog entries drm/msm/dpu: duplicate sm8150 catalog entries drm/msm/dpu: duplicate sm8250 catalog entries drm/msm/dpu: duplicate sm8350 catalog entries drm/msm/dpu: use defined symbol for sc8280xp's maxwidth drm/msm/dpu: enable DPU_CTL_SPLIT_DISPLAY for sc8280xp drm/msm/dpu: enable DSPP_2/3 for LM_2/3 on sm8450 drm/msm/dpu: drop duplicate vig_sblk instances drm/msm/dpu: enable DSPP on sc8180x drm/msm/dpu: deduplicate sc8180x with sm8150 drm/msm/dpu: deduplicate sm6115 with qcm2290 drm/msm/dpu: deduplicate sc8280xp with sm8450 drm/msm/dpu: drop unused macros from hw catalog drm/msm/dpu: inline IRQ_n_MASK defines drm/msm/dpu: rename INTF_foo_MASK to contain major DPU version drm/msm/dpu: rename CTL_foo_MASK to contain major DPU version drm/msm/dpu: rename VIG and DMA_foo_MASK to contain major DPU version drm/msm/dpu: rename MIXER_foo_MASK to contain major DPU version drm/msm/dpu: rename MERGE_3D_foo_MASK to contain major DPU version Konrad Dybcio (1): drm/msm/dpu: Allow variable SSPP/INTF_BLK size .../msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 211 ++ .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h| 211 ++ .../msm/disp/dpu1/catalog/dpu_5_0_sm8150.h| 97 + .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 91 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_lm6.h | 152 ++ .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h| 244 ++ .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h| 152 ++ .../msm/disp/dpu1/catalog/dpu_6_3_sm6115.h| 92 + .../msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 84 +
Re: [PATCH v2 1/7] drm/ttm: Fix a NULL pointer dereference
On 3/7/23 17:55, Christian König wrote:
Am 07.03.23 um 15:46 schrieb Thomas Hellström:
The LRU mechanism may look up a resource in the process of being removed
from an object. The locking rules here are a bit unclear but it looks
currently like res->bo assignment is protected by the LRU lock, whereas
bo->resource is protected by the object lock, while *clearing* of
bo->resource is also protected by the LRU lock. This means that if
we check that bo->resource points to the LRU resource under the LRU
lock we should be safe.
So perform that check before deciding to swap out a bo. That avoids
dereferencing a NULL bo->resource in ttm_bo_swapout().
Please make sure that this is pushed to drm-misc-fixes ASAP.
I've getting complains for this from different sides.
Thanks,
Christian.
Done.
/Thomas
Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4")
Cc: Christian König
Cc: Daniel Vetter
Cc: Christian Koenig
Cc: Huang Rui
Cc: Alex Deucher
Cc: Felix Kuehling
Cc: Philip Yang
Cc: Qiang Yu
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Tvrtko Ursulin
Cc: "Thomas Hellström"
Cc: Anshuman Gupta
Cc: Arunpravin Paneer Selvam
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Thomas Hellström
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c
b/drivers/gpu/drm/ttm/ttm_device.c
index c7a1862f322a..ae2f19dc9f81 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev,
struct ttm_operation_ctx *ctx,
struct ttm_buffer_object *bo = res->bo;
uint32_t num_pages;
- if (!bo)
+ if (!bo || bo->resource != res)
continue;
num_pages = PFN_UP(bo->base.size);
[PATCH] drm/msm: fix PM_DEVFREQ kconfig dependency warning
Since DEVFREQ_GOV_SIMPLE_ONDEMAND depends on PM_DEVFREQ, the latter
should either be selected or DRM_MSM should depend on PM_DEVFREQ.
Since most drivers select PM_DEVFREQ instead of depending on it,
add a select here to satisfy kconfig.
WARNING: unmet direct dependencies detected for DEVFREQ_GOV_SIMPLE_ONDEMAND
Depends on [n]: PM_DEVFREQ [=n]
Selected by [y]:
- DRM_MSM [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARCH_QCOM || SOC_IMX5 ||
COMPILE_TEST [=y]) && COMMON_CLK [=y] && IOMMU_SUPPORT [=y] && (QCOM_OCMEM [=n]
|| QCOM_OCMEM [=n]=n) && (QCOM_LLCC [=n] || QCOM_LLCC [=n]=n) &&
(QCOM_COMMAND_DB [=y] || QCOM_COMMAND_DB [=y]=n)
Fixes: 6563f60f14cb ("drm/msm/gpu: Add devfreq tuning debugfs")
Signed-off-by: Randy Dunlap
Reported-by: kernel test robot
Link: lore.kernel.org/r/202303071922.wjqdwqpe-...@intel.com
Cc: Rob Clark
Cc: Paul Gazzillo
Cc: Necip Fazil Yildiran
Cc: Chia-I Wu
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
---
drivers/gpu/drm/msm/Kconfig |1 +
1 file changed, 1 insertion(+)
diff -- a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -23,6 +23,7 @@ config DRM_MSM
select SHMEM
select TMPFS
select QCOM_SCM
+ select PM_DEVFREQ
select DEVFREQ_GOV_SIMPLE_ONDEMAND
select WANT_DEV_COREDUMP
select SND_SOC_HDMI_CODEC if SND_SOC
Re: [PATCH] amdgpu: Avoid building on UML
Applied. Thanks. Alex On Mon, Mar 6, 2023 at 11:17 AM Felix Kuehling wrote: > > Looks like this patch got lost over the holidays. Alex, are you OK with > applying this patch? Or are people looking for a more general solution > to not build HW drivers for UML? FWIW: > > Acked-by: Felix Kuehling > > > Am 2023-01-12 um 23:30 schrieb Peter Foley: > > The amdgpu driver tries to use fields not supported by UML's cpuinfo > > struct. Disable the driver when targeting UML to avoid tripping up > > allyesconfig. > > > > e.g. > > ../drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c: In > > function ‘intel_core_rkl_chk’: > > ../drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1742:33: > > error: initialization of ‘struct cpuinfo_x86 *’ from incompatible pointer > > type ‘struct cpuinfo_um *’ [-Werror=incompatible-pointer-types > > ] > > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In function > > ‘kfd_cpumask_to_apic_id’: > > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2157:48: error: > > ‘struct cpuinfo_um’ has no member named ‘apicid’ > > > > Signed-off-by: Peter Foley > > --- > > drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > > b/drivers/gpu/drm/amd/amdgpu/Kconfig > > index 5fcd510f1abb..aa0008ff8712 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > > @@ -3,6 +3,7 @@ > > config DRM_AMDGPU > > tristate "AMD GPU" > > depends on DRM && PCI && MMU > > + depends on !UML > > select FW_LOADER > > select DRM_DISPLAY_DP_HELPER > > select DRM_DISPLAY_HDMI_HELPER > > > > --- > > base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2 > > change-id: 20230112-amduml-565935d34bfb > > > > Best regards,
[PATCH 6.2 0340/1001] drm/ast: Init iosys_map pointer as I/O memory for damage handling
From: Thomas Zimmermann
[ Upstream commit b1def7fadfa544bd2467581ce40b659583eb7e79 ]
Ast hardware scans out the primary plane from video memory, which
is in I/O-memory space. Hence init the damage handler's iosys_map
pointer as I/O memory.
Not all platforms support accessing I/O memory as system memory,
although it's usually not a problem in ast's x86-based systems.
The error report is at [1].
Reported-by: kernel test robot
Signed-off-by: Thomas Zimmermann
Reviewed-by: Jocelyn Falempe
Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
Cc: Thomas Zimmermann
Cc: Jocelyn Falempe
Cc: Dave Airlie
Cc: dri-devel@lists.freedesktop.org
Link: https://lore.kernel.org/lkml/202212170111.einm0uns-...@intel.com/T/#u # 1
Link:
https://patchwork.freedesktop.org/patch/msgid/20221216193005.30280-1-tzimmerm...@suse.de
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/ast/ast_mode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 66a4a41c3fe94..d314b9e7c05f9 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -636,7 +636,7 @@ static void ast_handle_damage(struct ast_plane *ast_plane,
struct iosys_map *src
struct drm_framebuffer *fb,
const struct drm_rect *clip)
{
- struct iosys_map dst = IOSYS_MAP_INIT_VADDR(ast_plane->vaddr);
+ struct iosys_map dst = IOSYS_MAP_INIT_VADDR_IOMEM(ast_plane->vaddr);
iosys_map_incr(, drm_fb_clip_offset(fb->pitches[0], fb->format,
clip));
drm_fb_memcpy(, fb->pitches, src, fb, clip);
--
2.39.2
[PATCH 6.2 0228/1001] drm/nouveau/disp: Fix nvif_outp_acquire_dp() argument size
From: Kees Cook
[ Upstream commit 4076ea2419cf15bc1e1580f8b24ddf675fbdb02c ]
Both Coverity and GCC with -Wstringop-overflow noticed that
nvif_outp_acquire_dp() accidentally defined its second argument with 1
additional element:
drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_pior_atomic_enable':
drivers/gpu/drm/nouveau/dispnv50/disp.c:1813:17: error: 'nvif_outp_acquire_dp'
accessing 16 bytes in a region of size 15 [-Werror=stringop-overflow=]
1813 | nvif_outp_acquire_dp(_encoder->outp,
nv_encoder->dp.dpcd, 0, 0, false, false);
|
^~~
drivers/gpu/drm/nouveau/dispnv50/disp.c:1813:17: note: referencing argument 2
of type 'u8[16]' {aka 'unsigned char[16]'}
drivers/gpu/drm/nouveau/include/nvif/outp.h:24:5: note: in a call to function
'nvif_outp_acquire_dp'
24 | int nvif_outp_acquire_dp(struct nvif_outp *, u8 dpcd[16],
| ^~~~
Avoid these warnings by defining the argument size using the matching
define (DP_RECEIVER_CAP_SIZE, 15) instead of having it be a literal
(and incorrect) value (16).
Reported-by: coverity-bot
Addresses-Coverity-ID: 1527269 ("Memory - corruptions")
Addresses-Coverity-ID: 1527268 ("Memory - corruptions")
Link: https://lore.kernel.org/lkml/202211100848.FFBA2432@keescook/
Link: https://lore.kernel.org/lkml/202211100848.F4C2819BB@keescook/
Fixes: 813443721331 ("drm/nouveau/disp: move DP link config into acquire")
Reviewed-by: Lyude Paul
Cc: Ben Skeggs
Cc: Karol Herbst
Cc: David Airlie
Cc: Daniel Vetter
Cc: Dave Airlie
Cc: "Gustavo A. R. Silva"
Cc: dri-devel@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Kees Cook
Link: https://lore.kernel.org/r/20221127183036.never.139-k...@kernel.org
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/nouveau/include/nvif/outp.h | 3 ++-
drivers/gpu/drm/nouveau/nvif/outp.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index 45daadec3c0c7..fa76a7b5e4b37 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -3,6 +3,7 @@
#define __NVIF_OUTP_H__
#include
#include
+#include
struct nvif_disp;
struct nvif_outp {
@@ -21,7 +22,7 @@ int nvif_outp_acquire_rgb_crt(struct nvif_outp *);
int nvif_outp_acquire_tmds(struct nvif_outp *, int head,
bool hdmi, u8 max_ac_packet, u8 rekey, u8 scdc, bool
hda);
int nvif_outp_acquire_lvds(struct nvif_outp *, bool dual, bool bpc8);
-int nvif_outp_acquire_dp(struct nvif_outp *, u8 dpcd[16],
+int nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE],
int link_nr, int link_bw, bool hda, bool mst);
void nvif_outp_release(struct nvif_outp *);
int nvif_outp_infoframe(struct nvif_outp *, u8 type, struct
nvif_outp_infoframe_v0 *, u32 size);
diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 7da39f1eae9fb..c24bc5eae3ecf 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -127,7 +127,7 @@ nvif_outp_acquire(struct nvif_outp *outp, u8 proto, struct
nvif_outp_acquire_v0
}
int
-nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[16],
+nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE],
int link_nr, int link_bw, bool hda, bool mst)
{
struct nvif_outp_acquire_v0 args;
--
2.39.2
Re: [regression] RPI4B drm vc4: no crtc or sizes since 5.17 (works in 5.16; and still broken in at least 6.1)
Hi Maarten On Tue, 7 Mar 2023 at 16:25, AL13N wrote: > > AL13N schreef op 2023-03-06 17:34: > > Hi, > > > > I have a RPI4B connected on 2nd HDMI port (furthest away from power) > > to a 4K TV, which works until 5.16, from 5.17 there is no X (or > > plymouth), the cause of no X is that EDID gives nothing, and in the > > journal; there is "Cannot find any crct or sizes". Only the kernel is > > changed for this. > > > > In 5.16 instead of this message there is a bunch of hex lines prefixed > > with BAD. > > > > It is still broken in 6.1 at the very least. > > > > I donno if this is related to this part, but I wanted to try a newer > > kernel, because the RPI4 seems to do all the video decoding in > > software and cannot seem to handle it. > > > > > > logs: > > vc4-drm gpu: bound fef05700.hdmi (ops vc4_hdmi_ops [vc4]) > > vc4-drm gpu: bound fe004000.txp (ops vc4_txp_ops [vc4]) > > vc4-drm gpu: bound fe206000.pixelvalve (ops vc4_crtc_ops [vc4]) > > vc4-drm gpu: bound fe207000.pixelvalve (ops vc4_crtc_ops [vc4]) > > vc4-drm gpu: bound fe20a000.pixelvalve (ops vc4_crtc_ops [vc4]) > > vc4-drm gpu: bound fe216000.pixelvalve (ops vc4_crtc_ops [vc4]) > > vc4-drm gpu: bound fec12000.pixelvalve (ops vc4_crtc_ops [vc4]) > > checking generic (3ea81000 12c000) vs hw (0 ) > > fb0: switching to vc4 from simple > > Console: switching to colour dummy device 80x25 > > [drm] Initialized vc4 0.0.0 20140616 for gpu on minor 0 > > vc4-drm gpu: [drm] Cannot find any crtc or sizes > > 5.16 log has: > > vc4-drm gpu: bound fef05700.hdmi (ops vc4_hdmi_ops [vc4]) > vc4-drm gpu: bound fe004000.txp (ops vc4_txp_ops [vc4]) > vc4-drm gpu: bound fe206000.pixelvalve (ops vc4_crtc_ops [vc4]) > vc4-drm gpu: bound fe207000.pixelvalve (ops vc4_crtc_ops [vc4]) > vc4-drm gpu: bound fe20a000.pixelvalve (ops vc4_crtc_ops [vc4]) > vc4-drm gpu: bound fe216000.pixelvalve (ops vc4_crtc_ops [vc4]) > vc4-drm gpu: bound fec12000.pixelvalve (ops vc4_crtc_ops [vc4]) > [drm] Initialized vc4 0.0.0 20140616 for gpu on minor 0 > [00] BAD 00 ff ff ff ff ff ff 00 36 74 00 00 00 00 00 00 > [00] BAD 0b 1f 01 03 00 23 01 78 0a cf 74 a3 57 4c b0 23 > [00] BAD 09 48 4c 00 00 00 01 01 01 ff 01 ff ff 01 01 01 > [00] BAD 01 01 01 01 01 20 08 e8 00 30 f2 70 5a 80 b0 58 > [00] BAD 8a 00 c4 8e 21 00 00 1e 02 3a 80 18 71 38 2d 40 > [00] BAD 58 2c 45 00 c4 8e 21 00 00 1e 00 00 00 fc 00 53 > [00] BAD 41 4c 4f 52 41 0a 20 20 20 20 20 20 00 00 00 fd > [00] BAD 00 3b 46 1f 8c 3c 00 0a 20 20 20 20 20 20 01 aa > Console: switching to colour frame buffer device 240x67 > vc4-drm gpu: [drm] fb0: vc4drmfb frame buffer device > > > i donno what this bad is, but it doesn't happen in 5.17... maybe these > BAD got filtered out, but they did end up working for me? or something? > i donno... Run it through edid-decode - the checksum is wrong. Block 0, Base EDID: EDID Structure Version & Revision: 1.3 Vendor & Product Identification: Manufacturer: MST Model: 0 Made in: week 11 of 2021 Basic Display Parameters & Features: Analog display Input voltage level: 0.7/0.3 V Blank level equals black level Maximum image size: 35 cm x 1 cm Gamma: 2.20 RGB color display First detailed timing is the preferred timing Color Characteristics: Red : 0.6396, 0.3398 Green: 0.2998, 0.6904 Blue : 0.1376, 0.0380 White: 0.2822, 0.2968 Established Timings I & II: none Standard Timings: GTF : 2288x1432 61.000 Hz 16:10 90.463 kHz 282.245 MHz Detailed Timing Descriptors: DTD 1: 3840x2160 60.000 Hz 16:9 135.000 kHz 594.000 MHz (708 mm x 398 mm) Hfront 176 Hsync 88 Hback 296 Hpol P Vfront8 Vsync 10 Vback 72 Vpol P DTD 2: 1920x1080 60.000 Hz 16:967.500 kHz 148.500 MHz (708 mm x 398 mm) Hfront 88 Hsync 44 Hback 148 Hpol P Vfront4 Vsync 5 Vback 36 Vpol P Display Product Name: 'SALORA' Display Range Limits: Monitor ranges (GTF): 59-70 Hz V, 31-140 kHz H, max dotclock 600 MHz Extension blocks: 1 Checksum: 0xaa (should be 0xeb) Weird that it also says that it's an analog display when it's connected over HDMI. Something rather bizarre there, and I think it'll hit problems in drm_edid at [1] as we end up with a connector having no color_formats defined. I was discussing this with Maxime only last week, but in relation to VGA monitors connected through HDMI to VGA adapters without rewriting the EDID. If you have an issue between 5.16 and 5.17, then I'd guess at [2] and your monitor not asserting hotplug correctly. The raw hotplug status is reported in /sys/kernel/debug/dri/N/hdmi0_regs (N will be either 0 or 1 depending on the probe order of the vc4 and v3d drivers). Grep for HDMI_HOTPLUG. Incorrect hotplug behaviour causes grief when combined with HDMI2.0 and scrambling. If you don' t know the other end has been disconnected, then you
Re: [PATCH] drivers/gpu: fix typo in comment
Applied. Thanks!
On Sun, Mar 5, 2023 at 5:22 PM Husain Alshehhi wrote:
>
> Replace "isntance" with "instance".
>
> Signed-off-by: Husain Alshehhi
> ---
> .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h| 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
> b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
> index 007d6bdc3e39..734b34902fa7 100644
> --- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
> +++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
> @@ -1971,7 +1971,7 @@ struct dmub_cmd_psr_copy_settings_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2029,7 +2029,7 @@ struct dmub_cmd_psr_set_level_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2056,7 +2056,7 @@ struct dmub_rb_cmd_psr_enable_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2100,7 +2100,7 @@ struct dmub_cmd_psr_set_version_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2131,7 +2131,7 @@ struct dmub_cmd_psr_force_static_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2206,7 +2206,7 @@ struct dmub_cmd_update_dirty_rect_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2344,7 +2344,7 @@ struct dmub_cmd_update_cursor_payload0 {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2391,7 +2391,7 @@ struct dmub_cmd_psr_set_vtotal_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> @@ -2429,7 +2429,7 @@ struct dmub_cmd_psr_set_power_opt_data {
> uint8_t cmd_version;
> /**
> * Panel Instance.
> -* Panel isntance to identify which psr_state to use
> +* Panel instance to identify which psr_state to use
> * Currently the support is only for 0 or 1
> */
> uint8_t panel_inst;
> --
> 2.39.2
>
>
Re: [PATCH] drm/amd/display: add prefix to amdgpu_dm_plane.h functions
On Mon, Mar 6, 2023 at 3:23 AM David Tadokoro wrote:
>
> From: David Tadokoro
>
> The amdgpu_dm_plane.h functions didn't have names that indicated where
> they were declared.
>
> To better filter results in debug tools like ftrace, prefix these
> functions with 'amdgpu_dm_plane_'.
>
> Note that we may want to make this same change in other files like
> amdgpu_dm_crtc.h.
>
> Signed-off-by: David Tadokoro
Applied. Thanks!
Alex
> ---
> .../gpu/amdgpu/display/display-manager.rst| 2 +-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++---
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 20 +--
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.h | 12 +--
> 4 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst
> b/Documentation/gpu/amdgpu/display/display-manager.rst
> index b7abb18cfc82..be2651ecdd7f 100644
> --- a/Documentation/gpu/amdgpu/display/display-manager.rst
> +++ b/Documentation/gpu/amdgpu/display/display-manager.rst
> @@ -173,7 +173,7 @@ The alpha blending equation is configured from DRM to DC
> interface by the
> following path:
>
> 1. When updating a :c:type:`drm_plane_state `, DM calls
> - :c:type:`fill_blending_from_plane_state()` that maps
> + :c:type:`amdgpu_dm_plane_fill_blending_from_plane_state()` that maps
> :c:type:`drm_plane_state ` attributes to
> :c:type:`dc_plane_info ` struct to be handled in the
> OS-agnostic component (DC).
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 4217ebe6391b..f7111acd45cc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2923,7 +2923,7 @@ const struct amdgpu_ip_block_version dm_ip_block =
>
> static const struct drm_mode_config_funcs amdgpu_dm_mode_funcs = {
> .fb_create = amdgpu_display_user_framebuffer_create,
> - .get_format_info = amd_get_format_info,
> + .get_format_info = amdgpu_dm_plane_get_format_info,
> .output_poll_changed = drm_fb_helper_output_poll_changed,
> .atomic_check = amdgpu_dm_atomic_check,
> .atomic_commit = drm_atomic_helper_commit,
> @@ -4948,7 +4948,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
> if (ret)
> return ret;
>
> - ret = fill_plane_buffer_attributes(adev, afb, plane_info->format,
> + ret = amdgpu_dm_plane_fill_plane_buffer_attributes(adev, afb,
> plane_info->format,
>plane_info->rotation, tiling_flags,
>_info->tiling_info,
>_info->plane_size,
> @@ -4957,7 +4957,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
> if (ret)
> return ret;
>
> - fill_blending_from_plane_state(
> + amdgpu_dm_plane_fill_blending_from_plane_state(
> plane_state, _info->per_pixel_alpha,
> _info->pre_multiplied_alpha,
> _info->global_alpha, _info->global_alpha_value);
>
> @@ -4976,7 +4976,7 @@ static int fill_dc_plane_attributes(struct
> amdgpu_device *adev,
> int ret;
> bool force_disable_dcc = false;
>
> - ret = fill_dc_scaling_info(adev, plane_state, _info);
> + ret = amdgpu_dm_plane_fill_dc_scaling_info(adev, plane_state,
> _info);
> if (ret)
> return ret;
>
> @@ -7882,7 +7882,7 @@ static void amdgpu_dm_commit_cursors(struct
> drm_atomic_state *state)
> */
> for_each_old_plane_in_state(state, plane, old_plane_state, i)
> if (plane->type == DRM_PLANE_TYPE_CURSOR)
> - handle_cursor_update(plane, old_plane_state);
> + amdgpu_dm_plane_handle_cursor_update(plane,
> old_plane_state);
> }
>
> static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> @@ -7967,7 +7967,7 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
>
> bundle->surface_updates[planes_count].gamut_remap_matrix =
> _plane->gamut_remap_matrix;
> }
>
> - fill_dc_scaling_info(dm->adev, new_plane_state,
> + amdgpu_dm_plane_fill_dc_scaling_info(dm->adev,
> new_plane_state,
> >scaling_infos[planes_count]);
>
> bundle->surface_updates[planes_count].scaling_info =
> @@ -9634,7 +9634,7 @@ static int dm_update_plane_state(struct dc *dc,
> if (!needs_reset)
> return 0;
>
> - ret = dm_plane_helper_check_state(new_plane_state,
> new_crtc_state);
> + ret = amdgpu_dm_plane_helper_check_state(new_plane_state,
> new_crtc_state);
> if (ret)
> return ret;
>
> diff --git
Re: [PATCH v2 1/7] drm/ttm: Fix a NULL pointer dereference
Am 07.03.23 um 15:46 schrieb Thomas Hellström:
The LRU mechanism may look up a resource in the process of being removed
from an object. The locking rules here are a bit unclear but it looks
currently like res->bo assignment is protected by the LRU lock, whereas
bo->resource is protected by the object lock, while *clearing* of
bo->resource is also protected by the LRU lock. This means that if
we check that bo->resource points to the LRU resource under the LRU
lock we should be safe.
So perform that check before deciding to swap out a bo. That avoids
dereferencing a NULL bo->resource in ttm_bo_swapout().
Please make sure that this is pushed to drm-misc-fixes ASAP.
I've getting complains for this from different sides.
Thanks,
Christian.
Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4")
Cc: Christian König
Cc: Daniel Vetter
Cc: Christian Koenig
Cc: Huang Rui
Cc: Alex Deucher
Cc: Felix Kuehling
Cc: Philip Yang
Cc: Qiang Yu
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Tvrtko Ursulin
Cc: "Thomas Hellström"
Cc: Anshuman Gupta
Cc: Arunpravin Paneer Selvam
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Thomas Hellström
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index c7a1862f322a..ae2f19dc9f81 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct
ttm_operation_ctx *ctx,
struct ttm_buffer_object *bo = res->bo;
uint32_t num_pages;
- if (!bo)
+ if (!bo || bo->resource != res)
continue;
num_pages = PFN_UP(bo->base.size);
[PATCH] fbdev: tgafb: Fix potential divide by zero
fb_set_var would by called when user invokes ioctl with cmd
FBIOPUT_VSCREENINFO. User-provided data would finally reach
tgafb_check_var. In case var->pixclock is assigned to zero,
divide by zero would occur when checking whether reciprocal
of var->pixclock is too high.
Similar crashes have happened in other fbdev drivers. There
is no check and modification on var->pixclock along the call
chain to tgafb_check_var. We believe it could also be triggered
in driver tgafb from user site.
Signed-off-by: harperchen
---
drivers/video/fbdev/tgafb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c
index 14d37c49633c..b44004880f0d 100644
--- a/drivers/video/fbdev/tgafb.c
+++ b/drivers/video/fbdev/tgafb.c
@@ -173,6 +173,9 @@ tgafb_check_var(struct fb_var_screeninfo *var, struct
fb_info *info)
{
struct tga_par *par = (struct tga_par *)info->par;
+ if (!var->pixclock)
+ return -EINVAL;
+
if (par->tga_type == TGA_TYPE_8PLANE) {
if (var->bits_per_pixel != 8)
return -EINVAL;
--
2.25.1
drivers/gpu/drm/bridge/fsl-ldb.c:101: possible loss of information.
Hello there,
I just ran the static analyser "cppcheck" over the source code of
linux-6.2-rc1. It said:
linux-6.3-rc1/drivers/gpu/drm/bridge/fsl-ldb.c:101:3: style: int result is
returned as long value. If the return value is long to avoid loss of
information, then you have loss of information. [truncLongCastReturn]
Source code is
static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock)
{
if (fsl_ldb->lvds_dual_link)
return clock * 3500;
else
return clock * 7000;
}
Depending on the range of the value of clock, maybe unsigned long literals,
like 3500UL, should
have been used ?
Regards
David Binderman
[PATCH 00/11] tree-wide: remove support for Renesas R-Car H3 ES1
Because H3 ES1 becomes an increasing maintenance burden and was only available to a development group, we decided to remove upstream support for it. Here are the patches to remove driver changes. Review tags have been gathered before during an internal discussion. Only change since the internal version is a plain rebase to v6.3-rc1. A branch with all removals is here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/h3es1-removal Please apply individually per subsystem. There are no dependencies and the SoC doesn't boot anymore since v6.3-rc1. Thanks and happy hacking! Wolfram Sang (11): iommu/ipmmu-vmsa: remove R-Car H3 ES1.* handling drm: rcar-du: remove R-Car H3 ES1.* workarounds media: rcar-vin: remove R-Car H3 ES1.* handling media: rcar-vin: csi2: remove R-Car H3 ES1.* handling media: renesas: fdp1: remove R-Car H3 ES1.* handling thermal/drivers/rcar_gen3_thermal: remove R-Car H3 ES1.* handling ravb: remove R-Car H3 ES1.* handling mmc: renesas_sdhi: remove R-Car H3 ES1.* handling usb: host: xhci-rcar: remove leftover quirk handling usb: host: xhci-rcar: remove R-Car H3 ES1.* handling usb: gadget: udc: renesas_usb3: remove R-Car H3 ES1.* handling drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 37 ++--- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 48 - drivers/gpu/drm/rcar-du/rcar_du_drv.h | 2 - drivers/gpu/drm/rcar-du/rcar_du_regs.h| 3 +- drivers/iommu/ipmmu-vmsa.c| 1 - .../platform/renesas/rcar-vin/rcar-core.c | 36 - .../platform/renesas/rcar-vin/rcar-csi2.c | 15 ++ drivers/media/platform/renesas/rcar_fdp1.c| 4 -- drivers/mmc/host/renesas_sdhi_internal_dmac.c | 10 ++-- drivers/net/ethernet/renesas/ravb_main.c | 15 -- drivers/thermal/rcar_gen3_thermal.c | 52 +-- drivers/usb/gadget/udc/renesas_usb3.c | 23 +--- drivers/usb/host/xhci-rcar.c | 34 +--- 13 files changed, 16 insertions(+), 264 deletions(-) -- 2.35.1
[PATCH 02/11] drm: rcar-du: remove R-Car H3 ES1.* workarounds
R-Car H3 ES1.* was only available to an internal development group and
needed a lot of quirks and workarounds. These become a maintenance
burden now, so our development group decided to remove upstream support
and disable booting for this SoC. Public users only have ES2 onwards.
Signed-off-by: Wolfram Sang
---
Please apply individually per subsystem. There are no dependencies and the SoC
doesn't boot anymore since v6.3-rc1.
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 37 ++--
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 48 --
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 2 --
drivers/gpu/drm/rcar-du/rcar_du_regs.h | 3 +-
4 files changed, 4 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 008e172ed43b..84411c452e30 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -223,20 +223,6 @@ static void rcar_du_crtc_set_display_timing(struct
rcar_du_crtc *rcrtc)
* DU channels that have a display PLL can't use the internal
* system clock, and have no internal clock divider.
*/
-
- /*
-* The H3 ES1.x exhibits dot clock duty cycle stability issues.
-* We can work around them by configuring the DPLL to twice the
-* desired frequency, coupled with a /2 post-divider. Restrict
-* the workaround to H3 ES1.x as ES2.0 and all other SoCs have
-* no post-divider when a display PLL is present (as shown by
-* the workaround breaking HDMI output on M3-W during testing).
-*/
- if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY) {
- target *= 2;
- div = 1;
- }
-
extclk = clk_get_rate(rcrtc->extclock);
rcar_du_dpll_divider(rcrtc, , extclk, target);
@@ -245,30 +231,13 @@ static void rcar_du_crtc_set_display_timing(struct
rcar_du_crtc *rcrtc)
| DPLLCR_N(dpll.n) | DPLLCR_M(dpll.m)
| DPLLCR_STBY;
- if (rcrtc->index == 1) {
+ if (rcrtc->index == 1)
dpllcr |= DPLLCR_PLCS1
| DPLLCR_INCS_DOTCLKIN1;
- } else {
- dpllcr |= DPLLCR_PLCS0_PLL
+ else
+ dpllcr |= DPLLCR_PLCS0
| DPLLCR_INCS_DOTCLKIN0;
- /*
-* On ES2.x we have a single mux controlled via bit 21,
-* which selects between DCLKIN source (bit 21 = 0) and
-* a PLL source (bit 21 = 1), where the PLL is always
-* PLL1.
-*
-* On ES1.x we have an additional mux, controlled
-* via bit 20, for choosing between PLL0 (bit 20 = 0)
-* and PLL1 (bit 20 = 1). We always want to use PLL1,
-* so on ES1.x, in addition to setting bit 21, we need
-* to set the bit 20.
-*/
-
- if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
- dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
- }
-
rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
escr = ESCR_DCLKSEL_DCLKIN | div;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index b9a94c5260e9..1ffde19cb87f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -16,7 +16,6 @@
#include
#include
#include
-#include
#include
#include
@@ -387,43 +386,6 @@ static const struct rcar_du_device_info
rcar_du_r8a7795_info = {
.dpll_mask = BIT(2) | BIT(1),
};
-static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = {
- .gen = 3,
- .features = RCAR_DU_FEATURE_CRTC_IRQ
- | RCAR_DU_FEATURE_CRTC_CLOCK
- | RCAR_DU_FEATURE_VSP1_SOURCE
- | RCAR_DU_FEATURE_INTERLACED
- | RCAR_DU_FEATURE_TVM_SYNC,
- .quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY
- | RCAR_DU_QUIRK_H3_ES1_PLL,
- .channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
- .routes = {
- /*
-* R8A7795 has one RGB output, two HDMI outputs and one
-* LVDS output.
-*/
- [RCAR_DU_OUTPUT_DPAD0] = {
- .possible_crtcs = BIT(3),
- .port = 0,
- },
- [RCAR_DU_OUTPUT_HDMI0] = {
- .possible_crtcs = BIT(1),
- .port = 1,
- },
- [RCAR_DU_OUTPUT_HDMI1]
Re: [regression] RPI4B drm vc4: no crtc or sizes since 5.17 (works in 5.16; and still broken in at least 6.1)
AL13N schreef op 2023-03-06 17:34: Hi, I have a RPI4B connected on 2nd HDMI port (furthest away from power) to a 4K TV, which works until 5.16, from 5.17 there is no X (or plymouth), the cause of no X is that EDID gives nothing, and in the journal; there is "Cannot find any crct or sizes". Only the kernel is changed for this. In 5.16 instead of this message there is a bunch of hex lines prefixed with BAD. It is still broken in 6.1 at the very least. I donno if this is related to this part, but I wanted to try a newer kernel, because the RPI4 seems to do all the video decoding in software and cannot seem to handle it. logs: vc4-drm gpu: bound fef05700.hdmi (ops vc4_hdmi_ops [vc4]) vc4-drm gpu: bound fe004000.txp (ops vc4_txp_ops [vc4]) vc4-drm gpu: bound fe206000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fe207000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fe20a000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fe216000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fec12000.pixelvalve (ops vc4_crtc_ops [vc4]) checking generic (3ea81000 12c000) vs hw (0 ) fb0: switching to vc4 from simple Console: switching to colour dummy device 80x25 [drm] Initialized vc4 0.0.0 20140616 for gpu on minor 0 vc4-drm gpu: [drm] Cannot find any crtc or sizes 5.16 log has: vc4-drm gpu: bound fef05700.hdmi (ops vc4_hdmi_ops [vc4]) vc4-drm gpu: bound fe004000.txp (ops vc4_txp_ops [vc4]) vc4-drm gpu: bound fe206000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fe207000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fe20a000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fe216000.pixelvalve (ops vc4_crtc_ops [vc4]) vc4-drm gpu: bound fec12000.pixelvalve (ops vc4_crtc_ops [vc4]) [drm] Initialized vc4 0.0.0 20140616 for gpu on minor 0 [00] BAD 00 ff ff ff ff ff ff 00 36 74 00 00 00 00 00 00 [00] BAD 0b 1f 01 03 00 23 01 78 0a cf 74 a3 57 4c b0 23 [00] BAD 09 48 4c 00 00 00 01 01 01 ff 01 ff ff 01 01 01 [00] BAD 01 01 01 01 01 20 08 e8 00 30 f2 70 5a 80 b0 58 [00] BAD 8a 00 c4 8e 21 00 00 1e 02 3a 80 18 71 38 2d 40 [00] BAD 58 2c 45 00 c4 8e 21 00 00 1e 00 00 00 fc 00 53 [00] BAD 41 4c 4f 52 41 0a 20 20 20 20 20 20 00 00 00 fd [00] BAD 00 3b 46 1f 8c 3c 00 0a 20 20 20 20 20 20 01 aa Console: switching to colour frame buffer device 240x67 vc4-drm gpu: [drm] fb0: vc4drmfb frame buffer device i donno what this bad is, but it doesn't happen in 5.17... maybe these BAD got filtered out, but they did end up working for me? or something? i donno... I also noticed that earlier in the logs there are more bound lines: (some are double) vc4-drm gpu: bound fe40.hvs (ops vc4_hvs_ops [vc4]) vc4-drm gpu: bound fe40.hvs (ops vc4_hvs_ops [vc4]) and then here for some reason systemd does modprobe@drm.service ? is this just a delayed starting log line, or does it actually try to unload drm and reload? i doubt it? in any case there is more that appears before: vc4-drm gpu: bound fe40.hvs (ops vc4_hvs_ops [vc4]) vc4-drm gpu: bound fe40.hvs (ops vc4_hvs_ops [vc4]) vc4-drm gpu: bound fe40.hvs (ops vc4_hvs_ops [vc4]) vc4-drm gpu: bound fef00700.hdmi (ops vc4_hdmi_ops [vc4]) vc4-drm gpu: bound fe40.hvs (ops vc4_hvs_ops [vc4]) vc4-drm gpu: bound fef00700.hdmi (ops vc4_hdmi_ops [vc4]) so, the error message is weird, as it implies 2 possibilities. however, i think it did find a crtc since all those pixelvalve things use crtc functions? So then why do i have this problem on my RPI4? do most people just use the raspberry pi kernels? I looked at what was changed in crtc between 5.16 and 5.17, but not much: 2022-02-17 drm/vc4: crtc: Fix runtime_pm reference counting Maxime Ripard 1 -3/+5 2022-02-07 drm/vc4: crtc: Fix redundant variable assignment Maxime Ripard 1 -1/+0 2021-11-05 drm/vc4: crtc: Copy assigned channel to the CRTC Maxime Ripard 1 -2/+2 2021-11-05 drm/vc4: Fix non-blocking commit getting stuck forever Maxime Ripard 1 -1/+4 2021-11-05 drm/vc4: crtc: Drop feed_txp from state Maxime Ripard 1 -2/+1 2021-11-04 drm/vc4: Increase the core clock based on HVS load Maxime Ripard 1 -0/+15 2021-11-04 drm/vc4: crtc: Add some logging Maxime Ripard 1 -0/+6 2021-11-04 drm/vc4: crtc: Rework the encoder retrieval code (again) Maxime Ripard 1 -21/+9 2021-11-04 drm/vc4: crtc: Add encoder to vc4_crtc_config_pv prototype Maxime Ripard 1 -4/+3 2021-11-04 drm/vc4: Make vc4_crtc_get_encoder public Maxime Ripard 1 -4/+4 2021-10-25 drm/vc4: crtc: Make sure the HDMI controller is powered when disabling Maxime Ripard 1 -1/+18 I looked at them, but in my layman's knowledge, none of those really seemed to have any impact? So, maybe the problem is someplace else? Can anyone help to find out where this problem might be? even though this is old code, i still have this issue at least on 6.1 ? Regards, Maarten
Re: [PATCH RFC 00/18] Rust DRM subsystem abstractions (& preview AGX driver)
c types, and whether they should be marked as > method receivers (Yuck, internal rustc implementation hacks! But > Arc already does the same thing and it makes usage in > driver-implemented callbacks as `self` possible) are things I'd love > to discuss ^^. > > - Only what I need for my driver is implemented (plus a small amount of > obvious extras where better API completeness makes sense). I think the > general idea with Rust abstractions is that we add things as they > become necessary. > > - The plain GEM vs. GEM-shmem duality ended up with quite a hairy type > hierarchy. I'd love to figure out how to make this simpler... > > - drm::mm ends up requiring a built-in mutex in the abstraction, instead > of delegating that to the user with the usual Rust mutability rules. > This is because nodes can be dropped at any time, and those operations > need to be synchronized. We could try to avoid forbidding those drops > or mark the node type !Send, but that would make it a lot less > ergonomic to use... > > I'm looking for feedback on the abstractions of all kinds, so we can > move towards an upstreamable version. Optimistically, I'd love to get > this upstream for 6.5, and the driver for 6.6. > > Please feel free to ask any questions about the Rust bits, since I know > a lot of this is new to many of the C folks! ## About the drm-asahi driver > This is a fairly complete driver for Apple AGX G13 and G14 series GPUs. > > The driver today supports the Apple M1, M1 Pro, M1 Max, M1 Ultra, and M2 > SoCs, across two firmware revisions each. It has an explicit sync UAPI > heavily inspired by the upcoming Intel Xe UAPI, designed with Vulkan > support in mind. On the Mesa side we currently have a Gallium driver > that is mostly already upstream (missing the UAPI bits mostly) and > passes the dEQP GLES2/EGL tests, with most of GLES3.0 passing in > downstream work-in-progress branches. This is a reverse engineered > community driver (we have no hardware documentation of any kind, other > than some hints from aspects shared with PowerVR). > > While developing the driver, I tried to make use of Rust's safety and > lifetime features to provide not just CPU-side safety, but also > partial firmware-ABI safety. Thanks to this, it has turned out to be > a very stable driver even though GPU firmware crashes are fatal (no > restart capability, need to reboot!) and the FW/driver interface is a > huge mess of unsafe shared memory structures with complex pointer > chains. There are over 70 ABI types and 3000+ lines of firmware ABI type > definitions that vary between firmware builds and GPU cores... > > In a simpler blocking-submission form, it has been shipping in Asahi > Linux edge kernels since December [2], with lots of users and zero (!) > reported oopses (and only a couple reports of GPU firmware crashes, > though that issue should now be fixed). It has survived OOM scenarios > (Rust makes error cleanup easy!), UAPI-level fuzzing, countless broken > Mesa builds, uptimes of 40+ days, and more. > > The explicit sync refactor significantly increases performance (and > potential problems), but this version has survived a lot of torture > with dEQP/piglit tests and some manual corner case testing. > > In other words, Rust works! ^^ > > There are some design notes on the driver and further links at [10]. ## Links > [1] https://github.com/AsahiLinux/linux.git drm-rfc-base-20230307 > [2] https://asahilinux.org/2022/12/gpu-drivers-now-in-asahi-linux/ > [3] > https://lore.kernel.org/rust-for-linux/20230224-rust-error-v1-0-f8f9a9a87...@asahilina.net/T/ > [4] > https://lore.kernel.org/rust-for-linux/20230224-rust-macros-v1-0-b39fae46e...@asahilina.net/T/ > [5] > https://lore.kernel.org/rust-for-linux/20230224-rust-iopt-rtkit-v1-0-49ced3391...@asahilina.net/T/#m515bad2cff7f5a46f55897e6b73c6c2f1fb2c638 > [6] > https://lore.kernel.org/rust-for-linux/20230224-rust-iopt-rtkit-v1-0-49ced3391...@asahilina.net/T/#m4c64e390c43b3ff1b8470fc8b37eaf87f6e12c94 > [7] > https://lore.kernel.org/rust-for-linux/CQV7ZNT6LMXI.1XG4YXSH8I7JK@vincent-arch/T/ > [8] > https://lore.kernel.org/rust-for-linux/61f734d6-1497-755f-3632-3f261b890...@asahilina.net/T/ > [9] > https://lore.kernel.org/rust-for-linux/20230224-rust-arc-v1-0-568eea613...@asahilina.net/T/ > [10] https://github.com/AsahiLinux/docs/wiki/SW:AGX-driver-notes ~~ Lina
Re: [PATCH] accel: Link to compute accelerator subsystem intro
On 3/6/2023 9:35 PM, Bagas Sanjaya wrote:
Commit 2c204f3d53218d ("accel: add dedicated minor for accelerator
devices") adds link to accelerator nodes section of DRM internals doc
(Documentation/gpu/drm-internals.rst), but the target doesn't exist.
Instead, there is only an introduction doc for computer accelerator
subsytem.
Link to that doc until there is documentation of accelerator internals.
Fixes: 2c204f3d53218d ("accel: add dedicated minor for accelerator devices")
Signed-off-by: Bagas Sanjaya
Reviewed-by: Jeffrey Hugo
Re: [PATCH RFC 01/18] rust: drm: ioctl: Add DRM ioctl abstraction
On 3/7/23 11:25, Asahi Lina wrote:
DRM drivers need to be able to declare which driver-specific ioctls they
support. This abstraction adds the required types and a helper macro to
generate the ioctl definition inside the DRM driver.
Note that this macro is not usable until further bits of the
abstraction are in place (but it will not fail to compile on its own, if
not called).
Signed-off-by: Asahi Lina
---
drivers/gpu/drm/Kconfig | 7 ++
rust/bindings/bindings_helper.h | 2 +
rust/kernel/drm/ioctl.rs| 147
rust/kernel/drm/mod.rs | 5 ++
rust/kernel/lib.rs | 2 +
5 files changed, 163 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index dc0f94f02a82..dab8f0f9aa96 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -27,6 +27,13 @@ menuconfig DRM
details. You should also select and configure AGP
(/dev/agpgart) support if it is available for your platform.
[...]
+
+/// Declare the DRM ioctls for a driver.
+///
+/// Each entry in the list should have the form:
+///
+/// `(ioctl_number, argument_type, flags, user_callback),`
+///
+/// `argument_type` is the type name within the `bindings` crate.
+/// `user_callback` should have the following prototype:
+///
+/// ```
+/// fn foo(device: ::drm::device::Device,
+///data: bindings::argument_type,
+///file: ::drm::file::File,
+/// )
+/// ```
+/// where `Self` is the drm::drv::Driver implementation these ioctls are being
declared within.
+///
+/// # Examples
+///
+/// ```
+/// kernel::declare_drm_ioctls! {
+/// (FOO_GET_PARAM, drm_foo_get_param, ioctl::RENDER_ALLOW,
my_get_param_handler),
+/// }
+/// ```
+///
+#[macro_export]
+macro_rules! declare_drm_ioctls {
+( $(($cmd:ident, $struct:ident, $flags:expr, $func:expr)),* $(,)? ) => {
+const IOCTLS: &'static [$crate::drm::ioctl::DrmIoctlDescriptor] = {
+const _:() = {
+let i: u32 = $crate::bindings::DRM_COMMAND_BASE;
+// Assert that all the IOCTLs are in the right order and there
are no gaps,
+// and that the sizeof of the specified type is correct.
I believe that not necessarily the IOCTLs need to be in the right order and
with no gaps. For example, armada_drm.h has a gap in between 0x00 and
0x02 and exynos_drm.h also have gaps. Moreover, some drivers, like vgem and
virtgpu, start their IOCTLs with 0x01.
Best Regards,
- Maíra Canal
+$(
+let cmd: u32 =
$crate::macros::concat_idents!($crate::bindings::DRM_IOCTL_, $cmd);
+::core::assert!(i == $crate::ioctl::_IOC_NR(cmd));
+
::core::assert!(core::mem::size_of::<$crate::bindings::$struct>() ==
$crate::ioctl::_IOC_SIZE(cmd));
+let i: u32 = i + 1;
+)*
+};
+
+let ioctls = &[$(
+$crate::bindings::drm_ioctl_desc {
+cmd:
$crate::macros::concat_idents!($crate::bindings::DRM_IOCTL_, $cmd) as u32,
+func: {
+#[allow(non_snake_case)]
+unsafe extern "C" fn $cmd(
+raw_dev: *mut $crate::bindings::drm_device,
+raw_data: *mut ::core::ffi::c_void,
+raw_file_priv: *mut $crate::bindings::drm_file,
+) -> core::ffi::c_int {
+// SAFETY: We never drop this, and the DRM core
ensures the device lives
+// while callbacks are being called.
+//
+// FIXME: Currently there is nothing enforcing
that the types of the
+// dev/file match the current driver these ioctls
are being declared
+// for, and it's not clear how to enforce this
within the type system.
+let dev = ::core::mem::ManuallyDrop::new(unsafe {
+$crate::drm::device::Device::from_raw(raw_dev)
+});
+// SAFETY: This is just the ioctl argument, which
hopefully has the right type
+// (we've done our best checking the size).
+let data = unsafe { *(raw_data as *mut
$crate::bindings::$struct) };
+// SAFETY: This is just the DRM file structure
+let file = unsafe {
$crate::drm::file::File::from_raw(raw_file_priv) };
+
+match $func(&*dev, data, ) {
+Err(e) => e.to_kernel_errno(),
+Ok(i) =>
i.try_into().unwrap_or(ERANGE.to_kernel_errno()),
+}
+}
+Some($cmd)
+
Re: (subset) [PATCH] drm/sun4i: fix missing component unbind on bind errors
On Mon, 06 Mar 2023 11:32:42 +0100, Johan Hovold wrote: > Make sure to unbind all subcomponents when binding the aggregate device > fails. > > Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
[PATCH v4 01/17] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
This allows us to use strongly typed arguments.
v2:
- Bring NO_DATA back
- Provide explicit enum values
v4: Drop unnecessary '&' from kerneldoc (emersion)
Signed-off-by: Harry Wentland
Reviewed-by: Simon Ser
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Uma Shankar
Cc: Ville Syrjälä
Cc: Joshua Ashton
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
---
include/drm/display/drm_dp.h | 2 +-
include/drm/drm_connector.h | 49 ++--
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index ed10e6b6f99d..dae5e9c201e4 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -1623,7 +1623,7 @@ enum dp_pixelformat {
*
* This enum is used to indicate DP VSC SDP Colorimetry formats.
* It is based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
- * DB18] and a name of enum member follows DRM_MODE_COLORIMETRY definition.
+ * DB18] and a name of enum member follows enum drm_colorimetry definition.
*
* @DP_COLORIMETRY_DEFAULT: sRGB (IEC 61966-2-1) or
* ITU-R BT.601 colorimetry format
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4d830fc55a3d..6d6a53a6b010 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -371,29 +371,30 @@ enum drm_privacy_screen_status {
* a colorspace property which will be created and exposed to
* userspace.
*/
-
-/* For Default case, driver will set the colorspace */
-#define DRM_MODE_COLORIMETRY_DEFAULT 0
-/* CEA 861 Normal Colorimetry options */
-#define DRM_MODE_COLORIMETRY_NO_DATA 0
-#define DRM_MODE_COLORIMETRY_SMPTE_170M_YCC1
-#define DRM_MODE_COLORIMETRY_BT709_YCC 2
-/* CEA 861 Extended Colorimetry Options */
-#define DRM_MODE_COLORIMETRY_XVYCC_601 3
-#define DRM_MODE_COLORIMETRY_XVYCC_709 4
-#define DRM_MODE_COLORIMETRY_SYCC_601 5
-#define DRM_MODE_COLORIMETRY_OPYCC_601 6
-#define DRM_MODE_COLORIMETRY_OPRGB 7
-#define DRM_MODE_COLORIMETRY_BT2020_CYCC 8
-#define DRM_MODE_COLORIMETRY_BT2020_RGB9
-#define DRM_MODE_COLORIMETRY_BT2020_YCC10
-/* Additional Colorimetry extension added as part of CTA 861.G */
-#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D6511
-#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER12
-/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
-#define DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED13
-#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT14
-#define DRM_MODE_COLORIMETRY_BT601_YCC 15
+enum drm_colorspace {
+ /* For Default case, driver will set the colorspace */
+ DRM_MODE_COLORIMETRY_DEFAULT= 0,
+ DRM_MODE_COLORIMETRY_NO_DATA= 0,
+ /* CEA 861 Normal Colorimetry options */
+ DRM_MODE_COLORIMETRY_SMPTE_170M_YCC = 1,
+ DRM_MODE_COLORIMETRY_BT709_YCC = 2,
+ /* CEA 861 Extended Colorimetry Options */
+ DRM_MODE_COLORIMETRY_XVYCC_601 = 3,
+ DRM_MODE_COLORIMETRY_XVYCC_709 = 4,
+ DRM_MODE_COLORIMETRY_SYCC_601 = 5,
+ DRM_MODE_COLORIMETRY_OPYCC_601 = 6,
+ DRM_MODE_COLORIMETRY_OPRGB = 7,
+ DRM_MODE_COLORIMETRY_BT2020_CYCC= 8,
+ DRM_MODE_COLORIMETRY_BT2020_RGB = 9,
+ DRM_MODE_COLORIMETRY_BT2020_YCC = 10,
+ /* Additional Colorimetry extension added as part of CTA 861.G */
+ DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65 = 11,
+ DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER = 12,
+ /* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry
Format */
+ DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED = 13,
+ DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT = 14,
+ DRM_MODE_COLORIMETRY_BT601_YCC = 15,
+};
/**
* enum drm_bus_flags - bus_flags info for _display_info
@@ -826,7 +827,7 @@ struct drm_connector_state {
* colorspace change on Sink. This is most commonly used to switch
* to wider color gamuts like BT2020.
*/
- u32 colorspace;
+ enum drm_colorspace colorspace;
/**
* @writeback_job: Writeback job for writeback connectors
--
2.39.2
Re: [PATCH RFC 15/18] drm/asahi: Add the Asahi driver UAPI [DO NOT MERGE]
On Tue, Mar 7, 2023 at 3:28 PM Asahi Lina wrote:
>
> Adds the Asahi GPU driver UAPI. Note: this API is not yet stable and
> therefore not ready for merging!
>
> Signed-off-by: Asahi Lina
> ---
> include/uapi/drm/asahi_drm.h | 556
> +++
> 1 file changed, 556 insertions(+)
>
> diff --git a/include/uapi/drm/asahi_drm.h b/include/uapi/drm/asahi_drm.h
> new file mode 100644
> index ..7b15b486d03d
> --- /dev/null
> +++ b/include/uapi/drm/asahi_drm.h
> @@ -0,0 +1,556 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (C) The Asahi Linux Contributors
> + *
> + * Heavily inspired by xe_drm.h.
> + */
> +#ifndef _ASAHI_DRM_H_
> +#define _ASAHI_DRM_H_
> +
> +#include "drm.h"
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif
> +
> +#define DRM_ASAHI_UNSTABLE_UABI_VERSION10006
> +
> +#define DRM_ASAHI_GET_PARAMS 0x00
> +#define DRM_ASAHI_VM_CREATE0x01
> +#define DRM_ASAHI_VM_DESTROY 0x02
> +#define DRM_ASAHI_GEM_CREATE 0x03
> +#define DRM_ASAHI_GEM_MMAP_OFFSET 0x04
> +#define DRM_ASAHI_GEM_BIND 0x05
> +#define DRM_ASAHI_QUEUE_CREATE 0x06
> +#define DRM_ASAHI_QUEUE_DESTROY0x07
> +#define DRM_ASAHI_SUBMIT 0x08
> +#define DRM_ASAHI_GET_TIME 0x09
> +
> +#define DRM_ASAHI_MAX_CLUSTERS 32
> +
> +struct drm_asahi_params_global {
> + __u32 unstable_uabi_version;
> + __u32 pad0;
> +
> + __u64 feat_compat;
> + __u64 feat_incompat;
> +
> + __u32 gpu_generation;
> + __u32 gpu_variant;
> + __u32 gpu_revision;
> + __u32 chip_id;
> +
> + __u32 num_dies;
> + __u32 num_clusters_total;
> + __u32 num_cores_per_cluster;
> + __u32 num_frags_per_cluster;
> + __u32 num_gps_per_cluster;
> + __u32 num_cores_total_active;
> + __u64 core_masks[DRM_ASAHI_MAX_CLUSTERS];
> +
> + __u32 vm_page_size;
> + __u32 pad1;
> + __u64 vm_user_start;
> + __u64 vm_user_end;
> + __u64 vm_shader_start;
> + __u64 vm_shader_end;
> +
> + __u32 max_syncs_per_submission;
> + __u32 max_commands_per_submission;
> + __u32 max_commands_in_flight;
> + __u32 max_attachments;
> +
> + __u32 timer_frequency_hz;
> + __u32 min_frequency_khz;
> + __u32 max_frequency_khz;
> + __u32 max_power_mw;
> +
> + __u32 result_render_size;
> + __u32 result_compute_size;
> +};
> +
> +/*
> +enum drm_asahi_feat_compat {
> +};
> +*/
> +
> +enum drm_asahi_feat_incompat {
> + DRM_ASAHI_FEAT_MANDATORY_ZS_COMPRESSION = (1UL) << 0,
> +};
> +
> +struct drm_asahi_get_params {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + /** @param: Parameter group to fetch (MBZ) */
> + __u32 param_group;
> +
> + /** @pad: MBZ */
> + __u32 pad;
> +
> + /** @value: User pointer to write parameter struct */
> + __u64 pointer;
> +
> + /** @value: Size of user buffer, max size supported on return */
> + __u64 size;
> +};
> +
> +struct drm_asahi_vm_create {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + /** @value: Returned VM ID */
> + __u32 vm_id;
> +
> + /** @pad: MBZ */
> + __u32 pad;
> +};
> +
> +struct drm_asahi_vm_destroy {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + /** @value: VM ID to be destroyed */
> + __u32 vm_id;
> +
> + /** @pad: MBZ */
> + __u32 pad;
> +};
> +
> +#define ASAHI_GEM_WRITEBACK(1L << 0)
> +#define ASAHI_GEM_VM_PRIVATE (1L << 1)
> +
> +struct drm_asahi_gem_create {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + /** @size: Size of the BO */
> + __u64 size;
> +
> + /** @flags: BO creation flags */
> + __u32 flags;
> +
> + /** @handle: VM ID to assign to the BO, if ASAHI_GEM_VM_PRIVATE is
> set. */
> + __u32 vm_id;
> +
> + /** @handle: Returned GEM handle for the BO */
> + __u32 handle;
> +};
> +
> +struct drm_asahi_gem_mmap_offset {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + /** @handle: Handle for the object being mapped. */
> + __u32 handle;
> +
> + /** @flags: Must be zero */
> + __u32 flags;
> +
> + /** @offset: The fake offset to use for subsequent mmap call */
> + __u64 offset;
> +};
> +
> +enum drm_asahi_bind_op {
> + ASAHI_BIND_OP_BIND = 0,
> + ASAHI_BIND_OP_UNBIND = 1,
> + ASAHI_BIND_OP_UNBIND_ALL = 2,
> +};
> +
> +#define ASAHI_BIND_READ(1L << 0)
> +#define ASAHI_BIND_WRITE
Re: [PATCH v3 01/17] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
On Tuesday, March 7th, 2023 at 16:10, Harry Wentland wrote: > * This enum is used to indicate DP VSC SDP Colorimetry formats. > * It is based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through > - * DB18] and a name of enum member follows DRM_MODE_COLORIMETRY definition. > + * DB18] and a name of enum member follows drm_colorimetry definition. Nit: the "&" is not necessary here, "enum drm_colorimetry" alone should linkify properly.
[PATCH v3 17/17] drm/amd/display: Refactor avi_info_frame colorimetry determination
From: Joshua Ashton
Replace the messy two if-else chains here that were
on the same value with a switch on the enum.
Signed-off-by: Joshua Ashton
Signed-off-by: Harry Wentland
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Reviewed-by: Harry Wentland
---
.../gpu/drm/amd/display/dc/core/dc_resource.c | 28 +++
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index d9f2ef242b0f..34a7fb225629 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -3010,23 +3010,29 @@ static void set_avi_info_frame(
hdmi_info.bits.S0_S1 = scan_type;
/* C0, C1 : Colorimetry */
- if (color_space == COLOR_SPACE_YCBCR709 ||
- color_space == COLOR_SPACE_YCBCR709_LIMITED)
+ switch (color_space) {
+ case COLOR_SPACE_YCBCR709:
+ case COLOR_SPACE_YCBCR709_LIMITED:
hdmi_info.bits.C0_C1 = COLORIMETRY_ITU709;
- else if (color_space == COLOR_SPACE_YCBCR601 ||
- color_space == COLOR_SPACE_YCBCR601_LIMITED)
+ break;
+ case COLOR_SPACE_YCBCR601:
+ case COLOR_SPACE_YCBCR601_LIMITED:
hdmi_info.bits.C0_C1 = COLORIMETRY_ITU601;
- else {
- hdmi_info.bits.C0_C1 = COLORIMETRY_NO_DATA;
- }
- if (color_space == COLOR_SPACE_2020_RGB_FULLRANGE ||
- color_space == COLOR_SPACE_2020_RGB_LIMITEDRANGE ||
- color_space == COLOR_SPACE_2020_YCBCR) {
+ break;
+ case COLOR_SPACE_2020_RGB_FULLRANGE:
+ case COLOR_SPACE_2020_RGB_LIMITEDRANGE:
+ case COLOR_SPACE_2020_YCBCR:
hdmi_info.bits.EC0_EC2 = COLORIMETRYEX_BT2020RGBYCBCR;
hdmi_info.bits.C0_C1 = COLORIMETRY_EXTENDED;
- } else if (color_space == COLOR_SPACE_ADOBERGB) {
+ break;
+ case COLOR_SPACE_ADOBERGB:
hdmi_info.bits.EC0_EC2 = COLORIMETRYEX_ADOBERGB;
hdmi_info.bits.C0_C1 = COLORIMETRY_EXTENDED;
+ break;
+ case COLOR_SPACE_SRGB:
+ default:
+ hdmi_info.bits.C0_C1 = COLORIMETRY_NO_DATA;
+ break;
}
if (pixel_encoding && color_space == COLOR_SPACE_2020_YCBCR &&
--
2.39.2
[PATCH v3 14/17] drm/amd/display: Add debugfs for testing output colorspace
In order to IGT test colorspace we'll want to print
the currently enabled colorspace on a stream. We add
a new debugfs to do so, using the same scheme as
current bpc reporting.
This might also come in handy when debugging display
issues.
Signed-off-by: Harry Wentland
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Reviewed-By: Joshua Ashton
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 57 +++
1 file changed, 57 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 4a5dae578d97..f0022c16b708 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -906,6 +906,61 @@ static int amdgpu_current_bpc_show(struct seq_file *m,
void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_bpc);
+/*
+ * Returns the current bpc for the crtc.
+ * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_colorspace
+ */
+static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state = NULL;
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->output_color_space) {
+ case COLOR_SPACE_SRGB:
+ seq_printf(m, "RGB");
+ break;
+ case COLOR_SPACE_YCBCR601:
+ case COLOR_SPACE_YCBCR601_LIMITED:
+ seq_printf(m, "BT601_YCC");
+ break;
+ case COLOR_SPACE_YCBCR709:
+ case COLOR_SPACE_YCBCR709_LIMITED:
+ seq_printf(m, "BT709_YCC");
+ break;
+ case COLOR_SPACE_ADOBERGB:
+ seq_printf(m, "opRGB");
+ break;
+ case COLOR_SPACE_2020_RGB_FULLRANGE:
+ seq_printf(m, "BT2020_RGB");
+ break;
+ case COLOR_SPACE_2020_YCBCR:
+ seq_printf(m, "BT2020_YCC");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+
+
/*
* Example usage:
* Disable dsc passthrough, i.e.,: have dsc decoding at converver, not
external RX
@@ -3235,6 +3290,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
#endif
debugfs_create_file("amdgpu_current_bpc", 0644, crtc->debugfs_entry,
crtc, _current_bpc_fops);
+ debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
+ crtc, _current_colorspace_fops);
}
/*
--
2.39.2
[PATCH v3 13/17] drm/amd/display: Add support for explicit BT601_YCC
We use this by default but if userspace passes this explicitly we should respect it. Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Joshua Ashton Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-By: Joshua Ashton --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 580d076b7749..7f77e226f1eb 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -5330,6 +5330,12 @@ get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing, color_space = COLOR_SPACE_YCBCR601; } break; + case DRM_MODE_COLORIMETRY_BT601_YCC: + if (dc_crtc_timing->flags.Y_ONLY) + color_space = COLOR_SPACE_YCBCR601_LIMITED; + else + color_space = COLOR_SPACE_YCBCR601; + break; case DRM_MODE_COLORIMETRY_BT709_YCC: if (dc_crtc_timing->flags.Y_ONLY) color_space = COLOR_SPACE_YCBCR709_LIMITED; -- 2.39.2
[PATCH v3 16/17] drm/amd/display: Fallback to 2020_YCBCR if the pixel encoding is not RGB
From: Joshua Ashton Userspace might not aware whether we're sending RGB or YCbCr data to the display. If COLOR_SPACE_2020_RGB_FULLRANGE is requested but the output encoding is YCbCr we should send COLOR_SPACE_2020_YCBCR. Signed-off-by: Joshua Ashton Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Joshua Ashton Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-by: Harry Wentland --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index a15b26962496..d5e1f3423cce 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -5324,10 +5324,11 @@ get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing, color_space = COLOR_SPACE_ADOBERGB; break; case DRM_MODE_COLORIMETRY_BT2020: - color_space = COLOR_SPACE_2020_RGB_FULLRANGE; - break; case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED: - color_space = COLOR_SPACE_2020_YCBCR; + if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) + color_space = COLOR_SPACE_2020_RGB_FULLRANGE; + else + color_space = COLOR_SPACE_2020_YCBCR; break; case DRM_MODE_COLORIMETRY_DEFAULT: // ITU601 default: -- 2.39.2
[PATCH v3 15/17] drm/amd/display: Add default case for output_color_space switch
Signed-off-by: Harry Wentland
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Reviewed-By: Joshua Ashton
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 43 ++-
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 7f77e226f1eb..a15b26962496 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5308,7 +5308,29 @@ get_output_color_space(const struct dc_crtc_timing
*dc_crtc_timing,
enum dc_color_space color_space = COLOR_SPACE_SRGB;
switch (connector_state->colorspace) {
+ case DRM_MODE_COLORIMETRY_BT601_YCC:
+ if (dc_crtc_timing->flags.Y_ONLY)
+ color_space = COLOR_SPACE_YCBCR601_LIMITED;
+ else
+ color_space = COLOR_SPACE_YCBCR601;
+ break;
+ case DRM_MODE_COLORIMETRY_BT709_YCC:
+ if (dc_crtc_timing->flags.Y_ONLY)
+ color_space = COLOR_SPACE_YCBCR709_LIMITED;
+ else
+ color_space = COLOR_SPACE_YCBCR709;
+ break;
+ case DRM_MODE_COLORIMETRY_OPRGB:
+ color_space = COLOR_SPACE_ADOBERGB;
+ break;
+ case DRM_MODE_COLORIMETRY_BT2020:
+ color_space = COLOR_SPACE_2020_RGB_FULLRANGE;
+ break;
+ case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
+ color_space = COLOR_SPACE_2020_YCBCR;
+ break;
case DRM_MODE_COLORIMETRY_DEFAULT: // ITU601
+ default:
if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) {
color_space = COLOR_SPACE_SRGB;
/*
@@ -5330,27 +5352,6 @@ get_output_color_space(const struct dc_crtc_timing
*dc_crtc_timing,
color_space = COLOR_SPACE_YCBCR601;
}
break;
- case DRM_MODE_COLORIMETRY_BT601_YCC:
- if (dc_crtc_timing->flags.Y_ONLY)
- color_space = COLOR_SPACE_YCBCR601_LIMITED;
- else
- color_space = COLOR_SPACE_YCBCR601;
- break;
- case DRM_MODE_COLORIMETRY_BT709_YCC:
- if (dc_crtc_timing->flags.Y_ONLY)
- color_space = COLOR_SPACE_YCBCR709_LIMITED;
- else
- color_space = COLOR_SPACE_YCBCR709;
- break;
- case DRM_MODE_COLORIMETRY_OPRGB:
- color_space = COLOR_SPACE_ADOBERGB;
- break;
- case DRM_MODE_COLORIMETRY_BT2020:
- color_space = COLOR_SPACE_2020_RGB_FULLRANGE;
- break;
- case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
- color_space = COLOR_SPACE_2020_YCBCR;
- break;
}
return color_space;
--
2.39.2
[PATCH v3 09/17] drm/amd/display: Register Colorspace property for DP and HDMI
We want compositors to be able to set the output
colorspace on DP and HDMI outputs, based on the
caps reported from the receiver via EDID.
Signed-off-by: Harry Wentland
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Reviewed-By: Joshua Ashton
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f91b2ea13d96..2d883c6dae90 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7184,6 +7184,12 @@ static int amdgpu_dm_connector_get_modes(struct
drm_connector *connector)
return amdgpu_dm_connector->num_modes;
}
+static const u32 supported_colorspaces =
+ BIT(DRM_MODE_COLORIMETRY_BT709_YCC) |
+ BIT(DRM_MODE_COLORIMETRY_OPRGB) |
+ BIT(DRM_MODE_COLORIMETRY_BT2020) |
+ BIT(DRM_MODE_COLORIMETRY_BT2020_DEPRECATED);
+
void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
struct amdgpu_dm_connector *aconnector,
int connector_type,
@@ -7264,6 +7270,15 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
adev->mode_info.abm_level_property, 0);
}
+ if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
+ if
(!drm_mode_create_hdmi_colorspace_property(>base,
supported_colorspaces))
+
drm_connector_attach_colorspace_property(>base);
+ } else if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector_type == DRM_MODE_CONNECTOR_eDP) {
+ if (!drm_mode_create_dp_colorspace_property(>base,
supported_colorspaces))
+
drm_connector_attach_colorspace_property(>base);
+ }
+
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
--
2.39.2
[PATCH v3 08/17] drm/amd/display: Always pass connector_state to stream validation
We need the connector_state for colorspace and scaling information
and can get it from connector->state.
Signed-off-by: Harry Wentland
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Reviewed-By: Joshua Ashton
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4217ebe6391b..f91b2ea13d96 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5916,15 +5916,14 @@ create_stream_for_sink(struct amdgpu_dm_connector
*aconnector,
{
struct drm_display_mode *preferred_mode = NULL;
struct drm_connector *drm_connector;
- const struct drm_connector_state *con_state =
- dm_state ? _state->base : NULL;
+ const struct drm_connector_state *con_state = _state->base;
struct dc_stream_state *stream = NULL;
struct drm_display_mode mode;
struct drm_display_mode saved_mode;
struct drm_display_mode *freesync_mode = NULL;
bool native_mode_found = false;
bool recalculate_timing = false;
- bool scale = dm_state ? (dm_state->scaling != RMX_OFF) : false;
+ bool scale = dm_state->scaling != RMX_OFF;
int mode_refresh;
int preferred_refresh = 0;
enum color_transfer_func tf = TRANSFER_FUNC_UNKNOWN;
@@ -6541,7 +6540,9 @@ enum drm_mode_status
amdgpu_dm_connector_mode_valid(struct drm_connector *connec
goto fail;
}
- stream = create_validate_stream_for_sink(aconnector, mode, NULL, NULL);
+ stream = create_validate_stream_for_sink(aconnector, mode,
+
to_dm_connector_state(connector->state),
+NULL);
if (stream) {
dc_stream_release(stream);
result = MODE_OK;
--
2.39.2
[PATCH v3 05/17] drm/connector: Use common colorspace_names array
We an use bitfields to track the support ones for HDMI
and DP. This allows us to print colorspaces in a consistent
manner without needing to know whether we're dealing with
DP or HDMI.
Signed-off-by: Harry Wentland
Cc: Pekka Paalanen
Cc: Sebastian Wick
Cc: vitaly.pros...@amd.com
Cc: Uma Shankar
Cc: Ville Syrjälä
Cc: Joshua Ashton
Cc: Jani Nikula
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
---
drivers/gpu/drm/drm_connector.c | 131 +++-
include/drm/drm_connector.h | 1 +
2 files changed, 78 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ff4af48c029a..7649f0ac454f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1012,64 +1012,70 @@ static const struct drm_prop_enum_list
drm_dp_subconnector_enum_list[] = {
DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
drm_dp_subconnector_enum_list)
-static const struct drm_prop_enum_list hdmi_colorspaces[] = {
+
+static const char * const colorspace_names[] = {
/* For Default case, driver will set the colorspace */
- { DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
+ [DRM_MODE_COLORIMETRY_DEFAULT] = "Default",
/* Standard Definition Colorimetry based on CEA 861 */
- { DRM_MODE_COLORIMETRY_SMPTE_170M_YCC, "SMPTE_170M_YCC" },
- { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" },
+ [DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] = "SMPTE_170M_YCC",
+ [DRM_MODE_COLORIMETRY_BT709_YCC] = "BT709_YCC",
/* Standard Definition Colorimetry based on IEC 61966-2-4 */
- { DRM_MODE_COLORIMETRY_XVYCC_601, "XVYCC_601" },
+ [DRM_MODE_COLORIMETRY_XVYCC_601] = "XVYCC_601",
/* High Definition Colorimetry based on IEC 61966-2-4 */
- { DRM_MODE_COLORIMETRY_XVYCC_709, "XVYCC_709" },
+ [DRM_MODE_COLORIMETRY_XVYCC_709] = "XVYCC_709",
/* Colorimetry based on IEC 61966-2-1/Amendment 1 */
- { DRM_MODE_COLORIMETRY_SYCC_601, "SYCC_601" },
+ [DRM_MODE_COLORIMETRY_SYCC_601] = "SYCC_601",
/* Colorimetry based on IEC 61966-2-5 [33] */
- { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
+ [DRM_MODE_COLORIMETRY_OPYCC_601] = "opYCC_601",
/* Colorimetry based on IEC 61966-2-5 */
- { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
+ [DRM_MODE_COLORIMETRY_OPRGB] = "opRGB",
/* Colorimetry based on ITU-R BT.2020 */
- { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
+ [DRM_MODE_COLORIMETRY_BT2020_CYCC] = "BT2020_CYCC",
/* Colorimetry based on ITU-R BT.2020 */
- { DRM_MODE_COLORIMETRY_BT2020, "BT2020" },
+ [DRM_MODE_COLORIMETRY_BT2020] = "BT2020",
/* Colorimetry based on ITU-R BT.2020 */
- { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED, "BT2020_DEPRECATED" },
- /* Added as part of Additional Colorimetry Extension in 861.G */
- { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
- { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
+ [DRM_MODE_COLORIMETRY_BT2020_DEPRECATED] = "BT2020_DEPRECATED",
+ /* Colorimetry based on SMPTE RP 431-2 */
+ [DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65] = "P3_RGB_D65",
+ [DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER] = "P3_RGB_Theater",
+ [DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED] = "RGB_WIDE_FIXED",
+ /* Colorimetry based on scRGB (IEC 61966-2-2) */
+ [DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT] = "RGB_WIDE_FLOAT",
+ [DRM_MODE_COLORIMETRY_BT601_YCC] = "BT601_YCC",
};
+static const u32 hdmi_colorspaces =
+ BIT(DRM_MODE_COLORIMETRY_SMPTE_170M_YCC) |
+ BIT(DRM_MODE_COLORIMETRY_BT709_YCC) |
+ BIT(DRM_MODE_COLORIMETRY_XVYCC_601) |
+ BIT(DRM_MODE_COLORIMETRY_XVYCC_709) |
+ BIT(DRM_MODE_COLORIMETRY_SYCC_601) |
+ BIT(DRM_MODE_COLORIMETRY_OPYCC_601) |
+ BIT(DRM_MODE_COLORIMETRY_OPRGB) |
+ BIT(DRM_MODE_COLORIMETRY_BT2020_CYCC) |
+ BIT(DRM_MODE_COLORIMETRY_BT2020) |
+ BIT(DRM_MODE_COLORIMETRY_BT2020_DEPRECATED) |
+ BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) |
+ BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER);
+
/*
* As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel
Encoding/Colorimetry
* Format Table 2-120
*/
-static const struct drm_prop_enum_list dp_colorspaces[] = {
- /* For Default case, driver will set the colorspace */
- { DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
- { DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED, "RGB_Wide_Gamut_Fixed_Point" },
- /* Colorimetry based on scRGB (IEC 61966-2-2) */
- { DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT, "RGB_Wide_Gamut_Floating_Point"
},
- /* Colorimetry based on IEC 61966-2-5 */
- { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
- /* Colorimetry based on SMPTE RP 431-2 */
- { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
- /* Colorimetry based on ITU-R BT.2020 */
- {
