On 03/02/2023 01:45, Doug Anderson wrote:
Hi,
On Mon, Jan 23, 2023 at 8:05 AM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
Hi John,
On Mon, Jan 23, 2023 at 12:16:45PM +0000, John Keeping wrote:
On Sun, Jan 22, 2023 at 05:01:27PM +0200, Laurent Pinchart wrote:
On Sat, Jan 21, 2023 at 05:58:11PM +0000, John Keeping wrote:
On Sat, Jan 21, 2023 at 09:57:18AM +0100, Sam Ravnborg wrote:
On Fri, Jan 20, 2023 at 01:44:38PM -0800, Doug Anderson wrote:
On Fri, Jan 20, 2023 at 3:43 AM John Keeping wrote:
Commit 15b9ca1641f0 ("drm: Config orientation property if panel provides
it") added a helper to set the panel panel orientation early but only
connected this for drm_bridge_connector, which constructs a panel bridge
with DRM_BRIDGE_ATTACH_NO_CONNECTOR and creates the connector itself.
When the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is not specified and the
panel_bridge creates its own connector the orientation is not set unless
the panel does it in .get_modes which is too late and leads to a warning
splat from __drm_mode_object_add() because the device is already
registered.
Call the necessary function to set add the orientation property when the
connector is created so that it is available before the device is
registered.
I have no huge objection to your patch and it looks OK to me. That
being said, my understanding is that:
1. DRM_BRIDGE_ATTACH_NO_CONNECTOR is "the future" and not using the
flag is "deprecated".
Correct.
Could we take a look at how much is required to move the relevant driver
to use DRM_BRIDGE_ATTACH_NO_CONNECTOR?
If this is too much work now we may land this simple patch, but the
preference is to move all drivers to the new bridge handling and thus
asking display drivers to create the connector.
I fully agree with Doug and Sam here. Let's see if we can keep the yak
shaving minimal :-)
What display driver are we dealing with here?
This is dw-mipi-dsi-rockchip which uses the component path in
dw-mipi-dsi (and, in fact, is the only driver using that mode of
dw-mipi-dsi).
I'm not familiar enough with DRM to say whether it's easy to convert to
DRM_BRIDGE_ATTACH_NO_CONNECTOR - should dw-mipi-dsi-rockchip be moving
to use dw-mipi-dsi as a bridge driver or should dw_mipi_dsi_bind() have
a drm_bridge_attach_flags argument? But I'm happy to test patches if it
looks easy to convert to you :-)
I'd go for the former (use dw_mipi_dsi_probe() and acquire the DSI
bridge with of_drm_find_bridge() instead of using the component
framework) if possible, but I don't know how intrusive that would be.
I'm a bit confused about what's required since dw-mipi-dsi-rockchip
already uses dw_mipi_dsi_probe(),
Indeed, my bad.
but I think moving away from the
component framework would be significant work as that's how the MIPI
subdriver fits in to the overall Rockchip display driver.
It will be some work, yes. It however doesn't mean that the whole
Rockchip display driver needs to move away from the component framework,
it can be limited to the DSI encoder. It's not immediately clear to me
why the DSI encoder uses the component framework in the first place, and
if it would be difficult to move away from it.
Any changes / modernisation to the Rockchip MIPI driver look like it
will take more time than I have available to spend on this, so I'd
really like to see this patch land as it's a simple fix to an existing
working code path.
So who volunteers for fixing it properly ? :-)
I'll let Doug and Sam decide regarding mering this patch.
This thread seems to have gone silent.
I'm inclined to merge this patch (removing the "Fixes" tag) since it's
a one-line fix. While we want to encourage people to move to "the
future", it seems like it would be better to wait until someone is
trying to land something more intrusive than a 1-line fix before truly
forcing the issue.
I'll plan to merge the patch to drm-misc-next early next week assuming
there are no objections.
I'm ok for that,
Neil
-Doug
