Finding a possible CRTC by DDP component is done by first checking
static routes in three paths (main, external, third/extra path) and
then, if not found, we check for dynamic connection on a per-route
basis because, for example, on some SoCs the main route may output
to either a DSI display or DisplayPort and this is finally done by
assigning a CRTC mask to `possible_crtcs`, found with function
mtk_drm_find_comp_in_ddp_conn_path(): being that a mask the possible
values are BIT(x) and, if no CRTC is possible, zero.
Problem is, both mtk_drm_find_possible_crtc_by_comp() and the
aforementioned function are trying to return a negative error value
(but it's unsigned int!) if no CRTC was found, which is wrong for
multiple obvious reasons.
Cleanup both functions, so that:
- mtk_drm_find_comp_in_ddp_conn_path() returns a signed integer
with a negative number for error, or a bit/bitmask of the found
possible CRTC; and
- mtk_drm_find_possible_crtc_by_comp() always returns either a
bitmask of the possible CRTC, or zero if none available.
Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 44 ++++++++++-----------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index a9b5a21cde2d..c13359eeb3cd 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -513,29 +513,25 @@ static bool mtk_drm_find_comp_in_ddp(struct device *dev,
return false;
}
-static unsigned int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
- const struct mtk_drm_route *routes,
- unsigned int num_routes,
- struct mtk_ddp_comp *ddp_comp)
+static int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
+ const struct mtk_drm_route *routes,
+ unsigned int num_routes,
+ struct mtk_ddp_comp *ddp_comp)
{
- int ret;
- unsigned int i;
+ int i;
- if (!routes) {
- ret = -EINVAL;
- goto err;
+ if (!routes || !num_routes) {
+ DRM_ERROR("No connection routes specified!\n");
+ return -EINVAL;
}
for (i = 0; i < num_routes; i++)
if (dev == ddp_comp[routes[i].route_ddp].dev)
return BIT(routes[i].crtc_id);
- ret = -ENODEV;
-err:
-
- DRM_INFO("Failed to find comp in ddp table, ret = %d\n", ret);
+ DRM_ERROR("Failed to find component in ddp table\n");
- return 0;
+ return -ENODEV;
}
int mtk_ddp_comp_get_id(struct device_node *node,
@@ -557,22 +553,24 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
+ int ret;
if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path, private->data->main_len,
private->ddp_comp))
- ret = BIT(0);
+ return BIT(0);
else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
private->data->ext_len, private->ddp_comp))
- ret = BIT(1);
+ return BIT(1);
else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
private->data->third_len, private->ddp_comp))
- ret = BIT(2);
- else
- ret = mtk_drm_find_comp_in_ddp_conn_path(dev,
- private->data->conn_routes,
- private->data->num_conn_routes,
- private->ddp_comp);
+ return BIT(2);
+
+ ret = mtk_drm_find_comp_in_ddp_conn_path(dev, private->data->conn_routes,
+ private->data->num_conn_routes,
+ private->ddp_comp);
+ /* No CRTC is available: return a zero mask */
+ if (ret < 0)
+ return 0;
return ret;
}
--
2.43.0
Hi, Angelo:
On Thu, 2024-02-01 at 13:53 +0100, AngeloGioacchino Del Regno wrote:
> Finding a possible CRTC by DDP component is done by first checking
> static routes in three paths (main, external, third/extra path) and
> then, if not found, we check for dynamic connection on a per-route
> basis because, for example, on some SoCs the main route may output
> to either a DSI display or DisplayPort and this is finally done by
> assigning a CRTC mask to `possible_crtcs`, found with function
> mtk_drm_find_comp_in_ddp_conn_path(): being that a mask the possible
> values are BIT(x) and, if no CRTC is possible, zero.
>
> Problem is, both mtk_drm_find_possible_crtc_by_comp() and the
> aforementioned function are trying to return a negative error value
> (but it's unsigned int!) if no CRTC was found, which is wrong for
> multiple obvious reasons.
I does not find anywhere to return negative value. So this patch just
like a refine patch not bug fix.
Regards,
CK
>
> Cleanup both functions, so that:
> - mtk_drm_find_comp_in_ddp_conn_path() returns a signed integer
> with a negative number for error, or a bit/bitmask of the found
> possible CRTC; and
> - mtk_drm_find_possible_crtc_by_comp() always returns either a
> bitmask of the possible CRTC, or zero if none available.
>
> Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection
> capability")
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 44 ++++++++++---------
> --
> 1 file changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index a9b5a21cde2d..c13359eeb3cd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -513,29 +513,25 @@ static bool mtk_drm_find_comp_in_ddp(struct
> device *dev,
> return false;
> }
>
> -static unsigned int mtk_drm_find_comp_in_ddp_conn_path(struct device
> *dev,
> - const struct
> mtk_drm_route *routes,
> - unsigned int
> num_routes,
> - struct
> mtk_ddp_comp *ddp_comp)
> +static int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
> + const struct
> mtk_drm_route *routes,
> + unsigned int num_routes,
> + struct mtk_ddp_comp
> *ddp_comp)
> {
> - int ret;
> - unsigned int i;
> + int i;
>
> - if (!routes) {
> - ret = -EINVAL;
> - goto err;
> + if (!routes || !num_routes) {
> + DRM_ERROR("No connection routes specified!\n");
> + return -EINVAL;
> }
>
> for (i = 0; i < num_routes; i++)
> if (dev == ddp_comp[routes[i].route_ddp].dev)
> return BIT(routes[i].crtc_id);
>
> - ret = -ENODEV;
> -err:
> -
> - DRM_INFO("Failed to find comp in ddp table, ret = %d\n", ret);
> + DRM_ERROR("Failed to find component in ddp table\n");
>
> - return 0;
> + return -ENODEV;
> }
>
> int mtk_ddp_comp_get_id(struct device_node *node,
> @@ -557,22 +553,24 @@ unsigned int
> mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
> struct device *dev)
> {
> struct mtk_drm_private *private = drm->dev_private;
> - unsigned int ret = 0;
> + int ret;
>
> if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
> private->data->main_len,
> private->ddp_comp))
> - ret = BIT(0);
> + return BIT(0);
> else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
> private->data->ext_len,
> private->ddp_comp))
> - ret = BIT(1);
> + return BIT(1);
> else if (mtk_drm_find_comp_in_ddp(dev, private->data-
> >third_path,
> private->data->third_len,
> private->ddp_comp))
> - ret = BIT(2);
> - else
> - ret = mtk_drm_find_comp_in_ddp_conn_path(dev,
> - private->data-
> >conn_routes,
> - private->data-
> >num_conn_routes,
> - private-
> >ddp_comp);
> + return BIT(2);
> +
> + ret = mtk_drm_find_comp_in_ddp_conn_path(dev, private->data-
> >conn_routes,
> + private->data-
> >num_conn_routes,
> + private->ddp_comp);
> + /* No CRTC is available: return a zero mask */
> + if (ret < 0)
> + return 0;
>
> return ret;
> }