2020-11-13 09:49:21

by Yuti Amonkar

[permalink] [raw]
Subject: [PATCH v1 0/4] Add bus format negotiation support for Cadence MHDP8546 driver

This patch series add bus format negotiation support for Cadence MHDP8546 bridge
driver.

The patch series has four patches in the below sequence:
1. drm: bridge: cdns-mhdp8546: Add output bus format negotiation
Add minimal output bus format negotiation support.
2. drm: bridge: cdns-mhdp8546: Modify atomic_get_input_bus_format bridge function.
Get the input format based on output format supported.
3. drm: bridge: cdns-mhdp8546: Remove setting of bus format using connector info
Remove the bus format configuration using connector info structure.
4. drm: bridge: cdns-mhdp8546: Retrieve the pixel format and bpc based on bus format
Get the pixel format and bpc based on negotiated output bus format.

This patch series is dependent on tidss series [1] for the new connector model support.

[1]
https://patchwork.kernel.org/project/dri-devel/cover/[email protected]/

Yuti Amonkar (4):
drm: bridge: cdns-mhdp8546: Add output bus format negotiation
drm: bridge: cdns-mhdp8546: Modify atomic_get_input_bus_format bridge
function
drm: bridge: cdns-mhdp8546: Remove setting of bus format using
connector info
drm: bridge: cdns-mhdp8546: Retrieve the pixel format and bpc based on
bus format

.../drm/bridge/cadence/cdns-mhdp8546-core.c | 216 +++++++++++++++---
1 file changed, 182 insertions(+), 34 deletions(-)

--
2.17.1


2020-11-13 09:49:25

by Yuti Amonkar

[permalink] [raw]
Subject: [PATCH v1 2/4] drm: bridge: cdns-mhdp8546: Modify atomic_get_input_bus_format bridge function

Modify atomic_get_input_bus_format function to return input formats
based on the output format instead of using hardcoded value.

Signed-off-by: Yuti Amonkar <[email protected]>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 110 ++++++++++++++++--
1 file changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index bdb0d95aa412..623eadb8948f 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2078,27 +2078,117 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge *bridge)
return &cdns_mhdp_state->base;
}

+static const u32 cdns_mhdp_bus_fmts[] = {
+ MEDIA_BUS_FMT_YUV16_1X48,
+ MEDIA_BUS_FMT_RGB161616_1X48,
+ MEDIA_BUS_FMT_UYVY12_1X24,
+ MEDIA_BUS_FMT_YUV12_1X36,
+ MEDIA_BUS_FMT_RGB121212_1X36,
+ MEDIA_BUS_FMT_UYVY10_1X20,
+ MEDIA_BUS_FMT_YUV10_1X30,
+ MEDIA_BUS_FMT_RGB101010_1X30,
+ MEDIA_BUS_FMT_UYVY8_1X16,
+ MEDIA_BUS_FMT_YUV8_1X24,
+ MEDIA_BUS_FMT_RGB888_1X24
+};
+
+static bool cdns_mhdp_format_supported(u32 output_fmt)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(cdns_mhdp_bus_fmts); i++) {
+ if (output_fmt == cdns_mhdp_bus_fmts[i])
+ return true;
+ }
+
+ return false;
+}
+
+#define MAX_INPUT_FORMAT 4
+
static u32 *cdns_mhdp_get_input_bus_fmts(struct drm_bridge *bridge,
- struct drm_bridge_state *bridge_state,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state,
- u32 output_fmt,
- unsigned int *num_input_fmts)
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
{
u32 *input_fmts;
- u32 default_bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
+ unsigned int i = 0;

*num_input_fmts = 0;

- if (output_fmt != MEDIA_BUS_FMT_FIXED)
+ if (!cdns_mhdp_format_supported(output_fmt) &&
+ output_fmt != MEDIA_BUS_FMT_FIXED)
return NULL;

- input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
+ input_fmts = kcalloc(MAX_INPUT_FORMAT,
+ sizeof(*input_fmts), GFP_KERNEL);
if (!input_fmts)
return NULL;

- *num_input_fmts = 1;
- input_fmts[0] = default_bus_format;
+ switch (output_fmt) {
+ /* RGB */
+ case MEDIA_BUS_FMT_RGB161616_1X48:
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+ case MEDIA_BUS_FMT_RGB121212_1X36:
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+ case MEDIA_BUS_FMT_RGB101010_1X30:
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+
+ /* YUV444 */
+ case MEDIA_BUS_FMT_YUV16_1X48:
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+ break;
+ case MEDIA_BUS_FMT_YUV12_1X36:
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+ break;
+ case MEDIA_BUS_FMT_YUV10_1X30:
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+ break;
+ case MEDIA_BUS_FMT_YUV8_1X24:
+ input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+ break;
+
+ /* YUV422 */
+ case MEDIA_BUS_FMT_UYVY12_1X24:
+ input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
+ input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
+ input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+ break;
+ case MEDIA_BUS_FMT_UYVY10_1X20:
+ input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
+ input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+ break;
+ case MEDIA_BUS_FMT_UYVY8_1X16:
+ input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+ break;
+
+ case MEDIA_BUS_FMT_FIXED:
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+ break;
+ }
+
+ *num_input_fmts = i;
+
return input_fmts;
}

