Re: [PATCH v10 0/4] Separate panel orientation property creating and value setting

2022-06-01 Thread Hsin-Yi Wang 
On Tue, May 31, 2022 at 6:56 PM Hans de Goede  wrote:
>
> Hi,
>
> On 5/30/22 13:34, Hsin-Yi Wang wrote:
> > On Mon, May 30, 2022 at 4:53 PM Hans de Goede  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-hsi...@chromium.org/

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-hsi...@chromium.org/
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-hsi...@chromium.org/
>
> 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 dr

[PATCH v10 2/4] drm/mediatek: init panel orientation property

2022-05-31 Thread Hsin-Yi Wang 
Init panel orientation property after connector is initialized. Let the
panel driver decides the orientation value later.

Signed-off-by: Hsin-Yi Wang 
Acked-by: Chun-Kuang Hu 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index d9f10a33e6fa..3db44d9b161a 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -822,6 +822,13 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, 
struct mtk_dsi *dsi)
ret = PTR_ERR(dsi->connector);
goto err_cleanup_encoder;
}
+
+   ret = drm_connector_init_panel_orientation_property(dsi->connector);
+   if (ret) {
+   DRM_ERROR("Unable to init panel orientation\n");
+   goto err_cleanup_encoder;
+   }
+
drm_connector_attach_encoder(dsi->connector, >encoder);
 
return 0;
-- 
2.36.1.124.g0e6072fb45-goog

Re: [PATCH v10 0/4] Separate panel orientation property creating and value setting

2022-05-31 Thread Hsin-Yi Wang 
On Mon, May 30, 2022 at 4:53 PM Hans de Goede  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.

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-hsi...@chromium.org/

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(-)
> >
>

[PATCH v10 4/4] arm64: dts: mt8183: Add panel rotation

2022-05-31 Thread Hsin-Yi Wang 
krane, kakadu, and kodama boards have a default panel rotation.

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Enric Balletbo i Serra 
Tested-by: Enric Balletbo i Serra 
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index 8d5bf73a9099..f0dd5a06629d 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -276,6 +276,7 @@ panel: panel@0 {
avee-supply = <_lcd>;
pp1800-supply = <_lcd>;
backlight = <_lcd0>;
+   rotation = <270>;
port {
panel_in: endpoint {
remote-endpoint = <_out>;
-- 
2.36.1.124.g0e6072fb45-goog

[PATCH v10 1/4] gpu: drm: separate panel orientation property creating and value setting

2022-05-31 Thread Hsin-Yi Wang 
drm_dev_register() sets connector->registration_state to
DRM_CONNECTOR_REGISTERED and dev->registered to true. If
drm_connector_set_panel_orientation() is first called after
drm_dev_register(), it will fail several checks and results in following
warning.

Add a function to create panel orientation property and set default value
to UNKNOWN, so drivers can call this function to init the property earlier
, and let the panel set the real value later.

[4.480976] [ cut here ]
[4.485603] WARNING: CPU: 5 PID: 369 at drivers/gpu/drm/drm_mode_object.c:45 
__drm_mode_object_add+0xb4/0xbc

[4.609772] Call trace:
[4.612208]  __drm_mode_object_add+0xb4/0xbc
[4.616466]  drm_mode_object_add+0x20/0x2c
[4.620552]  drm_property_create+0xdc/0x174
[4.624723]  drm_property_create_enum+0x34/0x98
[4.629241]  drm_connector_set_panel_orientation+0x64/0xa0
[4.634716]  boe_panel_get_modes+0x88/0xd8
[4.638802]  drm_panel_get_modes+0x2c/0x48
[4.642887]  panel_bridge_get_modes+0x1c/0x28
[4.647233]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.652273]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.658266]  drm_mode_getconnector+0x1b4/0x45c
[4.662699]  drm_ioctl_kernel+0xac/0x128
[4.11]  drm_ioctl+0x268/0x410
[4.670002]  drm_compat_ioctl+0xdc/0xf0
[4.673829]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.678436]  el0_svc_common+0xf4/0x1c0
[4.682174]  do_el0_svc_compat+0x28/0x3c
[4.686088]  el0_svc_compat+0x10/0x1c
[4.689738]  el0_sync_compat_handler+0xa8/0xcc
[4.694171]  el0_sync_compat+0x178/0x180
[4.698082] ---[ end trace b4f2db9d9c88610b ]---
[4.702721] [ cut here ]
[4.707329] WARNING: CPU: 5 PID: 369 at 
drivers/gpu/drm/drm_mode_object.c:243 drm_object_attach_property+0x48/0xb8

