Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 4:20 PM Rob Clark  wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  
> > > wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > > The best practice to avoid those issues is to register its functions 
> > > > > only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > > we should
> > > > > to play nice with the other components that are waiting for us, so in 
> > > > > our case
> > > > > that would mean moving the DSI device registration to the bridge 
> > > > > probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, 
> > > > > kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those 
> > > > > changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > > (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > > important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >
> > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop 
> > > > indefinitely:
> > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > regulator
> > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > regulator
> > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > regulator
> > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > > regulator
> > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > regulator
> > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > regulator
> > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

Not a dsi panel, but for what it's worth, I did just test the series
with the db845c w/ HDMI using the lt9611 bridge.
So at least that is looking ok.

thanks
-john


Re: [Freedreno] [PATCH v2 00/22] drm/msm/dpu: switch dpu_plane to be virtual

2021-09-29 Thread abhinavk

Hi Dmitry

On 2021-07-04 18:20, Dmitry Baryshkov wrote:

As discussed on IRC, change dpu_plane implementation to be virtual:
register unified planes and select backing SSPP block at runtime.

Use msm.dpu_use_virtual_planes=1 to enable usage of virtual planes
rather than statically allocated SSPPs at the plane registration.

Patches 1-9 move state variables from struct dpu_plane onto the stack
allocation. State should not be a part of struct dpu_plane anyway.

Patches 10-18 make additional changes to plane code, reworking check,
debugfs, dropping old multirec support, which results in patch 19 
adding

support for virtual planes per se.

Patches 20-22 demonstrate my main goal behind reworking dpu_plane
support. They change dpu_plane to automatically use one of SSPP block
features - multirec, an ability to display two unscaled RGB rectangles
using single SSPP block. This allows us to double the amount of created
planes. If the user tries to enable more planes than actually supported
by the underlying SSPP blocks, atomic_check code would return an error.

As you can see, this patchset is not atomic, so different patches can 
go

separately.


I am half way through this series and have finished checking patches 
1-12
I am okay with patches 1-4, 6-12. Its a reasonable cleanup to make the 
dpu_plane struct lighter.
I need a little more time with the rest as I am comparing the downstream 
solution against yours.


As you mentioned, this patchset is not atomic, hence can you break it up 
like

-> cleanup of dpu_plane struct in one series
-> removal of current multirect and current src split which will include 
patch 5 as well


So that the first series can go through and it gives us a little more 
time to check the second

series.

Thanks

Abhinav



Changes since v1:
 - Add multirec implementation
 - Added msm.dpu_use_virtual_planes kernel parameter instead of using
   compile time switch
 - Changed code to always reallocate SSPPs in the CRTC atomic check to
   let the kernel pick up the best multirec config. This can be
   optimized later.
 - Rework RM SSPP API to always receive plane id
 - Removed scaler_cfg, pixel_ext and cdp_cfg from struct 
dpu_plane_state

 - Made _dpu_scaler_setup() call sspp's setup_scaler and setup_pe
 - Removed dpu_csc_cfg from dpu_plane

The following changes since commit 
e88bbc91849b2bf57683119c339e52916d34433f:


  Revert "drm/msm/mdp5: provide dynamic bandwidth management"
(2021-06-23 14:06:20 -0700)

are available in the Git repository at:

  https://git.linaro.org/people/dmitry.baryshkov/kernel.git 
dpu-multirec-2


for you to fetch changes up to 
19f6afd40097d4c826e56b8f4a8cbd807f7b61f6:


  drm/msm/dpu: add multirect support (2021-07-05 04:04:50 +0300)


Dmitry Baryshkov (22):
  drm/msm/dpu: move LUT levels out of QOS config
  drm/msm/dpu: remove pipe_qos_cfg from struct dpu_plane
  drm/msm/dpu: drop pipe_name from struct dpu_plane
  drm/msm/dpu: remove stage_cfg from struct dpu_crtc
  drm/msm/dpu: rip out master planes support
  drm/msm/dpu: move dpu_hw_pipe_cfg out of struct dpu_plane
  drm/msm/dpu: drop scaler config from plane state
  drm/msm/dpu: drop dpu_csc_cfg from dpu_plane
  drm/msm/dpu: remove dpu_hw_pipe_cdp_cfg from dpu_plane
  drm/msm/dpu: don't cache pipe->cap->features in dpu_plane
  drm/msm/dpu: don't cache pipe->cap->sblk in dpu_plane
  drm/msm/dpu: rip out debugfs support from dpu_plane
  drm/msm/dpu: drop src_split and multirect check from 
dpu_crtc_atomic_check

  drm/msm/dpu: add list of supported formats to the DPU caps
  drm/msm/dpu: simplify DPU_SSPP features checks
  drm/msm/dpu: do not limit the zpos property
  drm/msm/dpu: add support for SSPP allocation to RM
  drm/msm/dpu: move pipe_hw to dpu_plane_state
  drm/msm/dpu: add support for virtualized planes
  drm/msm/dpu: fix smart dma support
  drm/msm/dpu: fix CDP setup to account for multirect index
  drm/msm/dpu: add multirect support

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c   | 261 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h   |   2 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |  20 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |  20 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c|  41 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h|  52 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c|   2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h|   2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 234 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h|  70 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c  | 851 
+++--

 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h  |  75 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c |  81 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h |   6 +
 14 files changed, 793 insertions(+), 924 deletions(-)


Re: [Freedreno] [PATCH v2] drm/msm/dp: only signal audio when disconnected detected at dp_pm_resume

2021-09-29 Thread Stephen Boyd
Quoting Kuogee Hsieh (2021-09-29 09:17:04)
> Currently there is audio not working problem after system resume from suspend
> if hdmi monitor stay plugged in at DUT. However this problem does not happen
> at normal operation but at a particular test case. The root cause is DP driver
> signal audio with connected state at resume which trigger audio trying to 
> setup
> audio data path through DP main link but failed due to display port is not 
> setup
> and enabled by upper layer framework yet. This patch only have DP driver 
> signal
> audio only when DP is in disconnected state so that audio option shows correct
> state after system resume. DP driver will not signal audio with connected 
> state
> until display enabled executed by upper layer framework where display port is
> setup completed and main link is running.
>
> Changes in V2:
> -- add details commit text
>
> Fixes: 078867ce04ed ("drm/msm/dp: signal audio plugged change at 
> dp_pm_resume")
> Signed-off-by: Kuogee Hsieh 
> ---

Reviewed-by: Stephen Boyd 


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
>
> On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
> > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > The best practice to avoid those issues is to register its functions 
> > > > only after
> > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > we should
> > > > to play nice with the other components that are waiting for us, so in 
> > > > our case
> > > > that would mean moving the DSI device registration to the bridge probe.
> > > >
> > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin 
> > > > and msm
> > > > would be affected by this and wouldn't probe anymore after those 
> > > > changes.
> > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > (that still
> > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > important
> > > > and I wasn't confortable doing them.
> > >
> > >
> > > Hey Maxime,
> > >   Sorry for taking so long to get to this, but now that plumbers is
> > > over I've had a chance to check it out on kirin
> > >
> > > Rob Clark pointed me to his branch with some fixups here:
> > >
> > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > >
> > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > regulator
> > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > regulator
> > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > regulator
> > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > regulator
> > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > regulator
> > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > regulator
> > > [4.681898] adv7511 2-0039: failed to find dsi host
> >
> > I just realized Rob's tree is missing the kirin patch. My apologies!
> > I'll retest and let you know.
>
> Ok, just retested including the kirin patch and unfortunately I'm
> still seeing the same thing.  :(
>
> Will dig a bit and let you know when I find more.

Hey Maxime!
  I chased down the issue. The dsi probe code was still calling
drm_of_find_panel_or_bridge() in order to succeed.

I've moved the logic that looks for the bridge into the bridge_init
and with that it seems to work.

Feel free (assuming it looks ok) to fold this change into your kirin patch:
  
https://git.linaro.org/people/john.stultz/android-dev.git/commit/?id=4a35ccc4d7a53f68d6d93da3b47e232a7c75b91d

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 4:20 PM Rob Clark  wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  
> > > wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > > The best practice to avoid those issues is to register its functions 
> > > > > only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > > we should
> > > > > to play nice with the other components that are waiting for us, so in 
> > > > > our case
> > > > > that would mean moving the DSI device registration to the bridge 
> > > > > probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, 
> > > > > kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those 
> > > > > changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > > (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > > important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >
> > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop 
> > > > indefinitely:
> > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > regulator
> > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > regulator
> > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > regulator
> > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > > regulator
> > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > regulator
> > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > regulator
> > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

I believe Amit(cc'ed) tried to give it a run on his pocof1, but had
some troubles getting it working against a kernel that wasn't
suffering other regressions at the moment.

Amit/Caleb: Any chance one of you could try again w/ these merged to 5.15-rc3?

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread Rob Clark
On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
>
> On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
> > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > The best practice to avoid those issues is to register its functions 
> > > > only after
> > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > we should
> > > > to play nice with the other components that are waiting for us, so in 
> > > > our case
> > > > that would mean moving the DSI device registration to the bridge probe.
> > > >
> > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin 
> > > > and msm
> > > > would be affected by this and wouldn't probe anymore after those 
> > > > changes.
> > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > (that still
> > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > important
> > > > and I wasn't confortable doing them.
> > >
> > >
> > > Hey Maxime,
> > >   Sorry for taking so long to get to this, but now that plumbers is
> > > over I've had a chance to check it out on kirin
> > >
> > > Rob Clark pointed me to his branch with some fixups here:
> > >
> > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > >
> > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > regulator
> > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > regulator
> > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > regulator
> > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > regulator
> > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > regulator
> > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > regulator
> > > [4.681898] adv7511 2-0039: failed to find dsi host
> >
> > I just realized Rob's tree is missing the kirin patch. My apologies!
> > I'll retest and let you know.
>
> Ok, just retested including the kirin patch and unfortunately I'm
> still seeing the same thing.  :(
>
> Will dig a bit and let you know when I find more.

Did you have a chance to test it on anything using drm/msm with DSI
panels?  That would at least confirm that I didn't miss anything in
the drm/msm patch to swap the dsi-host vs bridge ordering..

BR,
-R


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread Laurent Pinchart
Hi Maxime,

(CC'ing Kieran)

On Fri, Sep 10, 2021 at 12:11:54PM +0200, Maxime Ripard wrote:
> Hi,
> 
> We've encountered an issue with the RaspberryPi DSI panel that prevented the
> whole display driver from probing.
> 
> The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> Only register our component once a DSI device is attached"), but the basic 
> idea
> is that since the panel is probed through i2c, there's no synchronization
> between its probe and the registration of the MIPI-DSI host it's attached to.
> 
> We initially moved the component framework registration to the MIPI-DSI Host
> attach hook to make sure we register our component only when we have a DSI
> device attached to our MIPI-DSI host, and then use lookup our DSI device in 
> our
> bind hook.
> 
> However, all the DSI bridges controlled through i2c are only registering their
> associated DSI device in their bridge attach hook, meaning with our change
> above, we never got that far, and therefore ended up in the same situation 
> than
> the one we were trying to fix for panels.
> 
> The best practice to avoid those issues is to register its functions only 
> after
> all its dependencies are live. We also shouldn't wait any longer than we 
> should
> to play nice with the other components that are waiting for us, so in our case
> that would mean moving the DSI device registration to the bridge probe.
> 
> I also had a look at all the DSI hosts, and it seems that exynos, kirin and 
> msm
> would be affected by this and wouldn't probe anymore after those changes.
> Exynos and kirin seems to be simple enough for a mechanical change (that still
> requires to be tested), but the changes in msm seemed to be far more important
> and I wasn't confortable doing them.
> 
> Let me know what you think,

I've tested this series on my RPi CM4-based board, and there's a clear
improvement: the sn65dsi83 now probes successfully !

The downside is that I can now look at a panel that desperately refuses
to display anything. That's a separate issue, but it prevents me from
telling whether this series introduces regressions :-S I'll try to debug
that separately.

Also, Kieran, would you be able to test this with the SN65DSI86 ?

> ---
> 
> Changes from v3:
>   - Converted exynos and kirin
>   - Converted all the affected bridge drivers
>   - Reworded the documentation a bit
> 
> Changes from v2:
>   - Changed the approach as suggested by Andrzej, and aligned the bridge on 
> the
> panel this time.
>   - Fixed some typos
> 
> Changes from v1:
>   - Change the name of drm_of_get_next function to drm_of_get_bridge
>   - Mention the revert of 87154ff86bf6 and squash the two patches that were
> reverting that commit
>   - Add some documentation
>   - Make drm_panel_attach and _detach succeed when no callback is there
> 
> Maxime Ripard (24):
>   drm/bridge: Add documentation sections
>   drm/bridge: Document the probe issue with MIPI-DSI bridges
>   drm/mipi-dsi: Create devm device registration
>   drm/mipi-dsi: Create devm device attachment
>   drm/bridge: adv7533: Switch to devm MIPI-DSI helpers
>   drm/bridge: adv7511: Register and attach our DSI device at probe
>   drm/bridge: anx7625: Switch to devm MIPI-DSI helpers
>   drm/bridge: anx7625: Register and attach our DSI device at probe
>   drm/bridge: lt8912b: Switch to devm MIPI-DSI helpers
>   drm/bridge: lt8912b: Register and attach our DSI device at probe
>   drm/bridge: lt9611: Switch to devm MIPI-DSI helpers
>   drm/bridge: lt9611: Register and attach our DSI device at probe
>   drm/bridge: lt9611uxc: Switch to devm MIPI-DSI helpers
>   drm/bridge: lt9611uxc: Register and attach our DSI device at probe
>   drm/bridge: ps8640: Switch to devm MIPI-DSI helpers
>   drm/bridge: ps8640: Register and attach our DSI device at probe
>   drm/bridge: sn65dsi83: Switch to devm MIPI-DSI helpers
>   drm/bridge: sn65dsi83: Register and attach our DSI device at probe
>   drm/bridge: sn65dsi86: Switch to devm MIPI-DSI helpers
>   drm/bridge: sn65dsi86: Register and attach our DSI device at probe
>   drm/bridge: tc358775: Switch to devm MIPI-DSI helpers
>   drm/bridge: tc358775: Register and attach our DSI device at probe
>   drm/kirin: dsi: Adjust probe order
>   drm/exynos: dsi: Adjust probe order
> 
>  Documentation/gpu/drm-kms-helpers.rst|  12 +++
>  drivers/gpu/drm/bridge/adv7511/adv7511.h |   1 -
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  15 ++-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c |  20 +---
>  drivers/gpu/drm/bridge/analogix/anx7625.c|  40 
>  drivers/gpu/drm/bridge/lontium-lt8912b.c |  31 ++
>  drivers/gpu/drm/bridge/lontium-lt9611.c  |  62 +---
>  drivers/gpu/drm/bridge/lontium-lt9611uxc.c   |  65 +---
>  drivers/gpu/drm/bridge/parade-ps8640.c   | 101 ++-
>  drivers/gpu/drm/bridge/tc358775.c|  50 +
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c|  86 
>  

Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
> > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > The best practice to avoid those issues is to register its functions only 
> > > after
> > > all its dependencies are live. We also shouldn't wait any longer than we 
> > > should
> > > to play nice with the other components that are waiting for us, so in our 
> > > case
> > > that would mean moving the DSI device registration to the bridge probe.
> > >
> > > I also had a look at all the DSI hosts, and it seems that exynos, kirin 
> > > and msm
> > > would be affected by this and wouldn't probe anymore after those changes.
> > > Exynos and kirin seems to be simple enough for a mechanical change (that 
> > > still
> > > requires to be tested), but the changes in msm seemed to be far more 
> > > important
> > > and I wasn't confortable doing them.
> >
> >
> > Hey Maxime,
> >   Sorry for taking so long to get to this, but now that plumbers is
> > over I've had a chance to check it out on kirin
> >
> > Rob Clark pointed me to his branch with some fixups here:
> >
> > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> >
> > But trying to boot hikey with that, I see the following loop indefinitely:
> > [4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > [4.681898] adv7511 2-0039: failed to find dsi host
>
> I just realized Rob's tree is missing the kirin patch. My apologies!
> I'll retest and let you know.

Ok, just retested including the kirin patch and unfortunately I'm
still seeing the same thing.  :(

Will dig a bit and let you know when I find more.

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
>
> On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> >
> > We've encountered an issue with the RaspberryPi DSI panel that prevented the
> > whole display driver from probing.
> >
> > The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> > Only register our component once a DSI device is attached"), but the basic 
> > idea
> > is that since the panel is probed through i2c, there's no synchronization
> > between its probe and the registration of the MIPI-DSI host it's attached 
> > to.
> >
> > We initially moved the component framework registration to the MIPI-DSI Host
> > attach hook to make sure we register our component only when we have a DSI
> > device attached to our MIPI-DSI host, and then use lookup our DSI device in 
> > our
> > bind hook.
> >
> > However, all the DSI bridges controlled through i2c are only registering 
> > their
> > associated DSI device in their bridge attach hook, meaning with our change
> > above, we never got that far, and therefore ended up in the same situation 
> > than
> > the one we were trying to fix for panels.
> >
> > The best practice to avoid those issues is to register its functions only 
> > after
> > all its dependencies are live. We also shouldn't wait any longer than we 
> > should
> > to play nice with the other components that are waiting for us, so in our 
> > case
> > that would mean moving the DSI device registration to the bridge probe.
> >
> > I also had a look at all the DSI hosts, and it seems that exynos, kirin and 
> > msm
> > would be affected by this and wouldn't probe anymore after those changes.
> > Exynos and kirin seems to be simple enough for a mechanical change (that 
> > still
> > requires to be tested), but the changes in msm seemed to be far more 
> > important
> > and I wasn't confortable doing them.
>
>
> Hey Maxime,
>   Sorry for taking so long to get to this, but now that plumbers is
> over I've had a chance to check it out on kirin
>
> Rob Clark pointed me to his branch with some fixups here:
>
> https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
>
> But trying to boot hikey with that, I see the following loop indefinitely:
> [4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> [4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> [4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> [4.681898] adv7511 2-0039: failed to find dsi host

I just realized Rob's tree is missing the kirin patch. My apologies!
I'll retest and let you know.

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
>
> We've encountered an issue with the RaspberryPi DSI panel that prevented the
> whole display driver from probing.
>
> The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> Only register our component once a DSI device is attached"), but the basic 
> idea
> is that since the panel is probed through i2c, there's no synchronization
> between its probe and the registration of the MIPI-DSI host it's attached to.
>
> We initially moved the component framework registration to the MIPI-DSI Host
> attach hook to make sure we register our component only when we have a DSI
> device attached to our MIPI-DSI host, and then use lookup our DSI device in 
> our
> bind hook.
>
> However, all the DSI bridges controlled through i2c are only registering their
> associated DSI device in their bridge attach hook, meaning with our change
> above, we never got that far, and therefore ended up in the same situation 
> than
> the one we were trying to fix for panels.
>
> The best practice to avoid those issues is to register its functions only 
> after
> all its dependencies are live. We also shouldn't wait any longer than we 
> should
> to play nice with the other components that are waiting for us, so in our case
> that would mean moving the DSI device registration to the bridge probe.
>
> I also had a look at all the DSI hosts, and it seems that exynos, kirin and 
> msm
> would be affected by this and wouldn't probe anymore after those changes.
> Exynos and kirin seems to be simple enough for a mechanical change (that still
> requires to be tested), but the changes in msm seemed to be far more important
> and I wasn't confortable doing them.


Hey Maxime,
  Sorry for taking so long to get to this, but now that plumbers is
over I've had a chance to check it out on kirin

Rob Clark pointed me to his branch with some fixups here:
   
https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework

But trying to boot hikey with that, I see the following loop indefinitely:
[4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
[4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
[4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
[4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
[4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
[4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
[4.681898] adv7511 2-0039: failed to find dsi host
[4.688836] adv7511 2-0039: supply avdd not found, using dummy regulator
[4.695724] adv7511 2-0039: supply dvdd not found, using dummy regulator
[4.702583] adv7511 2-0039: supply pvdd not found, using dummy regulator
[4.709369] adv7511 2-0039: supply a2vdd not found, using dummy regulator
[4.716232] adv7511 2-0039: supply v3p3 not found, using dummy regulator
[4.722972] adv7511 2-0039: supply v1p2 not found, using dummy regulator
[4.738720] adv7511 2-0039: failed to find dsi host

I'll have to dig a bit to figure out what's going wrong, but wanted to
give you the heads up that there seems to be a problem

thanks
-john


Re: [Freedreno] [PATCH] [RFC] qcom_scm: hide Kconfig symbol

2021-09-29 Thread Arnd Bergmann
On Wed, Sep 29, 2021 at 4:46 PM Bjorn Andersson
 wrote:
>
> On Wed 29 Sep 05:04 CDT 2021, Arnd Bergmann wrote:
>
> > On Wed, Sep 29, 2021 at 11:51 AM Will Deacon  wrote:
> > > On Mon, Sep 27, 2021 at 05:22:13PM +0200, Arnd Bergmann wrote:
> > > >
> > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > > index 124c41adeca1..989c83acbfee 100644
> > > > --- a/drivers/iommu/Kconfig
> > > > +++ b/drivers/iommu/Kconfig
> > > > @@ -308,7 +308,7 @@ config APPLE_DART
> > > >  config ARM_SMMU
> > > >   tristate "ARM Ltd. System MMU (SMMU) Support"
> > > >   depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> > > > - depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> > > > + select QCOM_SCM
> > > >   select IOMMU_API
> > > >   select IOMMU_IO_PGTABLE_LPAE
> > > >   select ARM_DMA_USE_IOMMU if ARM
> > >
> > > I don't want to get in the way of this patch because I'm also tired of the
> > > randconfig failures caused by QCOM_SCM. However, ARM_SMMU is applicable to
> > > a wide variety of (non-qcom) SoCs and so it seems a shame to require the
> > > QCOM_SCM code to be included for all of those when it's not strictly 
> > > needed
> > > at all.
> >
> > Good point, I agree that needs to be fixed. I think this additional
> > change should do the trick:
> >
>
> ARM_SMMU and QCOM_IOMMU are two separate implementations and both uses
> QCOM_SCM. So both of them should select QCOM_SCM.

Right, I figured that out later as well.

> "Unfortunately" the Qualcomm portion of ARM_SMMU is builtin
> unconditionally, so going with something like select QCOM_SCM if
> ARCH_QCOM would still require the stubs in qcom_scm.h.

Yes, sounds good. I also noticed that I still need one hack in there
if I do this:

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 55690af1b25d..36c304a8fc9b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -427,6 +427,9 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct
arm_smmu_device *smmu)
 {
const struct device_node *np = smmu->dev->of_node;

+   if (!IS_ENABLED(CONFIG_QCOM_SCM))
+   return ERR_PTR(-ENXIO);
+
 #ifdef CONFIG_ACPI
if (np == NULL) {
/* Match platform for ACPI boot */


Otherwise it still breaks with ARM_SMMU=y and QCOM_SCM=m.

Splitting out the qualcomm portion of the arm_smmu driver using
a separate 'bool' symbol should also work, if  you prefer that
and can suggest a name and help text for that symbol. It would
look like

diff --git a/drivers/iommu/arm/arm-smmu/Makefile
b/drivers/iommu/arm/arm-smmu/Makefile
index e240a7bcf310..b0cc01aa20c9 100644
--- a/drivers/iommu/arm/arm-smmu/Makefile
+++ b/drivers/iommu/arm/arm-smmu/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
 obj-$(CONFIG_ARM_SMMU) += arm_smmu.o
-arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o arm-smmu-qcom.o
+arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o
+arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index 9f465e146799..2c25cce38060 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -215,7 +215,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
arm_smmu_device *smmu)
of_device_is_compatible(np, "nvidia,tegra186-smmu"))
return nvidia_smmu_impl_init(smmu);

-   smmu = qcom_smmu_impl_init(smmu);
+   if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
+   smmu = qcom_smmu_impl_init(smmu);

if (of_device_is_compatible(np, "marvell,ap806-smmu-500"))
smmu->impl = _mmu500_impl;



   Arnd


[Freedreno] [PATCH] drm/msm/dsi: prevent unintentional integer overflow in dsi_pll_28nm_clk_recalc_rate()

2021-09-29 Thread Tim Gardner
Coverity warns of an unintentional integer overflow

CID 120715 (#1 of 1): Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression ref_clk * sdm_byp_div
  with type unsigned int (32 bits, unsigned) is evaluated using 32-bit 
arithmetic,
  and then used in a context that expects an expression of type unsigned long
  (64 bits, unsigned).
To avoid overflow, cast either ref_clk or sdm_byp_div to type unsigned long.
263vco_rate = ref_clk * sdm_byp_div;

Fix this and another possible overflow by casting ref_clk to unsigned long.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Dmitry Baryshkov 
Cc: Abhinav Kumar 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner 
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index 2da673a2add6..cfe4b30eb96d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -260,7 +260,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct 
clk_hw *hw,
sdm_byp_div = FIELD(
dsi_phy_read(base + 
REG_DSI_28nm_PHY_PLL_SDM_CFG0),
DSI_28nm_PHY_PLL_SDM_CFG0_BYP_DIV) + 1;
-   vco_rate = ref_clk * sdm_byp_div;
+   vco_rate = (unsigned long)ref_clk * sdm_byp_div;
} else {
/* sdm mode */
sdm_dc_off = FIELD(
@@ -274,7 +274,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct 
clk_hw *hw,
sdm_freq_seed = (sdm3 << 8) | sdm2;
DBG("sdm_freq_seed = %d", sdm_freq_seed);
 
-   vco_rate = (ref_clk * (sdm_dc_off + 1)) +
+   vco_rate = ((unsigned long)ref_clk * (sdm_dc_off + 1)) +
mult_frac(ref_clk, sdm_freq_seed, BIT(16));
DBG("vco rate = %lu", vco_rate);
}
-- 
2.33.0



[Freedreno] [PATCH] drm/msm: prevent NULL dereference in msm_gpu_crashstate_capture()

2021-09-29 Thread Tim Gardner
Coverity complains of a possible NULL dereference:

CID 120718 (#1 of 1): Dereference null return value (NULL_RETURNS)
23. dereference: Dereferencing a pointer that might be NULL state->bos when
calling msm_gpu_crashstate_get_bo. [show details]
301msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
302submit->bos[i].iova, submit->bos[i].flags);

Fix this by employing the same state->bos NULL check as is used in the next
for loop.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner 
---
 drivers/gpu/drm/msm/msm_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 8a3a592da3a4..2c46cd968ac4 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -296,7 +296,7 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
state->bos = kcalloc(nr,
sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
 
-   for (i = 0; i < submit->nr_bos; i++) {
+   for (i = 0; state->bos && i < submit->nr_bos; i++) {
if (should_dump(submit, i)) {
msm_gpu_crashstate_get_bo(state, 
submit->bos[i].obj,
submit->bos[i].iova, 
submit->bos[i].flags);
-- 
2.33.0



[Freedreno] [PATCH v2] drm/msm/dp: only signal audio when disconnected detected at dp_pm_resume

2021-09-29 Thread Kuogee Hsieh
Currently there is audio not working problem after system resume from suspend
if hdmi monitor stay plugged in at DUT. However this problem does not happen
at normal operation but at a particular test case. The root cause is DP driver
signal audio with connected state at resume which trigger audio trying to setup
audio data path through DP main link but failed due to display port is not setup
and enabled by upper layer framework yet. This patch only have DP driver signal
audio only when DP is in disconnected state so that audio option shows correct
state after system resume. DP driver will not signal audio with connected state
until display enabled executed by upper layer framework where display port is
setup completed and main link is running.

Changes in V2:
-- add details commit text

Fixes: 078867ce04ed ("drm/msm/dp: signal audio plugged change at dp_pm_resume")
Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_display.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 0e543a03..6f13008 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1356,14 +1356,14 @@ static int dp_pm_resume(struct device *dev)
 * can not declared display is connected unless
 * HDMI cable is plugged in and sink_count of
 * dongle become 1
+* also only signal audio when disconnected
 */
-   if (dp->link->sink_count)
+   if (dp->link->sink_count) {
dp->dp_display.is_connected = true;
-   else
+   } else {
dp->dp_display.is_connected = false;
-
-   dp_display_handle_plugged_change(g_dp_display,
-   dp->dp_display.is_connected);
+   dp_display_handle_plugged_change(g_dp_display, false);
+   }
 
DRM_DEBUG_DP("After, sink_count=%d is_connected=%d core_inited=%d 
power_on=%d\n",
dp->link->sink_count, dp->dp_display.is_connected,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [Freedreno] [PATCH v2 13/13] drm/msm: Implement HDCP 1.x using the new drm HDCP helpers

2021-09-29 Thread Sean Paul
On Tue, Sep 28, 2021 at 02:35:09PM -0700, abhin...@codeaurora.org wrote:
> On 2021-09-28 11:02, Sean Paul wrote:
> > On Tue, Sep 21, 2021 at 07:25:41PM -0700, abhin...@codeaurora.org wrote:
> > > On 2021-09-15 13:38, Sean Paul wrote:
> > > > From: Sean Paul 
> > > >
> > > > This patch adds HDCP 1.x support to msm DP connectors using the new HDCP
> > > > helpers.
> > > >
> > > > Cc: Stephen Boyd 
> > > > Signed-off-by: Sean Paul 
> > > > Link:
> > > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-s...@poorly.run
> > > > #v1
> > > >
> > > > Changes in v2:
> > > > -Squash [1] into this patch with the following changes (Stephen)
> > > >   -Update the sc7180 dtsi file
> > > >   -Remove resource names and just use index (Stephen)
> > > >
> > > 
> > > 
> > > > [1]
> > > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-14-s...@poorly.run
> > > > ---
> > 
> > /snip
> > 
> > > > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> > > > index 904535eda0c4..98731fd262d6 100644
> > > > --- a/drivers/gpu/drm/msm/Makefile
> > > > +++ b/drivers/gpu/drm/msm/Makefile
> > > > @@ -109,6 +109,7 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
> > > > dp/dp_ctrl.o \
> > > > dp/dp_display.o \
> > > > dp/dp_drm.o \
> > > > +   dp/dp_hdcp.o \
> > > > dp/dp_hpd.o \
> > > > dp/dp_link.o \
> > > > dp/dp_panel.o \
> > > > diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
> > > > b/drivers/gpu/drm/msm/dp/dp_debug.c
> > > > index 2f6247e80e9d..de16fca8782a 100644
> > > > --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> > > > +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> > 
> > /snip
> > 
> > > > +static ssize_t dp_hdcp_key_write(struct file *file, const char __user
> > > > *ubuf,
> > > > +size_t len, loff_t *offp)
> > > > +{
> > > > +   char *input_buffer;
> > > > +   int ret = 0;
> > > > +   struct dp_debug_private *debug = file->private_data;
> > > > +   struct drm_device *dev;
> > > > +
> > > > +   dev = debug->drm_dev;
> > > > +
> > > > +   if (len != (DRM_HDCP_KSV_LEN + DP_HDCP_NUM_KEYS * 
> > > > DP_HDCP_KEY_LEN))
> > > > +   return -EINVAL;
> > > > +
> > > > +   if (!debug->hdcp)
> > > > +   return -ENOENT;
> > > > +
> > > > +   input_buffer = memdup_user_nul(ubuf, len);
> > > > +   if (IS_ERR(input_buffer))
> > > > +   return PTR_ERR(input_buffer);
> > > > +
> > > > +   ret = dp_hdcp_ingest_key(debug->hdcp, input_buffer, len);
> > > > +
> > > > +   kfree(input_buffer);
> > > > +   if (ret < 0) {
> > > > +   DRM_ERROR("Could not ingest HDCP key, ret=%d\n", ret);
> > > > +   return ret;
> > > > +   }
> > > > +
> > > > +   *offp += len;
> > > > +   return len;
> > > > +}
> > > 
> > > It seems like the HDCP keys written using debugfs, just for my
> > > understanding,
> > > are you storing this in some secure partition and the usermode reads
> > > from it
> > > and writes them here?
> > > 
> > 
> > We have not sorted out the userspace side of HDCP enablement yet, so it
> > remains
> > to be seen whether the keys will be injected via debugfs/firmware
> > file/property.
> > 
> > /snip
> > 
> > > > +static int dp_connector_atomic_check(struct drm_connector *connector,
> > > > +struct drm_atomic_state *state)
> > > > +{
> > > > +   struct drm_connector_state *conn_state;
> > > > +   struct dp_connector_state *dp_state;
> > > > +
> > > > +   conn_state = drm_atomic_get_new_connector_state(state, 
> > > > connector);
> > > > +   dp_state = to_dp_connector_state(conn_state);
> > > > +
> > > > +   dp_state->hdcp_transition = drm_hdcp_atomic_check(connector, 
> > > > state);
> > > 
> > > I have a general question related to the transition flag and overall
> > > tying
> > > the HDCP
> > > enable and authentication to the commit.
> > > So lets say there is a case where the driver needs to disable HDCP.
> > > It could
> > > be due
> > > to link integrity failure OR some other error condition which
> > > usermode is
> > > not aware of.
> > > In that case, we will set this hdcp_transition to true but in the next
> > > commit we will
> > > actually do the authentication. What if usermode doesnt issue a new
> > > frame?
> > > This question arises because currently the link intergrity check is
> > > done
> > > using SW polling
> > > in the previous patchset. But as I had commented there, this occurs
> > > in HW
> > > for us.
> > > I dont see that isr itself in this patchset. So wanted to understand
> > > if
> > > thats part of this
> > > approach to still tie it with commit.
> > > 
> > > So if we go with the HW polling based approach which is the preferred
> > > method, we need to
> > > untie this from the commit.
> > > 
> > 
> > In the case of error, the worker will detect it and try to
> > re-authenticate. If
> > the re-authentication is 

Re: [Freedreno] [PATCH 2/2] [v2] qcom_scm: hide Kconfig symbol

2021-09-29 Thread Bjorn Andersson
On Tue 28 Sep 02:50 CDT 2021, Arnd Bergmann wrote:

> From: Arnd Bergmann 
> 
> Now that SCM can be a loadable module, we have to add another
> dependency to avoid link failures when ipa or adreno-gpu are
> built-in:
> 
> aarch64-linux-ld: drivers/net/ipa/ipa_main.o: in function `ipa_probe':
> ipa_main.c:(.text+0xfc4): undefined reference to `qcom_scm_is_available'
> 
> ld.lld: error: undefined symbol: qcom_scm_is_available
> >>> referenced by adreno_gpu.c
> >>>   gpu/drm/msm/adreno/adreno_gpu.o:(adreno_zap_shader_load) in 
> >>> archive drivers/built-in.a
> 
> This can happen when CONFIG_ARCH_QCOM is disabled and we don't select
> QCOM_MDT_LOADER, but some other module selects QCOM_SCM. Ideally we'd
> use a similar dependency here to what we have for QCOM_RPROC_COMMON,
> but that causes dependency loops from other things selecting QCOM_SCM.
> 
> This appears to be an endless problem, so try something different this
> time:
> 
>  - CONFIG_QCOM_SCM becomes a hidden symbol that nothing 'depends on'
>but that is simply selected by all of its users
> 
>  - All the stubs in include/linux/qcom_scm.h can go away
> 
>  - arm-smccc.h needs to provide a stub for __arm_smccc_smc() to
>allow compile-testing QCOM_SCM on all architectures.
> 
>  - To avoid a circular dependency chain involving RESET_CONTROLLER
>and PINCTRL_SUNXI, drop the 'select RESET_CONTROLLER' statement.
>According to my testing this still builds fine, and the QCOM
>platform selects this symbol already.
> 
> Acked-by: Kalle Valo 
> Signed-off-by: Arnd Bergmann 
> ---
> Changes in v2:
>   - drop the 'select RESET_CONTROLLER' line, rather than adding
> more of the same
> ---
>  drivers/firmware/Kconfig|  5 +-
>  drivers/gpu/drm/msm/Kconfig |  4 +-
>  drivers/iommu/Kconfig   |  2 +-
>  drivers/media/platform/Kconfig  |  2 +-
>  drivers/mmc/host/Kconfig|  2 +-
>  drivers/net/ipa/Kconfig |  1 +
>  drivers/net/wireless/ath/ath10k/Kconfig |  2 +-
>  drivers/pinctrl/qcom/Kconfig|  3 +-
>  include/linux/arm-smccc.h   | 10 
>  include/linux/qcom_scm.h| 71 -
>  10 files changed, 20 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 220a58cf0a44..cda7d7162cbb 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -203,10 +203,7 @@ config INTEL_STRATIX10_RSU
> Say Y here if you want Intel RSU support.
>  
>  config QCOM_SCM
> - tristate "Qcom SCM driver"
> - depends on ARM || ARM64
> - depends on HAVE_ARM_SMCCC
> - select RESET_CONTROLLER
> + tristate
>  
>  config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
>   bool "Qualcomm download mode enabled by default"
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index e9c6af78b1d7..3ddf739a6f9b 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -17,7 +17,7 @@ config DRM_MSM
>   select DRM_SCHED
>   select SHMEM
>   select TMPFS
> - select QCOM_SCM if ARCH_QCOM
> + select QCOM_SCM
>   select WANT_DEV_COREDUMP
>   select SND_SOC_HDMI_CODEC if SND_SOC
>   select SYNC_FILE
> @@ -55,7 +55,7 @@ config DRM_MSM_GPU_SUDO
>  
>  config DRM_MSM_HDMI_HDCP
>   bool "Enable HDMI HDCP support in MSM DRM driver"
> - depends on DRM_MSM && QCOM_SCM
> + depends on DRM_MSM
>   default y
>   help
> Choose this option to enable HDCP state machine
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 124c41adeca1..989c83acbfee 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -308,7 +308,7 @@ config APPLE_DART
>  config ARM_SMMU
>   tristate "ARM Ltd. System MMU (SMMU) Support"
>   depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> - depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> + select QCOM_SCM
>   select IOMMU_API
>   select IOMMU_IO_PGTABLE_LPAE
>   select ARM_DMA_USE_IOMMU if ARM

As noted in the RFC, I think you also need to fix QCOM_IOMMU.

In particular (iirc) since all the users of the iommu might be modules,
which would prevent QCOM_IOMMU from being selected.

The rest looks good.

Regards,
Bjorn

> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 157c924686e4..80321e03809a 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -565,7 +565,7 @@ config VIDEO_QCOM_VENUS
>   depends on VIDEO_DEV && VIDEO_V4L2 && QCOM_SMEM
>   depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST
>   select QCOM_MDT_LOADER if ARCH_QCOM
> - select QCOM_SCM if ARCH_QCOM
> + select QCOM_SCM
>   select VIDEOBUF2_DMA_CONTIG
>   select V4L2_MEM2MEM_DEV
>   help
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 

Re: [Freedreno] [PATCH] [RFC] qcom_scm: hide Kconfig symbol

2021-09-29 Thread Bjorn Andersson
On Wed 29 Sep 05:04 CDT 2021, Arnd Bergmann wrote:

> On Wed, Sep 29, 2021 at 11:51 AM Will Deacon  wrote:
> > On Mon, Sep 27, 2021 at 05:22:13PM +0200, Arnd Bergmann wrote:
> > >
> > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > index 124c41adeca1..989c83acbfee 100644
> > > --- a/drivers/iommu/Kconfig
> > > +++ b/drivers/iommu/Kconfig
> > > @@ -308,7 +308,7 @@ config APPLE_DART
> > >  config ARM_SMMU
> > >   tristate "ARM Ltd. System MMU (SMMU) Support"
> > >   depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> > > - depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> > > + select QCOM_SCM
> > >   select IOMMU_API
> > >   select IOMMU_IO_PGTABLE_LPAE
> > >   select ARM_DMA_USE_IOMMU if ARM
> >
> > I don't want to get in the way of this patch because I'm also tired of the
> > randconfig failures caused by QCOM_SCM. However, ARM_SMMU is applicable to
> > a wide variety of (non-qcom) SoCs and so it seems a shame to require the
> > QCOM_SCM code to be included for all of those when it's not strictly needed
> > at all.
> 
> Good point, I agree that needs to be fixed. I think this additional
> change should do the trick:
> 

ARM_SMMU and QCOM_IOMMU are two separate implementations and both uses
QCOM_SCM. So both of them should select QCOM_SCM.

"Unfortunately" the Qualcomm portion of ARM_SMMU is builtin
unconditionally, so going with something like select QCOM_SCM if
ARCH_QCOM would still require the stubs in qcom_scm.h.

Regards,
Bjorn

> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -308,7 +308,6 @@ config APPLE_DART
>  config ARM_SMMU
> tristate "ARM Ltd. System MMU (SMMU) Support"
> depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> -   select QCOM_SCM
> select IOMMU_API
> select IOMMU_IO_PGTABLE_LPAE
> select ARM_DMA_USE_IOMMU if ARM
> @@ -438,7 +437,7 @@ config QCOM_IOMMU
> # Note: iommu drivers cannot (yet?) be built as modules
> bool "Qualcomm IOMMU Support"
> depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> -   depends on QCOM_SCM=y
> +   select QCOM_SCM
> select IOMMU_API
> select IOMMU_IO_PGTABLE_LPAE
> select ARM_DMA_USE_IOMMU
> 
> I'll see if that causes any problems for the randconfig builds.
> 
>Arnd


[Freedreno] [PATCH] drm/msm: Fix null pointer dereference on pointer edp

2021-09-29 Thread Colin King
From: Colin Ian King 

The initialization of pointer dev dereferences pointer edp before
edp is null checked, so there is a potential null pointer deference
issue. Fix this by only dereferencing edp after edp has been null
checked.

Addresses-Coverity: ("Dereference before null check")
Fixes: ab5b0107ccf3 ("drm/msm: Initial add eDP support in msm drm driver (v5)")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/msm/edp/edp_ctrl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c 
b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index 4fb397ee7c84..fe1366b4c49f 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -1116,7 +1116,7 @@ void msm_edp_ctrl_power(struct edp_ctrl *ctrl, bool on)
 int msm_edp_ctrl_init(struct msm_edp *edp)
 {
struct edp_ctrl *ctrl = NULL;
-   struct device *dev = >pdev->dev;
+   struct device *dev;
int ret;
 
if (!edp) {
@@ -1124,6 +1124,7 @@ int msm_edp_ctrl_init(struct msm_edp *edp)
return -EINVAL;
}
 
+   dev = >pdev->dev;
ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
if (!ctrl)
return -ENOMEM;
-- 
2.32.0



[Freedreno] [PATCH][V2] drm/msm: Fix potential integer overflow on 32 bit multiply

2021-09-29 Thread Colin King
From: Colin Ian King 

In the case where clock is 2147485 or greater the 32 bit multiplication
by 1000 will cause an integer overflow. Fix this by making the constant
1000 an unsigned long to ensure a long multiply occurs to avoid the
overflow before assigning the result to the long result in variable
requested.  Most probably a theoretical overflow issue, but worth fixing
to clear up static analysis warnings.

Addresses-Coverity: ("Unintentional integer overflow")
Fixes: c8afe684c95c ("drm/msm: basic KMS driver for snapdragon")
Fixes: 3e87599b68e7 ("drm/msm/mdp4: add LVDS panel support")
Fixes: 937f941ca06f ("drm/msm/dp: Use qmp phy for DP PLL and PHY")
Fixes: ab5b0107ccf3 ("drm/msm: Initial add eDP support in msm drm driver (v5)")
Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")

Signed-off-by: Colin Ian King 
---
V2: Find and fix all unintentional integer overflows that match this
overflow pattern.
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c| 2 +-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c   | 2 +-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +-
 drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 ++--
 drivers/gpu/drm/msm/edp/edp_connector.c | 2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c  | 2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c   | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
index 88645dbc3785..83140066441e 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
@@ -50,7 +50,7 @@ static void mdp4_dtv_encoder_mode_set(struct drm_encoder 
*encoder,
 
DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
 
-   mdp4_dtv_encoder->pixclock = mode->clock * 1000;
+   mdp4_dtv_encoder->pixclock = mode->clock * 1000U;
 
DBG("pixclock=%lu", mdp4_dtv_encoder->pixclock);
 
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
index 10eb3e5b218e..d90dc0a39855 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
@@ -225,7 +225,7 @@ static void mdp4_lcdc_encoder_mode_set(struct drm_encoder 
*encoder,
 
DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
 
-   mdp4_lcdc_encoder->pixclock = mode->clock * 1000;
+   mdp4_lcdc_encoder->pixclock = mode->clock * 1000U;
 
DBG("pixclock=%lu", mdp4_lcdc_encoder->pixclock);
 
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
index 7288041dd86a..a965e7962a7f 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
@@ -64,7 +64,7 @@ static int mdp4_lvds_connector_mode_valid(struct 
drm_connector *connector,
struct drm_encoder *encoder = mdp4_lvds_connector->encoder;
long actual, requested;
 
-   requested = 1000 * mode->clock;
+   requested = 1000U * mode->clock;
actual = mdp4_lcdc_round_pixclk(encoder, requested);
 
DBG("requested=%ld, actual=%ld", requested, actual);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 62e75dc8afc6..6babeb79aeb0 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1316,7 +1316,7 @@ static int dp_ctrl_enable_mainlink_clocks(struct 
dp_ctrl_private *ctrl)
opts_dp->lanes = ctrl->link->link_params.num_lanes;
opts_dp->link_rate = ctrl->link->link_params.rate / 100;
dp_ctrl_set_clock_rate(ctrl, DP_CTRL_PM, "ctrl_link",
-   ctrl->link->link_params.rate * 1000);
+   ctrl->link->link_params.rate * 1000U);
 
phy_configure(phy, _io->phy_opts);
phy_power_on(phy);
@@ -1336,7 +1336,7 @@ static int dp_ctrl_enable_stream_clocks(struct 
dp_ctrl_private *ctrl)
int ret = 0;
 
dp_ctrl_set_clock_rate(ctrl, DP_STREAM_PM, "stream_pixel",
-   ctrl->dp_ctrl.pixel_rate * 1000);
+   ctrl->dp_ctrl.pixel_rate * 1000U);
 
ret = dp_power_clk_enable(ctrl->power, DP_STREAM_PM, true);
if (ret)
diff --git a/drivers/gpu/drm/msm/edp/edp_connector.c 
b/drivers/gpu/drm/msm/edp/edp_connector.c
index 73cb5fd97a5a..837e7873141f 100644
--- a/drivers/gpu/drm/msm/edp/edp_connector.c
+++ b/drivers/gpu/drm/msm/edp/edp_connector.c
@@ -64,7 +64,7 @@ static int edp_connector_mode_valid(struct drm_connector 
*connector,
struct msm_kms *kms = priv->kms;
long actual, requested;
 
-   requested = 1000 * mode->clock;
+   requested = 1000L * mode->clock;
actual = kms->funcs->round_pixclk(kms,
requested, edp_connector->edp->encoder);
 
diff --git 

[Freedreno] NAK: [PATCH] drm/msm/mdp4: Fix potential integer overflow on 32 bit multiply

2021-09-29 Thread Colin Ian King

On 29/09/2021 12:08, Colin King wrote:

From: Colin Ian King 

In the case where clock is 2147485 or greater the 32 bit multiplication
by 1000 will cause an integer overflow. Fix this by making the constant
1000 a long to ensure a long multiply occurs to avoid the overflow
before assigning the result to the long result in variable requested.
Most probably a theoretical overflow issue, but worth fixing.

Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 3e87599b68e7 ("drm/msm/mdp4: add LVDS panel support")
Signed-off-by: Colin Ian King 
---
  drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
index 7288041dd86a..deada745d5b9 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
@@ -64,7 +64,7 @@ static int mdp4_lvds_connector_mode_valid(struct 
drm_connector *connector,
struct drm_encoder *encoder = mdp4_lvds_connector->encoder;
long actual, requested;
  
-	requested = 1000 * mode->clock;

+   requested = 1000L * mode->clock;
actual = mdp4_lcdc_round_pixclk(encoder, requested);
  
  	DBG("requested=%ld, actual=%ld", requested, actual);




NACK: there are a few more occurrences of this in the msm driver, I'll 
fix them up for a V2.





[Freedreno] [PATCH] drm/msm/mdp4: Fix potential integer overflow on 32 bit multiply

2021-09-29 Thread Colin King
From: Colin Ian King 

In the case where clock is 2147485 or greater the 32 bit multiplication
by 1000 will cause an integer overflow. Fix this by making the constant
1000 a long to ensure a long multiply occurs to avoid the overflow
before assigning the result to the long result in variable requested.
Most probably a theoretical overflow issue, but worth fixing.

Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 3e87599b68e7 ("drm/msm/mdp4: add LVDS panel support")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
index 7288041dd86a..deada745d5b9 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
@@ -64,7 +64,7 @@ static int mdp4_lvds_connector_mode_valid(struct 
drm_connector *connector,
struct drm_encoder *encoder = mdp4_lvds_connector->encoder;
long actual, requested;
 
-   requested = 1000 * mode->clock;
+   requested = 1000L * mode->clock;
actual = mdp4_lcdc_round_pixclk(encoder, requested);
 
DBG("requested=%ld, actual=%ld", requested, actual);
-- 
2.32.0



[Freedreno] [PATCH] drm/msm/dp: Remove redundant initialization of variable bpp

2021-09-29 Thread Colin King
From: Colin Ian King 

The variable bpp is being initialized with a value that is never
read, it is being updated later on in both paths of an if statement.
The assignment is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 2181b60e1d1d..71db10c0f262 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -234,7 +234,7 @@ u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel,
u32 mode_edid_bpp, u32 mode_pclk_khz)
 {
struct dp_panel_private *panel;
-   u32 bpp = mode_edid_bpp;
+   u32 bpp;
 
if (!dp_panel || !mode_edid_bpp || !mode_pclk_khz) {
DRM_ERROR("invalid input\n");
-- 
2.32.0



Re: [Freedreno] [PATCH] [RFC] qcom_scm: hide Kconfig symbol

2021-09-29 Thread Arnd Bergmann
On Wed, Sep 29, 2021 at 11:51 AM Will Deacon  wrote:
> On Mon, Sep 27, 2021 at 05:22:13PM +0200, Arnd Bergmann wrote:
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 124c41adeca1..989c83acbfee 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -308,7 +308,7 @@ config APPLE_DART
> >  config ARM_SMMU
> >   tristate "ARM Ltd. System MMU (SMMU) Support"
> >   depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> > - depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> > + select QCOM_SCM
> >   select IOMMU_API
> >   select IOMMU_IO_PGTABLE_LPAE
> >   select ARM_DMA_USE_IOMMU if ARM
>
> I don't want to get in the way of this patch because I'm also tired of the
> randconfig failures caused by QCOM_SCM. However, ARM_SMMU is applicable to
> a wide variety of (non-qcom) SoCs and so it seems a shame to require the
> QCOM_SCM code to be included for all of those when it's not strictly needed
> at all.

Good point, I agree that needs to be fixed. I think this additional
change should do the trick:

--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -308,7 +308,6 @@ config APPLE_DART
 config ARM_SMMU
tristate "ARM Ltd. System MMU (SMMU) Support"
depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
-   select QCOM_SCM
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU if ARM
@@ -438,7 +437,7 @@ config QCOM_IOMMU
# Note: iommu drivers cannot (yet?) be built as modules
bool "Qualcomm IOMMU Support"
depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64)
-   depends on QCOM_SCM=y
+   select QCOM_SCM
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU

I'll see if that causes any problems for the randconfig builds.

   Arnd


Re: [Freedreno] [PATCH] [RFC] qcom_scm: hide Kconfig symbol

2021-09-29 Thread Will Deacon
On Mon, Sep 27, 2021 at 05:22:13PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> Now that SCM can be a loadable module, we have to add another
> dependency to avoid link failures when ipa or adreno-gpu are
> built-in:
> 
> aarch64-linux-ld: drivers/net/ipa/ipa_main.o: in function `ipa_probe':
> ipa_main.c:(.text+0xfc4): undefined reference to `qcom_scm_is_available'
> 
> ld.lld: error: undefined symbol: qcom_scm_is_available
> >>> referenced by adreno_gpu.c
> >>>   gpu/drm/msm/adreno/adreno_gpu.o:(adreno_zap_shader_load) in 
> >>> archive drivers/built-in.a
> 
> This can happen when CONFIG_ARCH_QCOM is disabled and we don't select
> QCOM_MDT_LOADER, but some other module selects QCOM_SCM. Ideally we'd
> use a similar dependency here to what we have for QCOM_RPROC_COMMON,
> but that causes dependency loops from other things selecting QCOM_SCM.
> 
> This appears to be an endless problem, so try something different this
> time:
> 
>  - CONFIG_QCOM_SCM becomes a hidden symbol that nothing 'depends on'
>but that is simply selected by all of its users
> 
>  - All the stubs in include/linux/qcom_scm.h can go away
> 
>  - arm-smccc.h needs to provide a stub for __arm_smccc_smc() to
>allow compile-testing QCOM_SCM on all architectures.
> 
>  - To avoid a circular dependency chain involving RESET_CONTROLLER
>and PINCTRL_SUNXI, change the 'depends on RESET_CONTROLLER' in
>the latter one to 'select'.
> 
> The last bit is rather annoying, as drivers should generally never
> 'select' another subsystem, and about half the users of the reset
> controller interface do this anyway.
> 
> Nevertheless, this version seems to pass all my randconfig tests
> and is more robust than any of the prior versions.
> 
> Comments?
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/firmware/Kconfig|  4 +-
>  drivers/gpu/drm/msm/Kconfig |  4 +-
>  drivers/iommu/Kconfig   |  2 +-
>  drivers/media/platform/Kconfig  |  2 +-
>  drivers/mmc/host/Kconfig|  2 +-
>  drivers/net/ipa/Kconfig |  1 +
>  drivers/net/wireless/ath/ath10k/Kconfig |  2 +-
>  drivers/pinctrl/qcom/Kconfig|  3 +-
>  drivers/pinctrl/sunxi/Kconfig   |  6 +--
>  include/linux/arm-smccc.h   | 10 
>  include/linux/qcom_scm.h| 71 -
>  11 files changed, 23 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 220a58cf0a44..f7dd82ef0b9c 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -203,9 +203,7 @@ config INTEL_STRATIX10_RSU
> Say Y here if you want Intel RSU support.
>  
>  config QCOM_SCM
> - tristate "Qcom SCM driver"
> - depends on ARM || ARM64
> - depends on HAVE_ARM_SMCCC
> + tristate
>   select RESET_CONTROLLER
>  
>  config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index e9c6af78b1d7..3ddf739a6f9b 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -17,7 +17,7 @@ config DRM_MSM
>   select DRM_SCHED
>   select SHMEM
>   select TMPFS
> - select QCOM_SCM if ARCH_QCOM
> + select QCOM_SCM
>   select WANT_DEV_COREDUMP
>   select SND_SOC_HDMI_CODEC if SND_SOC
>   select SYNC_FILE
> @@ -55,7 +55,7 @@ config DRM_MSM_GPU_SUDO
>  
>  config DRM_MSM_HDMI_HDCP
>   bool "Enable HDMI HDCP support in MSM DRM driver"
> - depends on DRM_MSM && QCOM_SCM
> + depends on DRM_MSM
>   default y
>   help
> Choose this option to enable HDCP state machine
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 124c41adeca1..989c83acbfee 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -308,7 +308,7 @@ config APPLE_DART
>  config ARM_SMMU
>   tristate "ARM Ltd. System MMU (SMMU) Support"
>   depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> - depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> + select QCOM_SCM
>   select IOMMU_API
>   select IOMMU_IO_PGTABLE_LPAE
>   select ARM_DMA_USE_IOMMU if ARM

I don't want to get in the way of this patch because I'm also tired of the
randconfig failures caused by QCOM_SCM. However, ARM_SMMU is applicable to
a wide variety of (non-qcom) SoCs and so it seems a shame to require the
QCOM_SCM code to be included for all of those when it's not strictly needed
at all.

Will