On 23/01/2020 08:52, Jonas Karlman wrote:
> On 2020-01-23 08:39, Boris Brezillon wrote:
>> On Wed, 22 Jan 2020 23:44:28 +0000 (UTC)
>> Jonas Karlman <jo...@kwiboo.se> wrote:
>>
>>>> +static int
>>>> +drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge,
>>>> +                                  struct drm_crtc_state *crtc_state,
>>>> +                                  struct drm_connector_state *conn_state)
>>>> +{
>>>> +  struct drm_connector *conn = conn_state->connector;
>>>> +  struct drm_encoder *encoder = bridge->encoder;
>>>> +  struct drm_bridge_state *last_bridge_state;
>>>> +  unsigned int i, num_out_bus_fmts;
>>>> +  struct drm_bridge *last_bridge;
>>>> +  u32 *out_bus_fmts;
>>>> +  int ret = 0;
>>>> +
>>>> +  last_bridge = list_last_entry(&encoder->bridge_chain,
>>>> +                                struct drm_bridge, chain_node);
>>>> +  last_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
>>>> +                                                      last_bridge);
>>>> +
>>>> +  if (last_bridge->funcs->atomic_get_output_bus_fmts) {
>>>> +          const struct drm_bridge_funcs *funcs = last_bridge->funcs;
>>>> +
>>>> +          /*
>>>> +           * If the driver implements ->atomic_get_output_bus_fmts() it
>>>> +           * should also implement the atomic state hooks.
>>>> +           */
>>>> +          if (WARN_ON(last_bridge_state))  
>>>
>>> This looks wrong, with this changed to WARN_ON(!last_bridge_state)
>>> my RK3328 HDMI2.0/YUV444/YUV420/10-bit branch at [1] starts working.
>>>
>>> With WARN_ON(last_bridge_state) I get:
>>>
>>> [    6.606658] WARNING: CPU: 0 PID: 167 at drivers/gpu/drm/drm_bridge.c:746 
>>> drm_atomic_bridge_chain_check+0x2b8/0x308
>>> [    6.606673] Hardware name: Pine64 Rock64 (DT)
>>>
>>> [    6.606754] Call trace:
>>> [    6.606759]  drm_atomic_bridge_chain_check+0x2b8/0x308
>>> [    6.606764]  drm_atomic_helper_check_modeset+0x89c/0xab8
>>> [    6.606768]  drm_atomic_helper_check+0x1c/0xa0
>>> [    6.606772]  drm_atomic_check_only+0x464/0x708
>>> [    6.606777]  drm_atomic_commit+0x18/0x58
>>
>> Add
>>
>> const drm_bridge_funcs ... = {
>>      ...
>>      .atomic_reset = drm_atomic_helper_bridge_reset,
>>      .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>>      .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>>      ...
>> };
>>
>> and that should work.
> 
> That is what I added and what made that this warning is being triggered.
> The comment state that atomic state is needed, but the check warns when there 
> is a state.
> 
> I have this:
> 
> static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>       ...
>       .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>       .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>       .atomic_get_output_bus_fmts = dw_hdmi_bridge_atomic_get_output_bus_fmts,
>       .atomic_get_input_bus_fmts = dw_hdmi_bridge_atomic_get_input_bus_fmts,
>       .atomic_check = dw_hdmi_bridge_atomic_check,
>       .atomic_reset = drm_atomic_helper_bridge_reset,
>       ...
> };
> 
> and
> 
> static const struct drm_bridge_funcs dw_hdmi_rockchip_bridge_funcs = {
>       ...
>       .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>       .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>       .atomic_get_input_bus_fmts = dw_hdmi_rockchip_get_input_bus_fmts,
>       .atomic_check = dw_hdmi_rockchip_bridge_atomic_check,
>       .atomic_reset = drm_atomic_helper_bridge_reset,
> };
> 
> after applying the following I got a hdmi signal again
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0c28816146ba..7e7b0fac8f4f 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -743,7 +743,7 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge 
> *bridge,
>                * If the driver implements ->atomic_get_output_bus_fmts() it
>                * should also implement the atomic state hooks.
>                */
> -             if (WARN_ON(last_bridge_state))
> +             if (WARN_ON(!last_bridge_state))
>                       return -EINVAL;
>  
>               out_bus_fmts = funcs->atomic_get_output_bus_fmts(last_bridge,
> 
> Regards,
> Jonas
> 
>>
>>>
>>> [1] 
>>> https://github.com/Kwiboo/linux-rockchip/commits/next-20200122-bus-format
>>>
>>> Regards,
>>> Jonas

With this fix :
Tested-by: Neil Armstrong <narmstr...@baylibre.com>

Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to