[4.833830] Call trace:
[4.836266]  drm_object_attach_property+0x48/0xb8
[4.840958]  drm_connector_set_panel_orientation+0x84/0xa0
[4.846432]  boe_panel_get_modes+0x88/0xd8
[4.850516]  drm_panel_get_modes+0x2c/0x48
[4.854600]  panel_bridge_get_modes+0x1c/0x28
[4.858946]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.863984]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.869978]  drm_mode_getconnector+0x1b4/0x45c
[4.874410]  drm_ioctl_kernel+0xac/0x128
[4.878320]  drm_ioctl+0x268/0x410
[4.881711]  drm_compat_ioctl+0xdc/0xf0
[4.885536]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.890142]  el0_svc_common+0xf4/0x1c0
[4.893879]  do_el0_svc_compat+0x28/0x3c
[4.897791]  el0_svc_compat+0x10/0x1c
[4.901441]  el0_sync_compat_handler+0xa8/0xcc
[4.905873]  el0_sync_compat+0x178/0x180
[4.909783] ---[ end trace b4f2db9d9c88610c ]---

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Sean Paul 
---
v9->v10: rebase to latest linux-next.
v9: 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220318074825.3359978-2-hsi...@chromium.org/
v8: 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220208084234.1684930-1-hsi...@chromium.org/
v7: 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220208073714.1540390-1-hsi...@chromium.org/
---
 drivers/gpu/drm/drm_connector.c | 58 +
 include/drm/drm_connector.h |  2 ++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 1c48d162c77e..d68cc78f6684 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1252,7 +1252,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = 
{
  * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
  * coordinates, so if userspace rotates the picture to adjust for
  * the orientation it must also apply the same transformation to the
- * touchscreen input coordinates. This property is initialized by calling
+ * touchscreen input coordinates. This property value is set by calling
  * drm_connector_set_panel_orientation() or
  * drm_connector_set_panel_orientation_with_quirk()
  *
@@ -2310,8 +2310,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
  * @connector: connector for which to set the panel-orientation property.
  * @panel_orientation: drm_panel_orientation value to set
  *
- * This function sets the connector's panel_orientation and attaches
- * a "panel orientation" property to the connector.
+ * This function sets the connector's panel_orientation value. If the property
+ * doesn't exist, it will try to create one.
  *
  * Calling this function on a connector where the panel_orientation has
  * already been set is a no-op (e.g. the orientation has been overridden with
@@ -2343,18 +2343,13 @@ int drm_connector_set_panel_orientation(
 
prop = dev->mode_config.panel_orientation_property;
if (!prop) {
-   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
-

[PATCH v10 3/4] drm/msm: init panel orientation property

2022-05-31 Thread Hsin-Yi Wang 
Init panel orientation property after connector is initialized. Let the
panel driver decides the orientation value later.

Signed-off-by: Hsin-Yi Wang 
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index cb84d185d73a..16ad3d8fab4d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -669,6 +669,10 @@ struct drm_connector *msm_dsi_manager_connector_init(u8 id)
connector->interlace_allowed = 0;
connector->doublescan_allowed = 0;
 
+   ret = drm_connector_init_panel_orientation_property(connector);
+   if (ret)
+   goto fail;
+
drm_connector_attach_encoder(connector, msm_dsi->encoder);
 
ret = msm_dsi_manager_panel_init(connector, id);
-- 
2.36.1.124.g0e6072fb45-goog

[PATCH v10 0/4] Separate panel orientation property creating and value setting

2022-05-31 Thread Hsin-Yi Wang 
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().

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(-)

-- 
2.36.1.124.g0e6072fb45-goog

[PATCH v9 3/4] drm/msm: init panel orientation property

2022-03-18 Thread Hsin-Yi Wang 
Init panel orientation property after connector is initialized. Let the
panel driver decides the orientation value later.

Signed-off-by: Hsin-Yi Wang 
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 0c1b7dde377c..b5dc86ebcab9 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -627,6 +627,10 @@ struct drm_connector *msm_dsi_manager_connector_init(u8 id)
connector->interlace_allowed = 0;
connector->doublescan_allowed = 0;
 
+   ret = drm_connector_init_panel_orientation_property(connector);
+   if (ret)
+   goto fail;
+
drm_connector_attach_encoder(connector, msm_dsi->encoder);
 
ret = msm_dsi_manager_panel_init(connector, id);
-- 
2.35.1.894.gb6a874cedc-goog

Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-03-18 Thread Hsin-Yi Wang 
On Fri, Feb 18, 2022 at 11:57 PM Harry Wentland  wrote:
>
> On 2022-02-18 07:12, Simon Ser wrote:
> > On Friday, February 18th, 2022 at 12:54, Hans de Goede 
> >  wrote:
> >
> >> On 2/18/22 12:39, Simon Ser wrote:
> >>> On Friday, February 18th, 2022 at 11:38, Hans de Goede 
> >>>  wrote:
> >>>
>  What I'm reading in the above is that it is being considered to allow
>  changing the panel-orientation value after the connector has been made
>  available to userspace; and let userspace know about this through a 
>  uevent.
> 
>  I believe that this is a bad idea, it is important to keep in mind here
>  what userspace (e.g. plymouth) uses this prorty for. This property is
>  used to rotate the image being rendered / shown on the framebuffer to
>  adjust for the panel orientation.
> 
>  So now lets assume we apply the correct upside-down orientation later
>  on a device with an upside-down mounted LCD panel. Then on boot the
>  following could happen:
> 
>  1. amdgpu exports a connector for the LCD panel to userspace without
>  setting panel-orient=upside-down
>  2. plymouth sees this and renders its splash normally, but since the
>  panel is upside-down it will now actually show upside-down
> >>>
> >>> At this point amdgpu hasn't probed the connector yet. So the connector
> >>> will be marked as disconnected, and plymouth shouldn't render anything.
> >>
> >> If before the initial probe of the connector there is a /dev/dri/card0
> >> which plymouth can access, then plymouth may at this point decide
> >> to disable any seemingly unused crtcs, which will make the screen go 
> >> black...
> >>
> >> I'm not sure if plymouth will actually do this, but AFAICT this would
> >> not be invalid behavior for a userspace kms consumer to do and I
> >> believe it is likely that mutter will disable unused crtcs.
> >>
> >> IMHO it is just a bad idea to register /dev/dri/card0 with userspace
> >> before the initial connector probe is done. Nothing good can come
> >> of that.
> >>
> >> If all the exposed connectors initially are going to show up as
> >> disconnected anyways what is the value in registering /dev/dri/card0
> >> with userspace early ?
> >
> > OK. I'm still unsure how I feel about this, but I think I agree with
> > you. That said, the amdgpu architecture is quite involved with multiple
> > abstraction levels, so I don't think I'm equipped to write a patch to
> > fix this...
> >
>
> amdgpu_dm's connector registration already triggers a detection. See the
> calls to dc_link_detect and amdgpu_dm_update_connector_after_detect in
> amdgpu_dm_initialize_drm_device.
>
> dc_link_detect is supposed to read the edid via
> dm_helpers_read_local_edid and amdgpu_dm_update_connector_after_detect
> will update the EDID on the connector via a
> drm_connector_update_edid_property call.
>
> This all happens at driver load.
>
> I don't know why you're seeing the embedded connector as disconnected
> unless the DP-MIPI bridge for some reason doesn't indicate that the panel
> is connected at driver load.
>
> Harry
>
> > cc Daniel Vetter: can you confirm probing all connectors is a good thing
> > to do on driver module load?
> >
>  I guess the initial modeline is inherited from the video-bios, but
>  what about the physical size? Note that you cannot just change the
>  physical size later either, that gets used to determine the hidpi
>  scaling factor in the bootsplash, and changing that after the initial
>  bootsplash dislay will also look ugly
> 
>  b) Why you need the edid for the panel-orientation property at all,
>  typically the edid prom is part of the panel and the panel does not
>  know that it is mounted e.g. upside down at all, that is a property
>  of the system as a whole not of the panel as a standalone unit so
>  in my experience getting panel-orient info is something which comes
>  from the firmware /video-bios not from edid ?
> >>>
> >>> This is an internal DRM thing. The orientation quirks logic uses the
> >>> mode size advertised by the EDID.
> >>
> >> The DMI based quirking does, yes. But e.g. the quirk code directly
> >> reading this from the Intel VBT does not rely on the mode.
> >>
> >> But if you are planning on using a DMI based quirk for the steamdeck
> >> then yes that needs the mode.
> >>
> >> Thee mode check is there for 2 reasons:
> >>
> >> 1. To avoid also applying the quirk to external displays, but
> >> I think that that is also solved in most drivers by only checking for
> >> a quirk at all on the eDP connector
> >>
> >> 2. Some laptop models ship with different panels in different badges
> >> some of these are portrait (so need a panel-orient) setting and others
> >> are landscape.
> >
> > That makes sense. So yeah the EDID mode based matching logic needs to
> > stay to accomodate for these cases.
> >
> >>> I agree that at least in the Steam
> >>> Deck case it may not make a lot of

[PATCH v9 4/4] arm64: dts: mt8183: Add panel rotation

2022-03-18 Thread Hsin-Yi Wang 
krane, kakadu, and kodama boards have a default panel rotation.

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Enric Balletbo i Serra 
Tested-by: Enric Balletbo i Serra 
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index 0f9480f91261..c7c6be106e2e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -276,6 +276,7 @@ panel: panel@0 {
avee-supply = <_lcd>;
pp1800-supply = <_lcd>;
backlight = <_lcd0>;
+   rotation = <270>;
port {
panel_in: endpoint {
remote-endpoint = <_out>;
-- 
2.35.1.894.gb6a874cedc-goog

[PATCH v9 2/4] drm/mediatek: init panel orientation property

2022-03-18 Thread Hsin-Yi Wang 
Init panel orientation property after connector is initialized. Let the
panel driver decides the orientation value later.

Signed-off-by: Hsin-Yi Wang 
Acked-by: Chun-Kuang Hu 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index ccb0511b9cd5..0376b33e9651 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -810,6 +810,13 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, 
struct mtk_dsi *dsi)
ret = PTR_ERR(dsi->connector);
goto err_cleanup_encoder;
}
+
+   ret = drm_connector_init_panel_orientation_property(dsi->connector);
+   if (ret) {
+   DRM_ERROR("Unable to init panel orientation\n");
+   goto err_cleanup_encoder;
+   }
+
drm_connector_attach_encoder(dsi->connector, >encoder);
 
return 0;
-- 
2.35.1.894.gb6a874cedc-goog

[PATCH v9 0/4] Separate panel orientation property creating and value setting

2022-03-18 Thread Hsin-Yi Wang 
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().

After this series, drm_connector_set_panel_orientation() works like
before, so it won't affect other drm drivers. 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(-)

-- 
2.35.1.894.gb6a874cedc-goog

[PATCH v9 1/4] gpu: drm: separate panel orientation property creating and value setting

2022-03-18 Thread Hsin-Yi Wang 
drm_dev_register() sets connector->registration_state to
DRM_CONNECTOR_REGISTERED and dev->registered to true. If
drm_connector_set_panel_orientation() is first called after
drm_dev_register(), it will fail several checks and results in following
warning.

Add a function to create panel orientation property and set default value
to UNKNOWN, so drivers can call this function to init the property earlier
, and let the panel set the real value later.

[4.480976] [ cut here ]
[4.485603] WARNING: CPU: 5 PID: 369 at drivers/gpu/drm/drm_mode_object.c:45 
__drm_mode_object_add+0xb4/0xbc

[4.609772] Call trace:
[4.612208]  __drm_mode_object_add+0xb4/0xbc
[4.616466]  drm_mode_object_add+0x20/0x2c
[4.620552]  drm_property_create+0xdc/0x174
[4.624723]  drm_property_create_enum+0x34/0x98
[4.629241]  drm_connector_set_panel_orientation+0x64/0xa0
[4.634716]  boe_panel_get_modes+0x88/0xd8
[4.638802]  drm_panel_get_modes+0x2c/0x48
[4.642887]  panel_bridge_get_modes+0x1c/0x28
[4.647233]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.652273]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.658266]  drm_mode_getconnector+0x1b4/0x45c
[4.662699]  drm_ioctl_kernel+0xac/0x128
[4.11]  drm_ioctl+0x268/0x410
[4.670002]  drm_compat_ioctl+0xdc/0xf0
[4.673829]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.678436]  el0_svc_common+0xf4/0x1c0
[4.682174]  do_el0_svc_compat+0x28/0x3c
[4.686088]  el0_svc_compat+0x10/0x1c
[4.689738]  el0_sync_compat_handler+0xa8/0xcc
[4.694171]  el0_sync_compat+0x178/0x180
[4.698082] ---[ end trace b4f2db9d9c88610b ]---
[4.702721] [ cut here ]
[4.707329] WARNING: CPU: 5 PID: 369 at 
drivers/gpu/drm/drm_mode_object.c:243 drm_object_attach_property+0x48/0xb8

[4.833830] Call trace:
[4.836266]  drm_object_attach_property+0x48/0xb8
[4.840958]  drm_connector_set_panel_orientation+0x84/0xa0
[4.846432]  boe_panel_get_modes+0x88/0xd8
[4.850516]  drm_panel_get_modes+0x2c/0x48
[4.854600]  panel_bridge_get_modes+0x1c/0x28
[4.858946]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.863984]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.869978]  drm_mode_getconnector+0x1b4/0x45c
[4.874410]  drm_ioctl_kernel+0xac/0x128
[4.878320]  drm_ioctl+0x268/0x410
[4.881711]  drm_compat_ioctl+0xdc/0xf0
[4.885536]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.890142]  el0_svc_common+0xf4/0x1c0
[4.893879]  do_el0_svc_compat+0x28/0x3c
[4.897791]  el0_svc_compat+0x10/0x1c
[4.901441]  el0_sync_compat_handler+0xa8/0xcc
[4.905873]  el0_sync_compat+0x178/0x180
[4.909783] ---[ end trace b4f2db9d9c88610c ]---

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/drm_connector.c | 58 +
 include/drm/drm_connector.h |  2 ++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 76a8c707c34b..149709e05622 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1252,7 +1252,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = 
{
  * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
  * coordinates, so if userspace rotates the picture to adjust for
  * the orientation it must also apply the same transformation to the
- * touchscreen input coordinates. This property is initialized by calling
+ * touchscreen input coordinates. This property value is set by calling
  * drm_connector_set_panel_orientation() or
  * drm_connector_set_panel_orientation_with_quirk()
  *
@@ -2344,8 +2344,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
  * @connector: connector for which to set the panel-orientation property.
  * @panel_orientation: drm_panel_orientation value to set
  *
- * This function sets the connector's panel_orientation and attaches
- * a "panel orientation" property to the connector.
+ * This function sets the connector's panel_orientation value. If the property
+ * doesn't exist, it will try to create one.
  *
  * Calling this function on a connector where the panel_orientation has
  * already been set is a no-op (e.g. the orientation has been overridden with
@@ -2377,18 +2377,13 @@ int drm_connector_set_panel_orientation(
 
prop = dev->mode_config.panel_orientation_property;
if (!prop) {
-   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
-   "panel orientation",
-   drm_panel_orientation_enum_list,
-   ARRAY_SIZE(drm_panel_orientation_enum_list));
-   if (!prop)
+   if (drm_connector_init_panel_orientation_property(connector) < 
0)
return -ENOMEM;
-
-   dev->mode_confi

Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-02-15 Thread Hsin-Yi Wang 
On Tue, Feb 15, 2022 at 8:04 PM Emil Velikov  wrote:
>
> Greetings everyone,
>
> Padron for joining in so late o/
>
> On Tue, 8 Feb 2022 at 08:42, Hsin-Yi Wang  wrote:
> >
> > drm_dev_register() sets connector->registration_state to
> > DRM_CONNECTOR_REGISTERED and dev->registered to true. If
> > drm_connector_set_panel_orientation() is first called after
> > drm_dev_register(), it will fail several checks and results in following
> > warning.
> >
> > Add a function to create panel orientation property and set default value
> > to UNKNOWN, so drivers can call this function to init the property earlier
> > , and let the panel set the real value later.
> >
>
> The warning illustrates a genuine race condition, where userspace will
> read the old/invalid property value/state. So this patch masks away
> the WARNING without addressing the actual issue.
> Instead can we fix the respective drivers, so that no properties are
> created after drm_dev_register()?
>
1. How about the proposal in previous version:
v7 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220208073714.1540390-1-hsi...@chromium.org/
we separate property creation
(drm_connector_init_panel_orientation_property) and value setting
(drm_connector_set_panel_orientation). This is also similar to some of
other optional properties are created, eg. vrr_capable.

And drm drivers have to make sure that if they want to use this
property, they have to create it before drm_dev_register(). For
example, in the 2nd patch, mtk_drm sets the property before calling
drm_dev_register().

2. I'm not sure how to handle the case that if user space tries to
read the property before the proper value is set. Currently drm
creates this property and the panels[1] will set the correct value
parsed from DT. If userspace calls before the panel sets the correct
value, it will get unknown (similar to the illustration you mentioned
below). Do you think that the drm should be responsible for parsing
the value if the panel provides it? In this way it's guaranteed that
the value is set when the property is created.

[1] 
https://elixir.bootlin.com/linux/latest/A/ident/drm_connector_set_panel_orientation

> Longer version:
> As we look into drm_dev_register() it's in charge of creating the
> dev/sysfs nodes (et al). Note that connectors cannot disappear at
> runtime.
> For panel orientation, we are creating an immutable connector
> properly, meaning that as soon as drm_dev_register() is called we must
> ensure that the property is available (if applicable) and set to the
> correct value.
>
> For illustration, consider the following scenario:
>  - DRM modules are loaded late - are not built-in and not part of
> initrd (or there's no initrd)
>  - kernel boots
>  - plymouth/similar user-space component kicks in before the
> driver/module is loaded
>  - module gets loaded, drm_dev_register() kicks in populating /dev/dri/card0
>  - plymouth opens the dev node and reads DRM_MODE_PANEL_ORIENTATION_UNKNOWN
>  - module updates the orientation property
>
> Thanks
> Emil

Re: [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-02-15 Thread Hsin-Yi Wang 
On Tue, Feb 15, 2022 at 12:03 PM Gabriel Krisman Bertazi
 wrote:
>
> Hsin-Yi Wang  writes:
>
> > On Tue, Feb 15, 2022 at 9:17 AM Gabriel Krisman Bertazi
> >  wrote:
> >>
> >> Hsin-Yi Wang  writes:
> >>
> >> > drm_dev_register() sets connector->registration_state to
> >> > DRM_CONNECTOR_REGISTERED and dev->registered to true. If
> >> > drm_connector_set_panel_orientation() is first called after
> >> > drm_dev_register(), it will fail several checks and results in following
> >> > warning.
> >>
> >> Hi,
> >>
> >> I stumbled upon this when investigating the same WARN_ON on amdgpu.
> >> Thanks for the patch :)
> >>
> >> > diff --git a/drivers/gpu/drm/drm_connector.c 
> >> > b/drivers/gpu/drm/drm_connector.c
> >> > index a50c82bc2b2fec..572ead7ac10690 100644
> >> > --- a/drivers/gpu/drm/drm_connector.c
> >> > +++ b/drivers/gpu/drm/drm_connector.c
> >> > @@ -1252,7 +1252,7 @@ static const struct drm_prop_enum_list 
> >> > dp_colorspaces[] = {
> >> >   *   INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
> >> >   *   coordinates, so if userspace rotates the picture to adjust for
> >> >   *   the orientation it must also apply the same transformation to the
> >> > - *   touchscreen input coordinates. This property is initialized by 
> >> > calling
> >> > + *   touchscreen input coordinates. This property value is set by 
> >> > calling
> >> >   *   drm_connector_set_panel_orientation() or
> >> >   *   drm_connector_set_panel_orientation_with_quirk()
> >> >   *
> >> > @@ -2341,8 +2341,8 @@ 
> >> > EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
> >> >   * @connector: connector for which to set the panel-orientation 
> >> > property.
> >> >   * @panel_orientation: drm_panel_orientation value to set
> >> >   *
> >> > - * This function sets the connector's panel_orientation and attaches
> >> > - * a "panel orientation" property to the connector.
> >> > + * This function sets the connector's panel_orientation value. If the 
> >> > property
> >> > + * doesn't exist, it will try to create one.
> >> >   *
> >> >   * Calling this function on a connector where the panel_orientation has
> >> >   * already been set is a no-op (e.g. the orientation has been 
> >> > overridden with
> >> > @@ -2373,19 +2373,12 @@ int drm_connector_set_panel_orientation(
> >> >   info->panel_orientation = panel_orientation;
> >> >
> >> >   prop = dev->mode_config.panel_orientation_property;
> >> > - if (!prop) {
> >> > - prop = drm_property_create_enum(dev, 
> >> > DRM_MODE_PROP_IMMUTABLE,
> >> > - "panel orientation",
> >> > - drm_panel_orientation_enum_list,
> >> > - 
> >> > ARRAY_SIZE(drm_panel_orientation_enum_list));
> >> > - if (!prop)
> >> > - return -ENOMEM;
> >> > -
> >> > - dev->mode_config.panel_orientation_property = prop;
> >> > - }
> >> > + if (!prop &&
> >> > + drm_connector_init_panel_orientation_property(connector) < 0)
> >> > + return -ENOMEM;
> >> >
> >>
> >> In the case where the property has not been created beforehand, you
> >> forgot to reinitialize prop here, after calling
> >> drm_connector_init_panel_orientation_property().  This means
> > hi Gabriel,
> >
> > drm_connector_init_panel_orientation_property() will create prop if
> > it's null. If prop fails to be created there, it will return -ENOMEM.
>
> Yes.  But *after the property is successfully created*, the prop variable is 
> still
> NULL.  And then you call the following, using prop, which is still NULL:
>
> >> > + drm_object_property_set_value(>base, prop,
> >> > +   info->panel_orientation);
>
> This will do property->dev right on the first line of code, and dereference 
> the
> null prop pointer.

Ah, right. Sorry that I totally missed this.
I'll fix it in the next version if the idea of this patch is accepted.
There might be another preferred solution for this.

>
> You must do
>
>   prop = dev->mode_config.panel_orientation_property;
>
> again after drm_connector_init_panel_orientation_property successfully
> returns, or call drm_object_property_set_value using
> dev->mode_config.panel_orientation_property directly:
>
>   drm_object_property_set_value(>base,
> dev->mode_config.panel_orientation_property
> info->panel_orientation);
>
> --
> Gabriel Krisman Bertazi

Re: [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-02-15 Thread Hsin-Yi Wang 
On Tue, Feb 15, 2022 at 9:17 AM Gabriel Krisman Bertazi
 wrote:
>
> Hsin-Yi Wang  writes:
>
> > drm_dev_register() sets connector->registration_state to
> > DRM_CONNECTOR_REGISTERED and dev->registered to true. If
> > drm_connector_set_panel_orientation() is first called after
> > drm_dev_register(), it will fail several checks and results in following
> > warning.
>
> Hi,
>
> I stumbled upon this when investigating the same WARN_ON on amdgpu.
> Thanks for the patch :)
>
> > diff --git a/drivers/gpu/drm/drm_connector.c 
> > b/drivers/gpu/drm/drm_connector.c
> > index a50c82bc2b2fec..572ead7ac10690 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1252,7 +1252,7 @@ static const struct drm_prop_enum_list 
> > dp_colorspaces[] = {
> >   *   INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
> >   *   coordinates, so if userspace rotates the picture to adjust for
> >   *   the orientation it must also apply the same transformation to the
> > - *   touchscreen input coordinates. This property is initialized by calling
> > + *   touchscreen input coordinates. This property value is set by calling
> >   *   drm_connector_set_panel_orientation() or
> >   *   drm_connector_set_panel_orientation_with_quirk()
> >   *
> > @@ -2341,8 +2341,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
> >   * @connector: connector for which to set the panel-orientation property.
> >   * @panel_orientation: drm_panel_orientation value to set
> >   *
> > - * This function sets the connector's panel_orientation and attaches
> > - * a "panel orientation" property to the connector.
> > + * This function sets the connector's panel_orientation value. If the 
> > property
> > + * doesn't exist, it will try to create one.
> >   *
> >   * Calling this function on a connector where the panel_orientation has
> >   * already been set is a no-op (e.g. the orientation has been overridden 
> > with
> > @@ -2373,19 +2373,12 @@ int drm_connector_set_panel_orientation(
> >   info->panel_orientation = panel_orientation;
> >
> >   prop = dev->mode_config.panel_orientation_property;
> > - if (!prop) {
> > - prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> > - "panel orientation",
> > - drm_panel_orientation_enum_list,
> > - ARRAY_SIZE(drm_panel_orientation_enum_list));
> > - if (!prop)
> > - return -ENOMEM;
> > -
> > - dev->mode_config.panel_orientation_property = prop;
> > - }
> > + if (!prop &&
> > + drm_connector_init_panel_orientation_property(connector) < 0)
> > + return -ENOMEM;
> >
>
> In the case where the property has not been created beforehand, you
> forgot to reinitialize prop here, after calling
> drm_connector_init_panel_orientation_property().  This means
hi Gabriel,

drm_connector_init_panel_orientation_property() will create prop if
it's null. If prop fails to be created there, it will return -ENOMEM.

> drm_object_property_set_value() will be called with a NULL second argument
> and Oops the kernel.
>
>
> > - drm_object_attach_property(>base, prop,
> > -info->panel_orientation);
> > + drm_object_property_set_value(>base, prop,
> > +   info->panel_orientation);
>
>
> --
> Gabriel Krisman Bertazi

[PATCH v8 2/3] drm/mediatek: init panel orientation property

2022-02-08 Thread Hsin-Yi Wang 
Init panel orientation property after connector is initialized. Let the
panel driver decides the orientation value later.

Signed-off-by: Hsin-Yi Wang 
Acked-by: Chun-Kuang Hu 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5d90d2eb001935..491bf5b0a2b984 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -965,6 +965,13 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, 
struct mtk_dsi *dsi)
ret = PTR_ERR(dsi->connector);
goto err_cleanup_encoder;
}
+
+   ret = drm_connector_init_panel_orientation_property(dsi->connector);
+   if (ret) {
+   DRM_ERROR("Unable to init panel orientation\n");
+   goto err_cleanup_encoder;
+   }
+
drm_connector_attach_encoder(dsi->connector, >encoder);
 
return 0;
-- 
2.35.0.263.gb82422642f-goog

[PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-02-08 Thread Hsin-Yi Wang 
drm_dev_register() sets connector->registration_state to
DRM_CONNECTOR_REGISTERED and dev->registered to true. If
drm_connector_set_panel_orientation() is first called after
drm_dev_register(), it will fail several checks and results in following
warning.

Add a function to create panel orientation property and set default value
to UNKNOWN, so drivers can call this function to init the property earlier
, and let the panel set the real value later.

[4.480976] [ cut here ]
[4.485603] WARNING: CPU: 5 PID: 369 at drivers/gpu/drm/drm_mode_object.c:45 
__drm_mode_object_add+0xb4/0xbc

[4.609772] Call trace:
[4.612208]  __drm_mode_object_add+0xb4/0xbc
[4.616466]  drm_mode_object_add+0x20/0x2c
[4.620552]  drm_property_create+0xdc/0x174
[4.624723]  drm_property_create_enum+0x34/0x98
[4.629241]  drm_connector_set_panel_orientation+0x64/0xa0
[4.634716]  boe_panel_get_modes+0x88/0xd8
[4.638802]  drm_panel_get_modes+0x2c/0x48
[4.642887]  panel_bridge_get_modes+0x1c/0x28
[4.647233]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.652273]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.658266]  drm_mode_getconnector+0x1b4/0x45c
[4.662699]  drm_ioctl_kernel+0xac/0x128
[4.11]  drm_ioctl+0x268/0x410
[4.670002]  drm_compat_ioctl+0xdc/0xf0
[4.673829]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.678436]  el0_svc_common+0xf4/0x1c0
[4.682174]  do_el0_svc_compat+0x28/0x3c
[4.686088]  el0_svc_compat+0x10/0x1c
[4.689738]  el0_sync_compat_handler+0xa8/0xcc
[4.694171]  el0_sync_compat+0x178/0x180
[4.698082] ---[ end trace b4f2db9d9c88610b ]---
[4.702721] [ cut here ]
[4.707329] WARNING: CPU: 5 PID: 369 at 
drivers/gpu/drm/drm_mode_object.c:243 drm_object_attach_property+0x48/0xb8

[4.833830] Call trace:
[4.836266]  drm_object_attach_property+0x48/0xb8
[4.840958]  drm_connector_set_panel_orientation+0x84/0xa0
[4.846432]  boe_panel_get_modes+0x88/0xd8
[4.850516]  drm_panel_get_modes+0x2c/0x48
[4.854600]  panel_bridge_get_modes+0x1c/0x28
[4.858946]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.863984]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.869978]  drm_mode_getconnector+0x1b4/0x45c
[4.874410]  drm_ioctl_kernel+0xac/0x128
[4.878320]  drm_ioctl+0x268/0x410
[4.881711]  drm_compat_ioctl+0xdc/0xf0
[4.885536]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.890142]  el0_svc_common+0xf4/0x1c0
[4.893879]  do_el0_svc_compat+0x28/0x3c
[4.897791]  el0_svc_compat+0x10/0x1c
[4.901441]  el0_sync_compat_handler+0xa8/0xcc
[4.905873]  el0_sync_compat+0x178/0x180
[4.909783] ---[ end trace b4f2db9d9c88610c ]---

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Sean Paul 
---
v7->v8:
- check if the prop is created to avoid leak issue when called multiple
  times.
- attempt to create prop in drm_connector_set_panel_orientation if prop
  is not created before, so driver don't need to call
  drm_connector_init_panel_orientation_property if they don't need to
  set the property earlier.
---
 drivers/gpu/drm/drm_connector.c | 62 -
 include/drm/drm_connector.h |  2 ++
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a50c82bc2b2fec..572ead7ac10690 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1252,7 +1252,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = 
{
  * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
  * coordinates, so if userspace rotates the picture to adjust for
  * the orientation it must also apply the same transformation to the
- * touchscreen input coordinates. This property is initialized by calling
+ * touchscreen input coordinates. This property value is set by calling
  * drm_connector_set_panel_orientation() or
  * drm_connector_set_panel_orientation_with_quirk()
  *
@@ -2341,8 +2341,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
  * @connector: connector for which to set the panel-orientation property.
  * @panel_orientation: drm_panel_orientation value to set
  *
- * This function sets the connector's panel_orientation and attaches
- * a "panel orientation" property to the connector.
+ * This function sets the connector's panel_orientation value. If the property
+ * doesn't exist, it will try to create one.
  *
  * Calling this function on a connector where the panel_orientation has
  * already been set is a no-op (e.g. the orientation has been overridden with
@@ -2373,19 +2373,12 @@ int drm_connector_set_panel_orientation(
info->panel_orientation = panel_orientation;
 
prop = dev->mode_config.panel_orientation_property;
-   if (!prop) {
-   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
-

[PATCH v8 3/3] arm64: dts: mt8183: Add panel rotation

2022-02-08 Thread Hsin-Yi Wang 
krane, kakadu, and kodama boards have a default panel rotation.

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Enric Balletbo i Serra 
Tested-by: Enric Balletbo i Serra 
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index b42d81d26d7211..d29d4378170971 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -276,6 +276,7 @@ panel: panel@0 {
avee-supply = <_lcd>;
pp1800-supply = <_lcd>;
backlight = <_lcd0>;
+   rotation = <270>;
port {
panel_in: endpoint {
remote-endpoint = <_out>;
-- 
2.35.0.263.gb82422642f-goog

[PATCH v7 3/3] arm64: dts: mt8183: Add panel rotation

2022-02-08 Thread Hsin-Yi Wang 
krane, kakadu, and kodama boards have a default panel rotation.

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Enric Balletbo i Serra 
Tested-by: Enric Balletbo i Serra 
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index b42d81d26d7211..d29d4378170971 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -276,6 +276,7 @@ panel: panel@0 {
avee-supply = <_lcd>;
pp1800-supply = <_lcd>;
backlight = <_lcd0>;
+   rotation = <270>;
port {
panel_in: endpoint {
remote-endpoint = <_out>;
-- 
2.35.0.263.gb82422642f-goog

Re: [Intel-gfx] [PATCH v7 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-02-08 Thread Hsin-Yi Wang 
On Tue, Feb 8, 2022 at 3:52 PM Ville Syrjälä
 wrote:
>
> On Tue, Feb 08, 2022 at 03:37:12PM +0800, Hsin-Yi Wang wrote:
> > +int drm_connector_init_panel_orientation_property(
> > + struct drm_connector *connector)
> > +{
> > + struct drm_device *dev = connector->dev;
> > + struct drm_property *prop;
> > +
> > + prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> > + "panel orientation",
> > + drm_panel_orientation_enum_list,
> > + ARRAY_SIZE(drm_panel_orientation_enum_list));
> > + if (!prop)
> > + return -ENOMEM;
> > +
> > + dev->mode_config.panel_orientation_property = prop;
>
> Leak when called multiple times. I guess you could just put
> this into drm_connector_create_standard_properties() instead
> and avoid that issue entirely.
>
I'll add a check for dev->mode_config.panel_orientation_property to
avoid the leak issue if called multiple times.
If we add in drm_connector_create_standard_properties(), we still need
another function to attach the property earlier for bridge/connectors
that require this property, since not all bridge/connectors need this
property.

> --
> Ville Syrjälä
> Intel

[PATCH v7 2/3] drm/mediatek: init panel orientation property

2022-02-08 Thread Hsin-Yi Wang 
Init panel orientation property after connector is initialized. Let the
panel driver decides the orientation value later.

Signed-off-by: Hsin-Yi Wang 
Acked-by: Chun-Kuang Hu 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5d90d2eb001935..491bf5b0a2b984 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -965,6 +965,13 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, 
struct mtk_dsi *dsi)
ret = PTR_ERR(dsi->connector);
goto err_cleanup_encoder;
}
+
+   ret = drm_connector_init_panel_orientation_property(dsi->connector);
+   if (ret) {
+   DRM_ERROR("Unable to init panel orientation\n");
+   goto err_cleanup_encoder;
+   }
+
drm_connector_attach_encoder(dsi->connector, >encoder);
 
return 0;
-- 
2.35.0.263.gb82422642f-goog

[PATCH v7 1/3] gpu: drm: separate panel orientation property creating and value setting

2022-02-08 Thread Hsin-Yi Wang 
drm_dev_register() sets connector->registration_state to
DRM_CONNECTOR_REGISTERED and dev->registered to true. If
drm_connector_set_panel_orientation() is first called after
drm_dev_register(), it will fail several checks and results in following
warning.

Add a function to create panel orientation property and set default value
to UNKNOWN, so drivers can call this function to init the property earlier
, and let the panel set the real value later.

[4.480976] [ cut here ]
[4.485603] WARNING: CPU: 5 PID: 369 at drivers/gpu/drm/drm_mode_object.c:45 
__drm_mode_object_add+0xb4/0xbc

[4.609772] Call trace:
[4.612208]  __drm_mode_object_add+0xb4/0xbc
[4.616466]  drm_mode_object_add+0x20/0x2c
[4.620552]  drm_property_create+0xdc/0x174
[4.624723]  drm_property_create_enum+0x34/0x98
[4.629241]  drm_connector_set_panel_orientation+0x64/0xa0
[4.634716]  boe_panel_get_modes+0x88/0xd8
[4.638802]  drm_panel_get_modes+0x2c/0x48
[4.642887]  panel_bridge_get_modes+0x1c/0x28
[4.647233]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.652273]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.658266]  drm_mode_getconnector+0x1b4/0x45c
[4.662699]  drm_ioctl_kernel+0xac/0x128
[4.11]  drm_ioctl+0x268/0x410
[4.670002]  drm_compat_ioctl+0xdc/0xf0
[4.673829]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.678436]  el0_svc_common+0xf4/0x1c0
[4.682174]  do_el0_svc_compat+0x28/0x3c
[4.686088]  el0_svc_compat+0x10/0x1c
[4.689738]  el0_sync_compat_handler+0xa8/0xcc
[4.694171]  el0_sync_compat+0x178/0x180
[4.698082] ---[ end trace b4f2db9d9c88610b ]---
[4.702721] [ cut here ]
[4.707329] WARNING: CPU: 5 PID: 369 at 
drivers/gpu/drm/drm_mode_object.c:243 drm_object_attach_property+0x48/0xb8

[4.833830] Call trace:
[4.836266]  drm_object_attach_property+0x48/0xb8
[4.840958]  drm_connector_set_panel_orientation+0x84/0xa0
[4.846432]  boe_panel_get_modes+0x88/0xd8
[4.850516]  drm_panel_get_modes+0x2c/0x48
[4.854600]  panel_bridge_get_modes+0x1c/0x28
[4.858946]  drm_bridge_connector_get_modes+0xa0/0xd4
[4.863984]  drm_helper_probe_single_connector_modes+0x218/0x700
[4.869978]  drm_mode_getconnector+0x1b4/0x45c
[4.874410]  drm_ioctl_kernel+0xac/0x128
[4.878320]  drm_ioctl+0x268/0x410
[4.881711]  drm_compat_ioctl+0xdc/0xf0
[4.885536]  __arm64_compat_sys_ioctl+0xc8/0x100
[4.890142]  el0_svc_common+0xf4/0x1c0
[4.893879]  do_el0_svc_compat+0x28/0x3c
[4.897791]  el0_svc_compat+0x10/0x1c
[4.901441]  el0_sync_compat_handler+0xa8/0xcc
[4.905873]  el0_sync_compat+0x178/0x180
[4.909783] ---[ end trace b4f2db9d9c88610c ]---

Signed-off-by: Hsin-Yi Wang 
Reviewed-by: Sean Paul 
---
v6 -> v7:
- Rebase to latest drm-misc.
- Add function for amdgpu_dm.
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  1 +
 drivers/gpu/drm/drm_connector.c   | 58 ++-
 drivers/gpu/drm/i915/display/icl_dsi.c|  1 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  1 +
 drivers/gpu/drm/i915/display/vlv_dsi.c|  1 +
 include/drm/drm_connector.h   |  2 +
 6 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e8a994559b6580..3eb0be187292ff 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8105,6 +8105,7 @@ static void amdgpu_set_panel_orientation(struct 
drm_connector *connector)
if (native_mode->hdisplay == 0 || native_mode->vdisplay == 0)
return;
 
+   drm_connector_init_panel_orientation_property(connector);
drm_connector_set_panel_orientation_with_quirk(connector,
   
DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
   native_mode->hdisplay,
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a50c82bc2b2fec..041e496c8f15a9 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1252,7 +1252,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = 
{
  * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
  * coordinates, so if userspace rotates the picture to adjust for
  * the orientation it must also apply the same transformation to the
- * touchscreen input coordinates. This property is initialized by calling
+ * touchscreen input coordinates. This property value is set by calling
  * drm_connector_set_panel_orientation() or
  * drm_connector_set_panel_orientation_with_quirk()
  *
@@ -2341,8 +2341,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
  * @connector: connector for which to set the panel-orientation property.
  *

Re: [PATCH] Revert "i2c: core: support bus regulator controlling in adapter"

2022-02-04 Thread Hsin-Yi Wang 
On Sun, Jan 16, 2022 at 2:44 AM Tareque Md.Hanif
 wrote:
>
> Hi Hsin-Yi,
>
> The issue still exists. I reverted a19f75de73c220b4496d2aefb7a605dd032f7c01 
> (the commit that reverted 5a7b95fb993ec399c8a685552aa6a8fc995c40bd) and 
> manually applied the patch (tags/v5.16). journalctl attached.

hi Tareque,

Can you apply the same setting[1] again and with this patch to see if
the issue is still there?
https://github.com/torvalds/linux/commit/6dc8265f9803ccb7e5da804e01601f0c14f270e0

[1] reverted a19f75de73c220b4496d2aefb7a605dd032f7c01 (the commit that
reverted 5a7b95fb993ec399c8a685552aa6a8fc995c40bd) and manually
applied the patch (tags/v5.16)

Thanks
>
> Regards,
>
> Tareque
>
> On Saturday, January 15, 2022, 11:27:07 PM GMT+6, Hsin-Yi Wang 
>  wrote:
>
>
> hi Tareque,
>
>
> On Fri, Jan 14, 2022 at 6:09 PM Tareque Md Hanif
>  wrote:
> >
> > Hi Hsin-Yi,
> >
> > On 1/12/22 16:58, Hsin-Yi Wang wrote:
> >
> > Can you help provide logs if we apply
> > 5a7b95fb993ec399c8a685552aa6a8fc995c40bd but revert
> > 8d35a2596164c1c9d34d4656fd42b445cd1e247f?
> >
> > Issue still exists. journalctl log attached in revert_8d.txt
> >
> >
> > > after apply 5a7b95fb993ec399c8a685552aa6a8fc995c40bd
> > > 1. delete SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late,
> > > i2c_resume_early) and function i2c_suspend_late() and
> > > i2c_resume_early().
> >
> > No issues. journalctl log attached in test1.txt
> >
> >
> > > 2. delete SET_RUNTIME_PM_OPS(i2c_runtime_suspend, i2c_runtime_resume,
> > > NULL) and function i2c_runtime_suspend() and i2c_runtime_resume().
> >
> > Issue exists. journalctl log attached in test2.txt
>
>
> Thanks for the testing.
> Can you help us test if applying the following patch on top of
> 5a7b95fb993ec399c8a685552aa6a8fc995c40bd works? Thanks
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 9eb4009cb250..6b046012aa08 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -484,7 +484,7 @@ static int i2c_resume_early(struct device *dev)
> struct i2c_client *client = i2c_verify_client(dev);
> int err;
>
> -  if (!client)
> +  if (!client || dev_pm_skip_resume(dev))
> return 0;
>
> if (pm_runtime_status_suspended(>dev) &&
> @@ -502,7 +502,7 @@ static int i2c_suspend_late(struct device *dev)
> struct i2c_client *client = i2c_verify_client(dev);
> int err;
>
> -  if (!client)
> +  if (!client || dev_pm_skip_suspend(dev))
> return 0;
>
> err = pm_generic_suspend_late(>dev);
>

Re: [PATCH] Revert "i2c: core: support bus regulator controlling in adapter"

2022-01-17 Thread Hsin-Yi Wang 
hi Tareque,


On Fri, Jan 14, 2022 at 6:09 PM Tareque Md Hanif
 wrote:
>
> Hi Hsin-Yi,
>
> On 1/12/22 16:58, Hsin-Yi Wang wrote:
>
> Can you help provide logs if we apply
> 5a7b95fb993ec399c8a685552aa6a8fc995c40bd but revert
> 8d35a2596164c1c9d34d4656fd42b445cd1e247f?
>
> Issue still exists. journalctl log attached in revert_8d.txt
>
>
> > after apply 5a7b95fb993ec399c8a685552aa6a8fc995c40bd
> > 1. delete SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late,
> > i2c_resume_early) and function i2c_suspend_late() and
> > i2c_resume_early().
>
> No issues. journalctl log attached in test1.txt
>
>
> > 2. delete SET_RUNTIME_PM_OPS(i2c_runtime_suspend, i2c_runtime_resume,
> > NULL) and function i2c_runtime_suspend() and i2c_runtime_resume().
>
> Issue exists. journalctl log attached in test2.txt

Thanks for the testing.
Can you help us test if applying the following patch on top of
5a7b95fb993ec399c8a685552aa6a8fc995c40bd works? Thanks

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 9eb4009cb250..6b046012aa08 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -484,7 +484,7 @@ static int i2c_resume_early(struct device *dev)
struct i2c_client *client = i2c_verify_client(dev);
int err;

-   if (!client)
+   if (!client || dev_pm_skip_resume(dev))
return 0;

if (pm_runtime_status_suspended(>dev) &&
@@ -502,7 +502,7 @@ static int i2c_suspend_late(struct device *dev)
struct i2c_client *client = i2c_verify_client(dev);
int err;

-   if (!client)
+   if (!client || dev_pm_skip_suspend(dev))
return 0;

err = pm_generic_suspend_late(>dev);

Re: [PATCH] Revert "i2c: core: support bus regulator controlling in adapter"

2022-01-12 Thread Hsin-Yi Wang 
hi Konstantin and Tareque,

Can you help provide logs if we apply
5a7b95fb993ec399c8a685552aa6a8fc995c40bd but revert
8d35a2596164c1c9d34d4656fd42b445cd1e247f?

Thanks

On Wed, Jan 12, 2022 at 6:02 PM Tareque Md Hanif
 wrote:
>
>
> On 1/12/22 15:51, Wolfram Sang wrote:
> > would the reporters of the
> > regression be available for further testing?
> Sure. I am available.

Re: [PATCH] Revert "i2c: core: support bus regulator controlling in adapter"

2022-01-12 Thread Hsin-Yi Wang 
On Wed, Jan 12, 2022 at 6:58 PM Hsin-Yi Wang  wrote:
>
> hi Konstantin and Tareque,
>
> Can you help provide logs if we apply
> 5a7b95fb993ec399c8a685552aa6a8fc995c40bd but revert
> 8d35a2596164c1c9d34d4656fd42b445cd1e247f?
>
Another thing might be helpful to test with:

after apply 5a7b95fb993ec399c8a685552aa6a8fc995c40bd
1. delete SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late,
i2c_resume_early) and function i2c_suspend_late() and
i2c_resume_early().
2. delete SET_RUNTIME_PM_OPS(i2c_runtime_suspend, i2c_runtime_resume,
NULL) and function i2c_runtime_suspend() and i2c_runtime_resume().

Does it still fail if we do 1 or 2?

Sorry that we don't have a platform with intel CPU and amd GPU
combination to test with.


> Thanks
>
> On Wed, Jan 12, 2022 at 6:02 PM Tareque Md Hanif
>  wrote:
> >
> >
> > On 1/12/22 15:51, Wolfram Sang wrote:
> > > would the reporters of the
> > > regression be available for further testing?
> > Sure. I am available.