On 22/08/2019 12:34, Boris Brezillon wrote:
> On Thu, 22 Aug 2019 12:22:02 +0200
> Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote:
>
>> On 22/08/2019 12:09, Boris Brezillon wrote:
>>> On Thu, 22 Aug 2019 11:29:40 +0200
>>> Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote:
>>>
>>>>>>> +/**
>>>>>>> + * struct drm_bus_caps - bus capabilities
>>>>>>> + * @supported_fmts: array of MEDIA_BUS_FMT_ formats
>>>>>>> + * @num_supported_fmts: size of the supported_fmts array
>>>>>>> + *
>>>>>>> + * Used by the core to negotiate the bus format at runtime.
>>>>>>> + */
>>>>>>> +struct drm_bus_caps {
>>>>>>> + const u32 *supported_fmts;
>>>>>>> + unsigned int num_supported_fmts;
>>>>>>> +};
>>>>>>> +
>>>>>>> /**
>>>>>>> * struct drm_bridge_state - Atomic bridge state object
>>>>>>> * @base: inherit from &drm_private_state
>>>>>>> * @bridge: the bridge this state refers to
>>>>>>> + * @input_bus_info: input bus information
>>>>>>> + * @output_bus_info: output bus information
>>>>>>
>>>>>> I would make this an array, to prepare for bridges that will have more
>>>>>> than one input or one output.
>>>>>
>>>>> Hm, not sure how you'd represent that. I guess you want to support
>>>>> bridges that can activate several inputs+outputs in parallel. Do we
>>>>> even support that right now?
>>>>> Also, how do we link the input to the output in that case? By their
>>>>> position in the array? And we'd need an extra field in bus_caps to
>>>>> point to the corresponding bridge input/output port.
>>>>>
>>>>> I'm glad we can discuss all those problems, but I'd like to focus on
>>>>> what's needed right now, not what could potentially be needed in the
>>>>> future. If we can design things to be more future-proof that's great,
>>>>> but in my experience, trying to solve problems that do not exist yet
>>>>> often leads to complex designs which prove to be wrong when we try to
>>>>> apply them to real situations when they finally occur.
>>>>
>>>> I had a thought when implementing bus format negotiation for dw-hdmi.
>>>>
>>>> Depending on the output bus-format, we could have different sets of possible
>>>> input drm_bus_caps.
>>>>
>>>> For example, if output is MEDIA_BUS_FMT_RGB888_1X24,
>>>> the input bus formats could only be MEDIA_BUS_FMT_RGB888_1X24 or MEDIA_BUS_FMT_YUV8_1X24.
>>>> Same for MEDIA_BUS_FMT_RGB101010_1X30, MEDIA_BUS_FMT_RGB121212_1X36, MEDIA_BUS_FMT_RGB161616_1X48...
>>>> And the special MEDIA_BUS_FMT_UYYVYY8_0_5X24, where input must be output.
>>>
>>> Sounds exactly like what Laurent was describing.
>>>
>>>>
>>>> How could we handle this ? I mean, could we have a fixed
>>>> output drm_bus_caps and be able to dynamically change the input drm_bus_caps ?
>>>
>>> If all output ends of bridges have fixed formats (meaning that
>>> something sets it to a fixed value, like a DT prop), there's no need
>>> for negotiation at all, since the output of the previous element in
>>> the chain will force input format of the current element.
>>>
>>>>
>>>> Adding matrix in struct drm_bridge seems over-engineered, no ?
>>>
>>> Will have to try it to give an answer to that question :).
>>
>> Another thought, I was thinking about a matrix with an optional callback checking
>> is this particular matrix line is possible, an example :
>>
>> struct drm_bridge_bus_fmt_assoc dw_hdmi_bus_fmt_matrix[] = {
>> { MEDIA_BUS_FMT_RGB888_1X24,
>> {MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24, 0},
>> {dw_hdmi_bus_fmt_no_yuv420_check, NULL},
>> },
>> { MEDIA_BUS_FMT_YUV8_1X24,
>> {MEDIA_BUS_FMT_YUV8_1X24, 0},
>> {dw_hdmi_bus_fmt_no_yuv420_check,
>> dw_hdmi_bus_fmt_yuv444_check,
>> NULL},
>> },
>> { MEDIA_BUS_FMT_RGB101010_1X30,
>> {MEDIA_BUS_FMT_RGB101010_1X30, MEDIA_BUS_FMT_YUV10_1X30, 0},
>> {dw_hdmi_bus_fmt_no_yuv420_check,
>> dw_hdmi_bus_fmt_rgb_10bit_check,
>> NULL},
>> },
>> { MEDIA_BUS_FMT_UYYVYY8_0_5X24,
>> {MEDIA_BUS_FMT_UYYVYY8_0_5X24, 0},
>> dw_hdmi_bus_fmt_yuv420_check,
>> },
>> { MEDIA_BUS_FMT_UYYVYY10_0_5X30,
>> {MEDIA_BUS_FMT_UYYVYY8_0_5X24, 0},
>> {dw_hdmi_bus_fmt_yuv420_check,
>> dw_hdmi_bus_fmt_yuv420_10bit_check,
>> NULL},
>> },
>> };
>>
>> Or with some flags combinations to check on the mode and some on the edid ?
>
> Okay, so you need to check/reject bus-formats at runtime (the mode is
> not known at attach time). Do you have an idea of what would be checked
> in those hooks?
>
The output bus format is dependent on the mode (drm_mode_is_420_only() and drm_mode_is_420_also())
and the display info(info->hdmi.scdc.supported, info->bpc and info->color_formats).
dw_hdmi_bus_fmt_yuv420_check would check :
drm_mode_is_420_only(info, mode) || (!info->hdmi.scdc.supported && drm_mode_is_420_also(info, mode))
and if encoder supports the corresponding input mode
dw_hdmi_bus_fmt_no_yuv420_check would be reverse.
dw_hdmi_bus_fmt_yuv444_check would check :
info->color_formats & DRM_COLOR_FORMAT_YCRCB444
and if encoder supports the corresponding input mode
dw_hdmi_bus_fmt_rgb_10bit_check would check:
info->bpc >= 10
and if encoder supports the corresponding input mode
10/12/16 bit selection is best-effort (at least how intel implemented it), this means
we would put the 16/12/10bits bus-formats first and 8bit last.
Same for YUV vs RGB, we should select YUV first if encoder supports it, otherwise RGB.
Neil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
