On Tue, May 31, 2022 at 6:56 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 5/30/22 13:34, Hsin-Yi Wang wrote: > > On Mon, May 30, 2022 at 4:53 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >> > >> Hi, > >> > >> On 5/30/22 10:19, Hsin-Yi Wang wrote: > >>> Some drivers, eg. mtk_drm and msm_drm, rely on the panel to set the > >>> orientation. Panel calls drm_connector_set_panel_orientation() to create > >>> orientation property and sets the value. However, connector properties > >>> can't be created after drm_dev_register() is called. The goal is to > >>> separate the orientation property creation, so drm drivers can create it > >>> earlier before drm_dev_register(). > >> > >> Sorry for jumping in pretty late in the discussion (based on the v10 > >> I seem to have missed this before). > >> > >> This sounds to me like the real issue here is that drm_dev_register() > >> is getting called too early? > >> > > Right. > > > >> To me it seems sensible to delay calling drm_dev_register() and > >> thus allowing userspace to start detecting available displays + > >> features until after the panel has been probed. > >> > > > > Most panels set this value very late, in .get_modes callback (since it > > is when the connector is known), though the value was known during > > panel probe. > > Hmm I would expect the main drm/kms driver to register the drm_connector > object after probing the panel, right ? > > So maybe this is a problem with the panel API? How about adding > separate callback to the panel API to get the orientation, which the > main drm/kms driver can then call before registering the connector ? > > And then have the main drm/kms driver call > drm_connector_set_panel_orientation() with the returned orientation > on the connecter before registering it. > > The new get_orientation callback for the panel should of course > be optional (IOW amy be NULL), so we probably want a small > helper for drivers using panel (sub)drivers to take care of > the process of getting the panel orientation from the panel > (if supported) and then setting it on the connector. > Hi Hans, Thanks for the suggestion. I've sent a new version for this: https://patchwork.kernel.org/project/dri-devel/patch/20220601081823.1038797-2-hsinyi@xxxxxxxxxxxx/ Panel can implement the optional callback to return the orientation property, while drm/kms driver will call a drm API to get the value then they can call drm_connector_set_panel_orientation(). Panel .get_mode will still call drm_connector_set_panel_orientation() but now it will be a no-op as the value was set by drm/kms driver previously. This is similar to the small patch below: https://patchwork.kernel.org/project/linux-mediatek/patch/20220530113033.124072-1-hsinyi@xxxxxxxxxxxx/ But it's now using the panel API. > > > I think we can also let drm check if they have remote panel nodes: If > > there is a panel and the panel sets the orientation, let the drm read > > this value and set the property. Does this workflow sound reasonable? > > > > The corresponding patch to implement this: > > https://patchwork.kernel.org/project/linux-mediatek/patch/20220530113033.124072-1-hsinyi@xxxxxxxxxxxx/ > > That is a suprisingly small patch (which is good). I guess that > my suggestion to add a new panel driver callback to get > the orientation would be a bit bigget then this. Still I think > that that would be a bit cleaner, as it would also solve this > for cases where the orientation comes from the panel itself > (through say some EDID extenstion) rather then from devicetree. > > Still I think either way should be acceptable upstream. > > Opinions from other drm devs on the above are very much welcome! > > Your small patch nicely avoids the probe ordering problem, > so it is much better then this patch series. > > Regards, > > Hans > > > > > > > Thanks > > > >> I see a devicetree patch in this series, so I guess that the panel > >> is described in devicetree. Especially in the case of devicetree > >> I would expect the kernel to have enough info to do the right > >> thing and make sure the panel is probed before calling > >> drm_dev_register() ? > >> > >> Regards, > >> > >> Hans > >> > >> > >> > >> > >>> > >>> After this series, drm_connector_set_panel_orientation() works like > >>> before. It won't affect existing callers of > >>> drm_connector_set_panel_orientation(). The only difference is that > >>> some drm drivers can call drm_connector_init_panel_orientation_property() > >>> earlier. > >>> > >>> Hsin-Yi Wang (4): > >>> gpu: drm: separate panel orientation property creating and value > >>> setting > >>> drm/mediatek: init panel orientation property > >>> drm/msm: init panel orientation property > >>> arm64: dts: mt8183: Add panel rotation > >>> > >>> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 + > >>> drivers/gpu/drm/drm_connector.c | 58 ++++++++++++++----- > >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 7 +++ > >>> drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 ++ > >>> include/drm/drm_connector.h | 2 + > >>> 5 files changed, 59 insertions(+), 13 deletions(-) > >>> > >> > > >
