With new connector model, tc358767 will not create the connector, when
DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
rely on format negotiation to setup the encoder format.
Add the missing input-format negotiation hook in the
drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
the case with older model.
Reported-by: Jan Kiszka <[email protected]>
Signed-off-by: Aradhya Bhatia <[email protected]>
---
Notes:
* Since I do not have hardware with me, this was just build tested. I would
appreciate it if someone could test and review it, especically somebody, who
uses the bridge for DPI/DSI to eDP format conversion.
* The Toshiba TC358767 bridge is not enabled in arm64 defconfig by default,
when it should be. Hence, I sent a quick patch[0] earlier.
[0]: https://lore.kernel.org/all/[email protected]/
drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index ef2e373606ba..0affcefdeb1c 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
return input_fmts;
}
+static u32 *
+tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ u32 *input_fmts;
+
+ *num_input_fmts = 0;
+
+ input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+ GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ /* This is the DSI/DPI-end bus format */
+ input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+ *num_input_fmts = 1;
+
+ return input_fmts;
+}
+
static const struct drm_bridge_funcs tc_dpi_bridge_funcs = {
.attach = tc_dpi_bridge_attach,
.mode_valid = tc_dpi_mode_valid,
@@ -1777,6 +1801,7 @@ static const struct drm_bridge_funcs tc_edp_bridge_funcs = {
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_get_input_bus_fmts = tc_edp_atomic_get_input_bus_fmts,
};
static bool tc_readable_reg(struct device *dev, unsigned int reg)
base-commit: c503e3eec382ac708ee7adf874add37b77c5d312
--
2.42.0
On 30.10.23 20:28, Aradhya Bhatia wrote:
> With new connector model, tc358767 will not create the connector, when
> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
> rely on format negotiation to setup the encoder format.
>
> Add the missing input-format negotiation hook in the
> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
>
> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
> the case with older model.
>
> Reported-by: Jan Kiszka <[email protected]>
> Signed-off-by: Aradhya Bhatia <[email protected]>
> ---
>
> Notes:
>
> * Since I do not have hardware with me, this was just build tested. I would
> appreciate it if someone could test and review it, especically somebody, who
> uses the bridge for DPI/DSI to eDP format conversion.
>
> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by default,
> when it should be. Hence, I sent a quick patch[0] earlier.
>
> [0]: https://lore.kernel.org/all/[email protected]/
>
> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index ef2e373606ba..0affcefdeb1c 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> return input_fmts;
> }
>
> +static u32 *
> +tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts)
> +{
> + u32 *input_fmts;
> +
> + *num_input_fmts = 0;
> +
> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> + GFP_KERNEL);
> + if (!input_fmts)
> + return NULL;
> +
> + /* This is the DSI/DPI-end bus format */
> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> + *num_input_fmts = 1;
> +
> + return input_fmts;
> +}
> +
> static const struct drm_bridge_funcs tc_dpi_bridge_funcs = {
> .attach = tc_dpi_bridge_attach,
> .mode_valid = tc_dpi_mode_valid,
> @@ -1777,6 +1801,7 @@ static const struct drm_bridge_funcs tc_edp_bridge_funcs = {
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> .atomic_reset = drm_atomic_helper_bridge_reset,
> + .atomic_get_input_bus_fmts = tc_edp_atomic_get_input_bus_fmts,
> };
>
> static bool tc_readable_reg(struct device *dev, unsigned int reg)
>
> base-commit: c503e3eec382ac708ee7adf874add37b77c5d312
Doesn't help, callback is never invoked. There must be more missing.
Regarding test setup: Maybe your colleague Preneeth can help to give you
access, he just received some devices from us. Otherwise, drop me
instrumentation patches.
Jan
--
Siemens AG, Technology
Linux Expert Center
Hi Jan,
On 31/10/2023 08:24, Jan Kiszka wrote:
> On 30.10.23 20:28, Aradhya Bhatia wrote:
>> With new connector model, tc358767 will not create the connector, when
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
>> rely on format negotiation to setup the encoder format.
>>
>> Add the missing input-format negotiation hook in the
>> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
>>
>> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
>> the case with older model.
>>
>> Reported-by: Jan Kiszka <[email protected]>
>> Signed-off-by: Aradhya Bhatia <[email protected]>
>> ---
>>
>> Notes:
>>
>> * Since I do not have hardware with me, this was just build tested. I would
>> appreciate it if someone could test and review it, especically somebody, who
>> uses the bridge for DPI/DSI to eDP format conversion.
>>
>> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by default,
>> when it should be. Hence, I sent a quick patch[0] earlier.
>>
>> [0]: https://lore.kernel.org/all/[email protected]/
>>
>> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index ef2e373606ba..0affcefdeb1c 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>> return input_fmts;
>> }
>>
>> +static u32 *
>> +tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>> + struct drm_bridge_state *bridge_state,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state,
>> + u32 output_fmt,
>> + unsigned int *num_input_fmts)
>> +{
>> + u32 *input_fmts;
>> +
>> + *num_input_fmts = 0;
>> +
>> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>> + GFP_KERNEL);
>> + if (!input_fmts)
>> + return NULL;
>> +
>> + /* This is the DSI/DPI-end bus format */
>> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>> + *num_input_fmts = 1;
>> +
>> + return input_fmts;
>> +}
>> +
>> static const struct drm_bridge_funcs tc_dpi_bridge_funcs = {
>> .attach = tc_dpi_bridge_attach,
>> .mode_valid = tc_dpi_mode_valid,
>> @@ -1777,6 +1801,7 @@ static const struct drm_bridge_funcs tc_edp_bridge_funcs = {
>> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>> .atomic_reset = drm_atomic_helper_bridge_reset,
>> + .atomic_get_input_bus_fmts = tc_edp_atomic_get_input_bus_fmts,
>> };
>>
>> static bool tc_readable_reg(struct device *dev, unsigned int reg)
>>
>> base-commit: c503e3eec382ac708ee7adf874add37b77c5d312
>
> Doesn't help, callback is never invoked. There must be more missing.
>
> Regarding test setup: Maybe your colleague Preneeth can help to give you
> access, he just received some devices from us. Otherwise, drop me
> instrumentation patches.
Can you try with this change:
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 0affcefdeb1c..137a9f5e3cad 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1579,6 +1579,13 @@ static struct edid *tc_get_edid(struct drm_bridge *bridge,
struct drm_connector *connector)
{
struct tc_data *tc = bridge_to_tc(bridge);
+ int ret;
+
+ ret = tc_get_display_props(tc);
+ if (ret < 0) {
+ dev_err(tc->dev, "failed to read display props: %d\n", ret);
+ return 0;
+ }
return drm_get_edid(connector, &tc->aux.ddc);
}
Tomi
On 31.10.23 11:53, Tomi Valkeinen wrote:
> Hi Jan,
>
> On 31/10/2023 08:24, Jan Kiszka wrote:
>> On 30.10.23 20:28, Aradhya Bhatia wrote:
>>> With new connector model, tc358767 will not create the connector, when
>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
>>> rely on format negotiation to setup the encoder format.
>>>
>>> Add the missing input-format negotiation hook in the
>>> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
>>>
>>> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
>>> the case with older model.
>>>
>>> Reported-by: Jan Kiszka <[email protected]>
>>> Signed-off-by: Aradhya Bhatia <[email protected]>
>>> ---
>>>
>>> Notes:
>>>
>>> * Since I do not have hardware with me, this was just build
>>> tested. I would
>>> appreciate it if someone could test and review it, especically
>>> somebody, who
>>> uses the bridge for DPI/DSI to eDP format conversion.
>>>
>>> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by
>>> default,
>>> when it should be. Hence, I sent a quick patch[0] earlier.
>>>
>>> [0]:
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
>>> 1 file changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/tc358767.c
>>> b/drivers/gpu/drm/bridge/tc358767.c
>>> index ef2e373606ba..0affcefdeb1c 100644
>>> --- a/drivers/gpu/drm/bridge/tc358767.c
>>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>>> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct
>>> drm_bridge *bridge,
>>> return input_fmts;
>>> }
>>> +static u32 *
>>> +tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>>> + struct drm_bridge_state *bridge_state,
>>> + struct drm_crtc_state *crtc_state,
>>> + struct drm_connector_state *conn_state,
>>> + u32 output_fmt,
>>> + unsigned int *num_input_fmts)
>>> +{
>>> + u32 *input_fmts;
>>> +
>>> + *num_input_fmts = 0;
>>> +
>>> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>>> + GFP_KERNEL);
>>> + if (!input_fmts)
>>> + return NULL;
>>> +
>>> + /* This is the DSI/DPI-end bus format */
>>> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>> + *num_input_fmts = 1;
>>> +
>>> + return input_fmts;
>>> +}
>>> +
>>> static const struct drm_bridge_funcs tc_dpi_bridge_funcs = {
>>> .attach = tc_dpi_bridge_attach,
>>> .mode_valid = tc_dpi_mode_valid,
>>> @@ -1777,6 +1801,7 @@ static const struct drm_bridge_funcs
>>> tc_edp_bridge_funcs = {
>>> .atomic_duplicate_state =
>>> drm_atomic_helper_bridge_duplicate_state,
>>> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>>> .atomic_reset = drm_atomic_helper_bridge_reset,
>>> + .atomic_get_input_bus_fmts = tc_edp_atomic_get_input_bus_fmts,
>>> };
>>> static bool tc_readable_reg(struct device *dev, unsigned int reg)
>>>
>>> base-commit: c503e3eec382ac708ee7adf874add37b77c5d312
>>
>> Doesn't help, callback is never invoked. There must be more missing.
>>
>> Regarding test setup: Maybe your colleague Preneeth can help to give you
>> access, he just received some devices from us. Otherwise, drop me
>> instrumentation patches.
>
> Can you try with this change:
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 0affcefdeb1c..137a9f5e3cad 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1579,6 +1579,13 @@ static struct edid *tc_get_edid(struct drm_bridge
> *bridge,
> struct drm_connector *connector)
> {
> struct tc_data *tc = bridge_to_tc(bridge);
> + int ret;
> +
> + ret = tc_get_display_props(tc);
> + if (ret < 0) {
> + dev_err(tc->dev, "failed to read display props: %d\n",
> ret);
> + return 0;
> + }
>
> return drm_get_edid(connector, &tc->aux.ddc);
> }
>
> Tomi
>
Yep, that does the trick.
Thanks,
Jan
PS: Your mail client is mangling tabs - was already suspecting our
server would reformat again.
--
Siemens AG, Technology
Linux Expert Center
On 31/10/2023 14:18, Jan Kiszka wrote:
> On 31.10.23 11:53, Tomi Valkeinen wrote:
>> Hi Jan,
>>
>> On 31/10/2023 08:24, Jan Kiszka wrote:
>>> On 30.10.23 20:28, Aradhya Bhatia wrote:
>>>> With new connector model, tc358767 will not create the connector, when
>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
>>>> rely on format negotiation to setup the encoder format.
>>>>
>>>> Add the missing input-format negotiation hook in the
>>>> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
>>>>
>>>> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
>>>> the case with older model.
>>>>
>>>> Reported-by: Jan Kiszka <[email protected]>
>>>> Signed-off-by: Aradhya Bhatia <[email protected]>
>>>> ---
>>>>
>>>> Notes:
>>>>
>>>> * Since I do not have hardware with me, this was just build
>>>> tested. I would
>>>> appreciate it if someone could test and review it, especically
>>>> somebody, who
>>>> uses the bridge for DPI/DSI to eDP format conversion.
>>>>
>>>> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by
>>>> default,
>>>> when it should be. Hence, I sent a quick patch[0] earlier.
>>>>
>>>> [0]:
>>>> https://lore.kernel.org/all/[email protected]/
>>>>
>>>> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
>>>> 1 file changed, 25 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/tc358767.c
>>>> b/drivers/gpu/drm/bridge/tc358767.c
>>>> index ef2e373606ba..0affcefdeb1c 100644
>>>> --- a/drivers/gpu/drm/bridge/tc358767.c
>>>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>>>> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct
>>>> drm_bridge *bridge,
>>>> return input_fmts;
>>>> }
>>>> +static u32 *
>>>> +tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>>>> + struct drm_bridge_state *bridge_state,
>>>> + struct drm_crtc_state *crtc_state,
>>>> + struct drm_connector_state *conn_state,
>>>> + u32 output_fmt,
>>>> + unsigned int *num_input_fmts)
>>>> +{
>>>> + u32 *input_fmts;
>>>> +
>>>> + *num_input_fmts = 0;
>>>> +
>>>> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>>>> + GFP_KERNEL);
>>>> + if (!input_fmts)
>>>> + return NULL;
>>>> +
>>>> + /* This is the DSI/DPI-end bus format */
>>>> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>> + *num_input_fmts = 1;
>>>> +
>>>> + return input_fmts;
>>>> +}
>>>> +
>>>> static const struct drm_bridge_funcs tc_dpi_bridge_funcs = {
>>>> .attach = tc_dpi_bridge_attach,
>>>> .mode_valid = tc_dpi_mode_valid,
>>>> @@ -1777,6 +1801,7 @@ static const struct drm_bridge_funcs
>>>> tc_edp_bridge_funcs = {
>>>> .atomic_duplicate_state =
>>>> drm_atomic_helper_bridge_duplicate_state,
>>>> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>>>> .atomic_reset = drm_atomic_helper_bridge_reset,
>>>> + .atomic_get_input_bus_fmts = tc_edp_atomic_get_input_bus_fmts,
>>>> };
>>>> static bool tc_readable_reg(struct device *dev, unsigned int reg)
>>>>
>>>> base-commit: c503e3eec382ac708ee7adf874add37b77c5d312
>>>
>>> Doesn't help, callback is never invoked. There must be more missing.
>>>
>>> Regarding test setup: Maybe your colleague Preneeth can help to give you
>>> access, he just received some devices from us. Otherwise, drop me
>>> instrumentation patches.
>>
>> Can you try with this change:
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c
>> b/drivers/gpu/drm/bridge/tc358767.c
>> index 0affcefdeb1c..137a9f5e3cad 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -1579,6 +1579,13 @@ static struct edid *tc_get_edid(struct drm_bridge
>> *bridge,
>> struct drm_connector *connector)
>> {
>> struct tc_data *tc = bridge_to_tc(bridge);
>> + int ret;
>> +
>> + ret = tc_get_display_props(tc);
>> + if (ret < 0) {
>> + dev_err(tc->dev, "failed to read display props: %d\n",
>> ret);
>> + return 0;
>> + }
>>
>> return drm_get_edid(connector, &tc->aux.ddc);
>> }
>>
>> Tomi
>>
>
> Yep, that does the trick.
Alright, good. I'll send a proper patch.
> Thanks,
> Jan
>
> PS: Your mail client is mangling tabs - was already suspecting our
> server would reformat again.
Nah, the above was just a quick copy paste from my terminal window, and
when copying from the terminal it's always just spaces.
Tomi
Hi Aradhya,
On Tue, Oct 31, 2023 at 12:58:46AM +0530, Aradhya Bhatia wrote:
> With new connector model, tc358767 will not create the connector, when
> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
> rely on format negotiation to setup the encoder format.
>
> Add the missing input-format negotiation hook in the
> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
>
> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
> the case with older model.
>
> Reported-by: Jan Kiszka <[email protected]>
> Signed-off-by: Aradhya Bhatia <[email protected]>
> ---
>
> Notes:
>
> * Since I do not have hardware with me, this was just build tested. I would
> appreciate it if someone could test and review it, especically somebody, who
> uses the bridge for DPI/DSI to eDP format conversion.
>
> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by default,
> when it should be. Hence, I sent a quick patch[0] earlier.
>
> [0]: https://lore.kernel.org/all/[email protected]/
>
> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index ef2e373606ba..0affcefdeb1c 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> return input_fmts;
> }
>
> +static u32 *
> +tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts)
> +{
> + u32 *input_fmts;
> +
> + *num_input_fmts = 0;
> +
> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> + GFP_KERNEL);
> + if (!input_fmts)
> + return NULL;
> +
> + /* This is the DSI/DPI-end bus format */
> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> + *num_input_fmts = 1;
> +
> + return input_fmts;
> +}
You could benefit from using the helper:
drm_atomic_helper_bridge_propagate_bus_fmt()
Sam
Hi Sam,
Thank you for the suggestion!
On 06-Nov-23 18:08, Sam Ravnborg wrote:
> Hi Aradhya,
>
> On Tue, Oct 31, 2023 at 12:58:46AM +0530, Aradhya Bhatia wrote:
>> With new connector model, tc358767 will not create the connector, when
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will
>> rely on format negotiation to setup the encoder format.
>>
>> Add the missing input-format negotiation hook in the
>> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support.
>>
>> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
>> the case with older model.
>>
>> Reported-by: Jan Kiszka <[email protected]>
>> Signed-off-by: Aradhya Bhatia <[email protected]>
>> ---
>>
>> Notes:
>>
>> * Since I do not have hardware with me, this was just build tested. I would
>> appreciate it if someone could test and review it, especically somebody, who
>> uses the bridge for DPI/DSI to eDP format conversion.
>>
>> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by default,
>> when it should be. Hence, I sent a quick patch[0] earlier.
>>
>> [0]: https://lore.kernel.org/all/[email protected]/
>>
>> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index ef2e373606ba..0affcefdeb1c 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>> return input_fmts;
>> }
>>
>> +static u32 *
>> +tc_edp_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>> + struct drm_bridge_state *bridge_state,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state,
>> + u32 output_fmt,
>> + unsigned int *num_input_fmts)
>> +{
>> + u32 *input_fmts;
>> +
>> + *num_input_fmts = 0;
>> +
>> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>> + GFP_KERNEL);
>> + if (!input_fmts)
>> + return NULL;
>> +
>> + /* This is the DSI/DPI-end bus format */
>> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>> + *num_input_fmts = 1;
>> +
>> + return input_fmts;
>> +}
>
> You could benefit from using the helper:
> drm_atomic_helper_bridge_propagate_bus_fmt()
You are right!
Upon taking a second look, I realize that the bridge chain works with
MEDIA_BUS_FMT_FIXED bus format, when tc358767 is being used in DPI/DSI
to eDP mode (because the panel-bridge does not have a get_output_bus_fmt
hook, and uses the same helper for its get_input_bus_fmt hook). My patch
creates a deviation from that, by forcing MEDIA_BUS_FMT_RGB888_1X24 even
when eDP is involved.
Using the helper here, will certainly address this deviation.
However, for the DPI/DSI to DP mode, MEDIA_BUS_FMT_RGB888_1X24 bus
format is required, and *just* using the helper as its get_input_bus_fmt
hook, might not be enough.
Since tc358767 is the last bridge in DPI/DSI to DP mode, the
output_fmt parameter get defaulted to MEDIA_BUS_FMT_FIXED too, as there
is no get_output_bus_fmt hook present in the driver. If we simply us
the helper here, the input_fmt will also get set to MEDIA_BUS_FMT_FIXED.
This too is an unwanted deviation.
It seems like the right way to address both the cases, would be by
adding the get_output_bus_fmt hook that sets output_fmt to
MEDIA_BUS_FMT_RGB888_1X24, as well as using the helper as the
get_input_bus_fmt hook.
If this seems good to you too, I will send a new version of Tomi's
series[0] which incorporates this patch.
Regards
Aradhya
[0]:
https://lore.kernel.org/all/[email protected]/