[PATCHv2 19/22] drm/bridge: tc358767: copy the mode data, instead of storing the pointer

[Andrzej Hajda]

On 15.04.2019 13:19, Tomi Valkeinen wrote:
> On 15/04/2019 13:09, Andrzej Hajda wrote:
>> On 26.03.2019 11:31, Tomi Valkeinen wrote:
>>> In tc_bridge_mode_set callback, we store the pointer to the given
>>> drm_display_mode, and use the mode later. Storing a pointer in such a
>>> way looks very suspicious to me, and I have observed odd issues where
>>> the timings were apparently (at least mostly) zero.
>>>
>>> Do a copy of the drm_display_mode instead to ensure we don't refer to
>>> freed/modified data.
>>
>> Why not tc->bridge.encoder->crtc->state->adjusted_mode or
>>
>> tc->bridge.encoder->crtc->state->mode ?
>>
>>
>> They should exists if the mode is set.
> Well, one reason was that my main concern was to get the DP output
> working (as it was quite broken wrt. the link setup), and I wanted to
> minimize all the other changes. This change felt the simplest way to fix
> this issue and get forward, without possibly causing new problems.
>
> Second, I'm a bit confused about DRM mode setting, and didn't want to
> possibly move the driver into the wrong direction. It's not clear to me
> whether the "mode" referred here is about the input or the output mode.
> And, in both cases, if there's a bridge between the CRTC and the
> TC358767, we would definitely be looking at the wrong mode if we peek at
> the CRTC's.


DRM does not support multiple intermediate modes, there is .mode as
requested by userspace and .adjusted_mode with twisted semantic, with
recent improvements[1].


[1]: https://www.kernel.org/doc/html/latest/gpu/drm-kms.html


Regards

Andrzej


>
>  Tomi
>