--
2.17.1

2020-11-13 09:49:30

by Yuti Amonkar

[permalink] [raw]
Subject: [PATCH v1 1/4] drm: bridge: cdns-mhdp8546: Add output bus format negotiation

This patch adds minimal output bus format negotiation support.
Currently we are adding support for only MEDIA_BUS_FMT_FIXED.

Signed-off-by: Yuti Amonkar <[email protected]>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 6beccd2a408e..bdb0d95aa412 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2102,6 +2102,23 @@ static u32 *cdns_mhdp_get_input_bus_fmts(struct drm_bridge *bridge,
return input_fmts;
}

+static u32 *cdns_mhdp_get_output_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ unsigned int *num_output_fmts)
+{
+ u32 *output_fmts;
+
+ output_fmts = kzalloc(sizeof(*output_fmts), GFP_KERNEL);
+ if (!output_fmts)
+ return NULL;
+
+ *num_output_fmts = 1;
+ output_fmts[0] = MEDIA_BUS_FMT_FIXED;
+ return output_fmts;
+}
+
static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,
struct drm_crtc_state *crtc_state,
@@ -2170,6 +2187,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
.atomic_destroy_state = cdns_mhdp_bridge_atomic_destroy_state,
.atomic_reset = cdns_mhdp_bridge_atomic_reset,
.atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts,
+ .atomic_get_output_bus_fmts = cdns_mhdp_get_output_bus_fmts,
.detect = cdns_mhdp_bridge_detect,
.get_edid = cdns_mhdp_bridge_get_edid,
.hpd_enable = cdns_mhdp_bridge_hpd_enable,
--
2.17.1

2020-11-13 09:50:11

by Yuti Amonkar

[permalink] [raw]
Subject: [PATCH v1 4/4] drm: bridge: cdns-mhdp8546: Retrieve the pixel format and bpc based on bus format

Get the pixel format and bpc based on the output bus format
negotiated instead of hardcoding the values.

Signed-off-by: Yuti Amonkar <[email protected]>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 82 +++++++++++++++----
1 file changed, 64 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 6f900bceb50c..44d79b0bd6d2 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1512,24 +1512,8 @@ static int cdns_mhdp_get_modes(struct drm_connector *connector)

drm_connector_update_edid_property(connector, edid);
num_modes = drm_add_edid_modes(connector, edid);
- kfree(edid);

- /*
- * HACK: Warn about unsupported display formats until we deal
- * with them correctly.
- */
- if (connector->display_info.color_formats &&
- !(connector->display_info.color_formats &
- mhdp->display_fmt.color_format))
- dev_warn(mhdp->dev,
- "%s: No supported color_format found (0x%08x)\n",
- __func__, connector->display_info.color_formats);
-
- if (connector->display_info.bpc &&
- connector->display_info.bpc < mhdp->display_fmt.bpc)
- dev_warn(mhdp->dev, "%s: Display bpc only %d < %d\n",
- __func__, connector->display_info.bpc,
- mhdp->display_fmt.bpc);
+ kfree(edid);

return num_modes;
}
@@ -1689,6 +1673,66 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
return 0;
}

+static void cdns_mhdp_get_display_fmt(struct cdns_mhdp_device *mhdp,
+ struct drm_bridge_state *state)
+{
+ u32 bus_fmt, bpc, pxlfmt;
+
+ bus_fmt = state->output_bus_cfg.format;
+ switch (bus_fmt) {
+ case MEDIA_BUS_FMT_RGB161616_1X48:
+ pxlfmt = DRM_COLOR_FORMAT_RGB444;
+ bpc = 16;
+ break;
+ case MEDIA_BUS_FMT_YUV16_1X48:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB444;
+ bpc = 16;
+ break;
+ case MEDIA_BUS_FMT_RGB121212_1X36:
+ pxlfmt = DRM_COLOR_FORMAT_RGB444;
+ bpc = 12;
+ break;
+ case MEDIA_BUS_FMT_UYVY12_1X24:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB422;
+ bpc = 12;
+ break;
+ case MEDIA_BUS_FMT_YUV12_1X36:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB444;
+ bpc = 12;
+ break;
+ case MEDIA_BUS_FMT_RGB101010_1X30:
+ pxlfmt = DRM_COLOR_FORMAT_RGB444;
+ bpc = 10;
+ break;
+ case MEDIA_BUS_FMT_UYVY10_1X20:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB422;
+ bpc = 10;
+ break;
+ case MEDIA_BUS_FMT_YUV10_1X30:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB444;
+ bpc = 10;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ pxlfmt = DRM_COLOR_FORMAT_RGB444;
+ bpc = 8;
+ break;
+ case MEDIA_BUS_FMT_UYVY8_1X16:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB422;
+ bpc = 8;
+ break;
+ case MEDIA_BUS_FMT_YUV8_1X24:
+ pxlfmt = DRM_COLOR_FORMAT_YCRCB444;
+ bpc = 8;
+ break;
+ default:
+ pxlfmt = DRM_COLOR_FORMAT_RGB444;
+ bpc = 8;
+ }
+
+ mhdp->display_fmt.color_format = pxlfmt;
+ mhdp->display_fmt.bpc = bpc;
+}
+
static void cdns_mhdp_configure_video(struct cdns_mhdp_device *mhdp,
const struct drm_display_mode *mode)
{
@@ -2211,6 +2255,8 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
const struct drm_display_mode *mode = &crtc_state->adjusted_mode;

+ cdns_mhdp_get_display_fmt(mhdp, bridge_state);
+
mutex_lock(&mhdp->link_mutex);

if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
@@ -2539,7 +2585,7 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
mhdp->link.rate = mhdp->host.link_rate;
mhdp->link.num_lanes = mhdp->host.lanes_cnt;

- /* The only currently supported format */
+ /* Initialize color format bpc and y_only to default values*/
mhdp->display_fmt.y_only = false;
mhdp->display_fmt.color_format = DRM_COLOR_FORMAT_RGB444;
mhdp->display_fmt.bpc = 8;
--
2.17.1

2020-11-13 09:51:46

by Yuti Amonkar

[permalink] [raw]
Subject: [PATCH v1 3/4] drm: bridge: cdns-mhdp8546: Remove setting of bus format using connector info

As we are using bus negotiations for selecting bus format
remove the setting of bus format using the connector info
structure.

Signed-off-by: Yuti Amonkar <[email protected]>
---
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 623eadb8948f..6f900bceb50c 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1630,7 +1630,6 @@ static const struct drm_connector_funcs cdns_mhdp_conn_funcs = {

static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)
{
- u32 bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
struct drm_connector *conn = &mhdp->connector;
struct drm_bridge *bridge = &mhdp->bridge;
int ret;
@@ -1651,11 +1650,6 @@ static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)

drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);

- ret = drm_display_info_set_bus_formats(&conn->display_info,
- &bus_format, 1);
- if (ret)
- return ret;
-
ret = drm_connector_attach_encoder(conn, bridge->encoder);
if (ret) {
dev_err(mhdp->dev, "Failed to attach connector to encoder\n");
--
2.17.1

2020-11-16 20:15:31

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] Add bus format negotiation support for Cadence MHDP8546 driver

Hi,

On 13/11/2020 11:46, Yuti Amonkar wrote:
> This patch series add bus format negotiation support for Cadence MHDP8546 bridge
> driver.
>
> The patch series has four patches in the below sequence:
> 1. drm: bridge: cdns-mhdp8546: Add output bus format negotiation
> Add minimal output bus format negotiation support.
> 2. drm: bridge: cdns-mhdp8546: Modify atomic_get_input_bus_format bridge function.
> Get the input format based on output format supported.
> 3. drm: bridge: cdns-mhdp8546: Remove setting of bus format using connector info
> Remove the bus format configuration using connector info structure.
> 4. drm: bridge: cdns-mhdp8546: Retrieve the pixel format and bpc based on bus format
> Get the pixel format and bpc based on negotiated output bus format.
>
> This patch series is dependent on tidss series [1] for the new connector model support.
>
> [1]
> https://patchwork.kernel.org/project/dri-devel/cover/[email protected]/

Can you explain how this works?

Afaics, what we should be doing is:

- We don't have proper bus formats for DP output, so we need to use MEDIA_BUS_FMT_FIXED as the
output format. This is what you do in the first patch. (But is that patch even needed, if
MEDIA_BUS_FMT_FIXED is the default anyway)

- In cdns_mhdp_get_input_bus_fmts, the function should exit if the given output format is not
MEDIA_BUS_FMT_FIXED.

- In cdns_mhdp_get_input_bus_fmts, the driver should return all the RGB bus formats, if MHDP is able
to upscale/downscale RGB (e.g. RGB 8-bpc to RGB 12-bpc).

- If the monitor supports YUV modes according to the display_info, cdns_mhdp_get_input_bus_fmts can
also return those.

- Then later, in atomic_check and commit, mhdp driver has the negotiated bus format, which it should
use to program the registers.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki