Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: Fix timeout issues on command mode panels

2021-09-10 Thread Marijn Suijten
Hi Angelo!

On 2021-09-01 19:43:47, AngeloGioacchino Del Regno wrote:
> In function dpu_encoder_phys_cmd_wait_for_commit_done we are always
> checking if the relative CTL is started by waiting for an interrupt
> to fire: it is fine to do that, but then sometimes we call this
> function while the CTL is up and has never been put down, but that
> interrupt gets raised only when the CTL gets a state change from
> 0 to 1 (disabled to enabled), so we're going to wait for something
> that will never happen on its own.
> 
> Solving this while avoiding to restart the CTL is actually possible
> and can be done by just checking if it is already up and running
> when the wait_for_commit_done function is called: in this case, so,
> if the CTL was already running, we can say that the commit is done
> if the command transmission is complete (in other terms, if the
> interface has been flushed).
> 
> Signed-off-by: AngeloGioacchino Del Regno 
> 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index aa01698d6b25..b5b1b555ac4e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -682,6 +682,9 @@ static int dpu_encoder_phys_cmd_wait_for_commit_done(
>   if (!dpu_encoder_phys_cmd_is_master(phys_enc))
>   return 0;
>  
> + if (phys_enc->hw_ctl->ops.is_started)
> + return dpu_encoder_phys_cmd_wait_for_tx_complete(phys_enc);

In the previous commit you introduced is_started to the ops struct as
function pointer, and you probably intend to call it here instead of
just checking whether it might be NULL.

As far as I remember this was also the reason for previously mentioning
that it was faulty and required a v2 in:
https://lore.kernel.org/linux-arm-msm/bdc67afc-3736-5497-c43f-5165c55e0...@somainline.org/

Thanks!

- Marijn

> +
>   return _dpu_encoder_phys_cmd_wait_for_ctl_start(phys_enc);
>  }
>  
> -- 
> 2.32.0
> 


Re: [Freedreno] [PATCH] drm/msm: Disable frequency clamping on a630

2021-09-10 Thread Caleb Connolly




On 10/09/2021 18:18, Rob Clark wrote:

On Tue, Sep 7, 2021 at 7:20 PM Bjorn Andersson
 wrote:


On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:


On 8/9/2021 9:48 PM, Caleb Connolly wrote:



On 09/08/2021 17:12, Rob Clark wrote:

On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
 wrote:

[..]

I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?

The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.

The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min state to
the max state and back again extremely rapidly under workloads as small
as refreshing UI. Where previously the GPU would rarely if ever go above
342MHz when interacting with the device, it now jumps between min and
max many times per second.

If my understanding is correct, the current implementation of the GMU
set freq is the following:
   - Get OPP for frequency to set
   - Push the frequency to the GMU - immediately updating the core clock
   - Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
up somewhere in power management code and causes the gx regulator level
to be updated


Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().



On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
I'm lucky I managed to hit a few keys before it crashes, so I spent a
few hours looking into this as well...

As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
The opp-level is just there for show and isn't used by anything, at
least not on 845.

Further more, I'm missing something in my tree, so the interconnect
doesn't hit sync_state, and as such we're not actually scaling the
buses. So the problem is not that Linux doesn't turn on the buses in
time.

So I suspect that the "AHB bus error" isn't saying that we turned off
the bus, but rather that the GPU becomes unstable or something of that
sort.


Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
Aquarium for 20 minutes without a problem. I then switched the gpu
devfreq governor to "userspace" and ran the following:

while true; do
   echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
   echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
done

It took 19 iterations of this loop to crash the GPU.


I assume you still had aquarium running, to keep the gpu awake while
you ran that loop?

Fwiw, I modified this slightly to match sc7180's min/max gpu freq and
could not trigger any issue.. interestingly sc7180 has a lower min
freq (180) and higher max freq (800) so it was toggling over a wider
freq range.  I also tried on a device that  had the higher 825MHz opp
(since I noticed that was the only opp that used
RPMH_REGULATOR_LEVEL_TURBO_L1 and wanted to rule that out), but could
not reproduce.

I guess a630 (sdm845) should have higher power draw (it is 2x # of
shader cores and 2x GMEM size, but lower max freq).. the question is,
is this the reason we see this on sdm845 and not sc7180?  Or is there
some other difference.  On the gpu side of this, they are both closely
related (ie. the same "sub-generation" of a6xx, same gmu fw, etc)..
I'm less sure about the other parts (icc, rpmh, etc)


My guess would be power draw, nobody has mentioned this yet but I've realised that the vdd_gfx rail is powered by a buck 
converter, which could explain a lot of the symptoms.


Buck converters depend on high frequency switching and inductors to work, this inherently leads to some lag time when 
changing voltages, and also means that the behaviour of the regulator is defined in part by how much current is being 
drawn. Wikipedia has a pretty good explanation: https://en.wikipedia.org/wiki/Buck_converter


At the best of times these regulators have a known voltage ripple, when under load and when rapidly switching voltages 
this will get a lot worse.


Someone with an oscilloscope and schematics could probe the rail and probably see exactly what's going on when the GPU 
crashes. Because of the lag time in the regulator changing voltage, it might be undershooting whilst the GPU is trying 
to clock up and draw more current - causing instability and crashes.


BR,
-R


So the problem doesn't seem to be Rob's change, it's just that prior to
it the chance to hitting it is way lower. Question is still what it is
that we're triggering.

Re: [Freedreno] [PATCH] drm/msm: Disable frequency clamping on a630

2021-09-10 Thread Rob Clark
On Thu, Sep 9, 2021 at 1:54 PM Rob Clark  wrote:
>
> On Thu, Sep 9, 2021 at 12:50 PM Akhil P Oommen  wrote:
> >
> > On 9/9/2021 9:42 PM, Amit Pundir wrote:
> > > On Thu, 9 Sept 2021 at 17:47, Amit Pundir  wrote:
> > >>
> > >> On Wed, 8 Sept 2021 at 07:50, Bjorn Andersson
> > >>  wrote:
> > >>>
> > >>> On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
> > >>>
> >  On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > >
> > >
> > > On 09/08/2021 17:12, Rob Clark wrote:
> > >> On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > >>  wrote:
> > >>> [..]
> > >>> I am a bit confused. We don't define a power domain for gpu in dt,
> > >>> correct? Then what exactly set_opp do here? Do you think this 
> > >>> usleep is
> > >>> what is helping here somehow to mask the issue?
> > > The power domains (for cx and gx) are defined in the GMU DT, the OPPs 
> > > in
> > > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > > the "min" state, and the highest frequency (71000) and OPP level
> > > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined 
> > > in
> > > sdm845.dtsi under the gpu node.
> > >
> > > The new devfreq behaviour unmasks what I think is a driver bug, it
> > > inadvertently puts much more strain on the GPU regulators than they
> > > usually get. With the new behaviour the GPU jumps from it's min state 
> > > to
> > > the max state and back again extremely rapidly under workloads as 
> > > small
> > > as refreshing UI. Where previously the GPU would rarely if ever go 
> > > above
> > > 342MHz when interacting with the device, it now jumps between min and
> > > max many times per second.
> > >
> > > If my understanding is correct, the current implementation of the GMU
> > > set freq is the following:
> > >- Get OPP for frequency to set
> > >- Push the frequency to the GMU - immediately updating the core 
> > > clock
> > >- Call dev_pm_opp_set_opp() which triggers a notify chain, this 
> > > winds
> > > up somewhere in power management code and causes the gx regulator 
> > > level
> > > to be updated
> > 
> >  Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing 
> >  else. We
> >  were using a different api earlier which got deprecated -
> >  dev_pm_opp_set_bw().
> > 
> > >>>
> > >>> On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> > >>> I'm lucky I managed to hit a few keys before it crashes, so I spent a
> > >>> few hours looking into this as well...
> > >>>
> > >>> As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> > >>> The opp-level is just there for show and isn't used by anything, at
> > >>> least not on 845.
> > >>>
> > >>> Further more, I'm missing something in my tree, so the interconnect
> > >>> doesn't hit sync_state, and as such we're not actually scaling the
> > >>> buses. So the problem is not that Linux doesn't turn on the buses in
> > >>> time.
> > >>>
> > >>> So I suspect that the "AHB bus error" isn't saying that we turned off
> > >>> the bus, but rather that the GPU becomes unstable or something of that
> > >>> sort.
> > >>>
> > >>>
> > >>> Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> > >>> Aquarium for 20 minutes without a problem. I then switched the gpu
> > >>> devfreq governor to "userspace" and ran the following:
> > >>>
> > >>> while true; do
> > >>>echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
> > >>>echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> > >>> done
> > >>>
> > >>> It took 19 iterations of this loop to crash the GPU.
> > >>
> > >> Ack. With your above script, I can reproduce a crash too on db845c
> > >> (A630) running v5.14. I didn't get any crash log though and device
> > >> just rebooted to USB crash mode.
> > >>
> > >> And same crash on RB5 (A650) too https://hastebin.com/raw/ejutetuwun
> >
> > Are we sure this is the same issue? It could be, but I thought we were
> > seeing a bunch of random gpu errors (which may eventually hit device crash).
>
> In the sense that async-serror often seems to be a clk issue, it
> *could* be related.. but this would have to be triggered by CPU
> access.  The symptom does seem very different.
>

The more I think about it, the more I think this is a different
issue.. a650 is somewhat different wrt gmu (ie. hfi vs legacy code
paths).

Amit, could you try the same experiment (with 9bc95570175a ("drm/msm:
Devfreq tuning") revert) while running something like webgl aquarium
to prevent the GPU from suspending?  I'm kinda suspecting the issue
you hit is more likely some suspend/resume issue.

BR,
-R


Re: [Freedreno] [PATCH] drm/msm: Disable frequency clamping on a630

2021-09-10 Thread Rob Clark
On Tue, Sep 7, 2021 at 7:20 PM Bjorn Andersson
 wrote:
>
> On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
>
> > On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > >
> > >
> > > On 09/08/2021 17:12, Rob Clark wrote:
> > > > On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > > >  wrote:
> [..]
> > > > > I am a bit confused. We don't define a power domain for gpu in dt,
> > > > > correct? Then what exactly set_opp do here? Do you think this usleep 
> > > > > is
> > > > > what is helping here somehow to mask the issue?
> > > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > > the "min" state, and the highest frequency (71000) and OPP level
> > > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
> > > sdm845.dtsi under the gpu node.
> > >
> > > The new devfreq behaviour unmasks what I think is a driver bug, it
> > > inadvertently puts much more strain on the GPU regulators than they
> > > usually get. With the new behaviour the GPU jumps from it's min state to
> > > the max state and back again extremely rapidly under workloads as small
> > > as refreshing UI. Where previously the GPU would rarely if ever go above
> > > 342MHz when interacting with the device, it now jumps between min and
> > > max many times per second.
> > >
> > > If my understanding is correct, the current implementation of the GMU
> > > set freq is the following:
> > >   - Get OPP for frequency to set
> > >   - Push the frequency to the GMU - immediately updating the core clock
> > >   - Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
> > > up somewhere in power management code and causes the gx regulator level
> > > to be updated
> >
> > Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
> > were using a different api earlier which got deprecated -
> > dev_pm_opp_set_bw().
> >
>
> On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> I'm lucky I managed to hit a few keys before it crashes, so I spent a
> few hours looking into this as well...
>
> As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> The opp-level is just there for show and isn't used by anything, at
> least not on 845.
>
> Further more, I'm missing something in my tree, so the interconnect
> doesn't hit sync_state, and as such we're not actually scaling the
> buses. So the problem is not that Linux doesn't turn on the buses in
> time.
>
> So I suspect that the "AHB bus error" isn't saying that we turned off
> the bus, but rather that the GPU becomes unstable or something of that
> sort.
>
>
> Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> Aquarium for 20 minutes without a problem. I then switched the gpu
> devfreq governor to "userspace" and ran the following:
>
> while true; do
>   echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
>   echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> done
>
> It took 19 iterations of this loop to crash the GPU.

I assume you still had aquarium running, to keep the gpu awake while
you ran that loop?

Fwiw, I modified this slightly to match sc7180's min/max gpu freq and
could not trigger any issue.. interestingly sc7180 has a lower min
freq (180) and higher max freq (800) so it was toggling over a wider
freq range.  I also tried on a device that  had the higher 825MHz opp
(since I noticed that was the only opp that used
RPMH_REGULATOR_LEVEL_TURBO_L1 and wanted to rule that out), but could
not reproduce.

I guess a630 (sdm845) should have higher power draw (it is 2x # of
shader cores and 2x GMEM size, but lower max freq).. the question is,
is this the reason we see this on sdm845 and not sc7180?  Or is there
some other difference.  On the gpu side of this, they are both closely
related (ie. the same "sub-generation" of a6xx, same gmu fw, etc)..
I'm less sure about the other parts (icc, rpmh, etc)

BR,
-R

> So the problem doesn't seem to be Rob's change, it's just that prior to
> it the chance to hitting it is way lower. Question is still what it is
> that we're triggering.
>
> Regards,
> Bjorn


Re: [Freedreno] [PATCH] dt-bindings: More use 'enum' instead of 'oneOf' plus 'const' entries

2021-09-10 Thread Guenter Roeck

On 9/10/21 9:51 AM, Rob Herring wrote:

'enum' is equivalent to 'oneOf' with a list of 'const' entries, but 'enum'
is more concise and yields better error messages.

Fix a couple more cases which have appeared.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: Mark Brown 
Cc: Wim Van Sebroeck 
Cc: Guenter Roeck 
Cc: Jonathan Marek 
Cc: Aswath Govindraju 
Cc: Marc Zyngier 
Cc: Linus Walleij 
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-...@vger.kernel.org
Cc: linux-watch...@vger.kernel.org
Signed-off-by: Rob Herring 
---
  .../bindings/display/msm/dsi-phy-7nm.yaml  |  8 
  .../devicetree/bindings/spi/omap-spi.yaml  |  6 +++---
  .../bindings/watchdog/maxim,max63xx.yaml   | 14 +++---


For watchdog:

Acked-by: Guenter Roeck 


  3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml 
b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
index 4265399bb154..c851770bbdf2 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
@@ -14,10 +14,10 @@ allOf:
  
  properties:

compatible:
-oneOf:
-  - const: qcom,dsi-phy-7nm
-  - const: qcom,dsi-phy-7nm-8150
-  - const: qcom,sc7280-dsi-phy-7nm
+enum:
+  - qcom,dsi-phy-7nm
+  - qcom,dsi-phy-7nm-8150
+  - qcom,sc7280-dsi-phy-7nm
  
reg:

  items:
diff --git a/Documentation/devicetree/bindings/spi/omap-spi.yaml 
b/Documentation/devicetree/bindings/spi/omap-spi.yaml
index e55538186cf6..9952199cae11 100644
--- a/Documentation/devicetree/bindings/spi/omap-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/omap-spi.yaml
@@ -84,9 +84,9 @@ unevaluatedProperties: false
  if:
properties:
  compatible:
-  oneOf:
-- const: ti,omap2-mcspi
-- const: ti,omap4-mcspi
+  enum:
+- ti,omap2-mcspi
+- ti,omap4-mcspi
  
  then:

properties:
diff --git a/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml 
b/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml
index f2105eedac2c..ab9641e845db 100644
--- a/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml
+++ b/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml
@@ -15,13 +15,13 @@ maintainers:
  
  properties:

compatible:
-oneOf:
-  - const: maxim,max6369
-  - const: maxim,max6370
-  - const: maxim,max6371
-  - const: maxim,max6372
-  - const: maxim,max6373
-  - const: maxim,max6374
+enum:
+  - maxim,max6369
+  - maxim,max6370
+  - maxim,max6371
+  - maxim,max6372
+  - maxim,max6373
+  - maxim,max6374
  
reg:

  description: This is a 1-byte memory-mapped address





Re: [Freedreno] [PATCH] dt-bindings: More use 'enum' instead of 'oneOf' plus 'const' entries

2021-09-10 Thread Mark Brown
On Fri, Sep 10, 2021 at 11:51:53AM -0500, Rob Herring wrote:

> 'enum' is equivalent to 'oneOf' with a list of 'const' entries, but 'enum'
> is more concise and yields better error messages.

Acked-by: Mark Brown 


signature.asc
Description: PGP signature


[Freedreno] [PATCH] dt-bindings: More use 'enum' instead of 'oneOf' plus 'const' entries

2021-09-10 Thread Rob Herring
'enum' is equivalent to 'oneOf' with a list of 'const' entries, but 'enum'
is more concise and yields better error messages.

Fix a couple more cases which have appeared.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: Mark Brown 
Cc: Wim Van Sebroeck 
Cc: Guenter Roeck 
Cc: Jonathan Marek 
Cc: Aswath Govindraju 
Cc: Marc Zyngier 
Cc: Linus Walleij 
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-...@vger.kernel.org
Cc: linux-watch...@vger.kernel.org
Signed-off-by: Rob Herring 
---
 .../bindings/display/msm/dsi-phy-7nm.yaml  |  8 
 .../devicetree/bindings/spi/omap-spi.yaml  |  6 +++---
 .../bindings/watchdog/maxim,max63xx.yaml   | 14 +++---
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml 
b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
index 4265399bb154..c851770bbdf2 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
@@ -14,10 +14,10 @@ allOf:
 
 properties:
   compatible:
-oneOf:
-  - const: qcom,dsi-phy-7nm
-  - const: qcom,dsi-phy-7nm-8150
-  - const: qcom,sc7280-dsi-phy-7nm
+enum:
+  - qcom,dsi-phy-7nm
+  - qcom,dsi-phy-7nm-8150
+  - qcom,sc7280-dsi-phy-7nm
 
   reg:
 items:
diff --git a/Documentation/devicetree/bindings/spi/omap-spi.yaml 
b/Documentation/devicetree/bindings/spi/omap-spi.yaml
index e55538186cf6..9952199cae11 100644
--- a/Documentation/devicetree/bindings/spi/omap-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/omap-spi.yaml
@@ -84,9 +84,9 @@ unevaluatedProperties: false
 if:
   properties:
 compatible:
-  oneOf:
-- const: ti,omap2-mcspi
-- const: ti,omap4-mcspi
+  enum:
+- ti,omap2-mcspi
+- ti,omap4-mcspi
 
 then:
   properties:
diff --git a/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml 
b/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml
index f2105eedac2c..ab9641e845db 100644
--- a/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml
+++ b/Documentation/devicetree/bindings/watchdog/maxim,max63xx.yaml
@@ -15,13 +15,13 @@ maintainers:
 
 properties:
   compatible:
-oneOf:
-  - const: maxim,max6369
-  - const: maxim,max6370
-  - const: maxim,max6371
-  - const: maxim,max6372
-  - const: maxim,max6373
-  - const: maxim,max6374
+enum:
+  - maxim,max6369
+  - maxim,max6370
+  - maxim,max6371
+  - maxim,max6372
+  - maxim,max6373
+  - maxim,max6374
 
   reg:
 description: This is a 1-byte memory-mapped address
-- 
2.30.2



[Freedreno] [PATCH v4 21/24] drm/bridge: tc358775: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device. This also avoids leaking the device when we detach
the bridge.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/tc358775.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c 
b/drivers/gpu/drm/bridge/tc358775.c
index 2272adcc5b4a..35e66d1b6456 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -610,11 +610,10 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
return -EPROBE_DEFER;
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
dev_err(dev, "failed to create dsi device\n");
-   ret = PTR_ERR(dsi);
-   goto err_dsi_device;
+   return PTR_ERR(dsi);
}
 
tc->dsi = dsi;
@@ -623,19 +622,15 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
dev_err(dev, "failed to attach dsi to host\n");
-   goto err_dsi_attach;
+   return ret;
}
 
/* Attach the panel-bridge to the dsi bridge */
return drm_bridge_attach(bridge->encoder, tc->panel_bridge,
 &tc->bridge, flags);
-err_dsi_attach:
-   mipi_dsi_device_unregister(dsi);
-err_dsi_device:
-   return ret;
 }
 
 static const struct drm_bridge_funcs tc_bridge_funcs = {
-- 
2.31.1



[Freedreno] [PATCH v4 16/24] drm/bridge: ps8640: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/parade-ps8640.c | 97 +++---
 1 file changed, 55 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c 
b/drivers/gpu/drm/bridge/parade-ps8640.c
index c943045f3370..8d161b6cdbb2 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -215,52 +215,10 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
 {
struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
-   struct device *dev = &ps_bridge->page[0]->dev;
-   struct device_node *in_ep, *dsi_node;
-   struct mipi_dsi_device *dsi;
-   struct mipi_dsi_host *host;
-   int ret;
-   const struct mipi_dsi_device_info info = { .type = "ps8640",
-  .channel = 0,
-  .node = NULL,
-};
 
if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
return -EINVAL;
 
-   /* port@0 is ps8640 dsi input port */
-   in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
-   if (!in_ep)
-   return -ENODEV;
-
-   dsi_node = of_graph_get_remote_port_parent(in_ep);
-   of_node_put(in_ep);
-   if (!dsi_node)
-   return -ENODEV;
-
-   host = of_find_mipi_dsi_host_by_node(dsi_node);
-   of_node_put(dsi_node);
-   if (!host)
-   return -ENODEV;
-
-   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
-   if (IS_ERR(dsi)) {
-   dev_err(dev, "failed to create dsi device\n");
-   ret = PTR_ERR(dsi);
-   return ret;
-   }
-
-   ps_bridge->dsi = dsi;
-
-   dsi->host = host;
-   dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
- MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
-   dsi->format = MIPI_DSI_FMT_RGB888;
-   dsi->lanes = NUM_MIPI_LANES;
-   ret = devm_mipi_dsi_attach(dev, dsi);
-   if (ret)
-   return ret;
-
/* Attach the panel-bridge to the dsi bridge */
return drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge,
 &ps_bridge->bridge, flags);
@@ -307,6 +265,53 @@ static const struct drm_bridge_funcs ps8640_bridge_funcs = 
{
.pre_enable = ps8640_pre_enable,
 };
 
+static int ps8640_bridge_host_attach(struct device *dev, struct ps8640 
*ps_bridge)
+{
+   struct device_node *in_ep, *dsi_node;
+   struct mipi_dsi_device *dsi;
+   struct mipi_dsi_host *host;
+   int ret;
+   const struct mipi_dsi_device_info info = { .type = "ps8640",
+  .channel = 0,
+  .node = NULL,
+};
+
+   /* port@0 is ps8640 dsi input port */
+   in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
+   if (!in_ep)
+   return -ENODEV;
+
+   dsi_node = of_graph_get_remote_port_parent(in_ep);
+   of_node_put(in_ep);
+   if (!dsi_node)
+   return -ENODEV;
+
+   host = of_find_mipi_dsi_host_by_node(dsi_node);
+   of_node_put(dsi_node);
+   if (!host)
+   return -EPROBE_DEFER;
+
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
+   if (IS_ERR(dsi)) {
+   dev_err(dev, "failed to create dsi device\n");
+   return PTR_ERR(dsi);
+   }
+
+   ps_bridge->dsi = dsi;
+
+   dsi->host = host;
+   dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
+ MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+   dsi->format = MIPI_DSI_FMT_RGB888;
+   dsi->lanes = NUM_MIPI_LANES;
+
+   ret = devm_mipi_dsi_attach(dev, dsi);
+   if (ret)
+   return ret;
+
+   return 0;
+}
+
 static int ps8640_probe(struct i2c_client *client)
 {
struct device *dev = &client->dev;
@@ -373,7 +378,15 @@ static int ps8640_probe(struct i2c_client *client)
 
drm_bridge_add(&ps_bridge->bridge);
 
+   ret = ps8640_bridge_host_attach(dev, ps_bridge);
+   if (ret)
+   goto err_bridge_remove;
+
return 0;
+
+err_bridge_remove:
+   drm_bridge_remove(&ps_bridge->bridge);
+   return ret;
 }
 
 static int ps8640_remove(struct i2c_client *client)
-- 
2.31.1



[Freedreno] [PATCH v4 15/24] drm/bridge: ps8640: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device. This also avoids leaking the device on removal.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/parade-ps8640.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c 
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 685e9c38b2db..c943045f3370 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -243,7 +243,7 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
if (!host)
return -ENODEV;
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
dev_err(dev, "failed to create dsi device\n");
ret = PTR_ERR(dsi);
@@ -257,17 +257,13 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
  MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->lanes = NUM_MIPI_LANES;
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret)
-   goto err_dsi_attach;
+   return ret;
 
/* Attach the panel-bridge to the dsi bridge */
return drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge,
 &ps_bridge->bridge, flags);
-
-err_dsi_attach:
-   mipi_dsi_device_unregister(dsi);
-   return ret;
 }
 
 static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
-- 
2.31.1



[Freedreno] [PATCH v4 11/24] drm/bridge: lt9611: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 24 
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c 
b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 29b1ce2140ab..654131aca5ed 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -760,6 +760,7 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct 
lt9611 *lt9611,
const struct mipi_dsi_device_info info = { "lt9611", 0, NULL };
struct mipi_dsi_device *dsi;
struct mipi_dsi_host *host;
+   struct device *dev = lt9611->dev;
int ret;
 
host = of_find_mipi_dsi_host_by_node(dsi_node);
@@ -768,7 +769,7 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct 
lt9611 *lt9611,
return ERR_PTR(-EPROBE_DEFER);
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
dev_err(lt9611->dev, "failed to create dsi device\n");
return dsi;
@@ -779,29 +780,15 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct 
lt9611 *lt9611,
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
  MIPI_DSI_MODE_VIDEO_HSE;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
-   dev_err(lt9611->dev, "failed to attach dsi to host\n");
-   mipi_dsi_device_unregister(dsi);
+   dev_err(dev, "failed to attach dsi to host\n");
return ERR_PTR(ret);
}
 
return dsi;
 }
 
-static void lt9611_bridge_detach(struct drm_bridge *bridge)
-{
-   struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
-
-   if (lt9611->dsi1) {
-   mipi_dsi_detach(lt9611->dsi1);
-   mipi_dsi_device_unregister(lt9611->dsi1);
-   }
-
-   mipi_dsi_detach(lt9611->dsi0);
-   mipi_dsi_device_unregister(lt9611->dsi0);
-}
-
 static int lt9611_connector_init(struct drm_bridge *bridge, struct lt9611 
*lt9611)
 {
int ret;
@@ -855,9 +842,7 @@ static int lt9611_bridge_attach(struct drm_bridge *bridge,
return 0;
 
 err_unregister_dsi0:
-   lt9611_bridge_detach(bridge);
drm_connector_cleanup(<9611->connector);
-   mipi_dsi_device_unregister(lt9611->dsi0);
 
return ret;
 }
@@ -952,7 +937,6 @@ static void lt9611_bridge_hpd_enable(struct drm_bridge 
*bridge)
 
 static const struct drm_bridge_funcs lt9611_bridge_funcs = {
.attach = lt9611_bridge_attach,
-   .detach = lt9611_bridge_detach,
.mode_valid = lt9611_bridge_mode_valid,
.enable = lt9611_bridge_enable,
.disable = lt9611_bridge_disable,
-- 
2.31.1



[Freedreno] [PATCH v4 10/24] drm/bridge: lt8912b: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c 
b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index cc968d65936b..c642d1e02b2f 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -544,10 +544,6 @@ static int lt8912_bridge_attach(struct drm_bridge *bridge,
if (ret)
goto error;
 
-   ret = lt8912_attach_dsi(lt);
-   if (ret)
-   goto error;
-
lt->is_attached = true;
 
return 0;
@@ -706,8 +702,15 @@ static int lt8912_probe(struct i2c_client *client,
 
drm_bridge_add(<->bridge);
 
+   ret = lt8912_attach_dsi(lt);
+   if (ret)
+   goto err_attach;
+
return 0;
 
+err_attach:
+   drm_bridge_remove(<->bridge);
+   lt8912_free_i2c(lt);
 err_i2c:
lt8912_put_dt(lt);
 err_dt_parse:
-- 
2.31.1



[Freedreno] [PATCH v4 01/24] drm/bridge: Add documentation sections

2021-09-10 Thread Maxime Ripard
The bridge documentation overview is quite packed already, and we'll add
some more documentation that isn't part of an overview at all.

Let's add some sections to the documentation to separate each bits.

Reviewed-by: Andrzej Hajda 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Maxime Ripard 
---
 Documentation/gpu/drm-kms-helpers.rst |  6 ++
 drivers/gpu/drm/drm_bridge.c  | 14 +-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index 389892f36185..10f8df7aecc0 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -151,6 +151,12 @@ Overview
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
:doc: overview
 
+Display Driver Integration
+--
+
+.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
+   :doc: display driver integration
+
 Bridge Operations
 -
 
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index a8ed66751c2d..baff74ea4a33 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -49,6 +49,15 @@
  * Chaining multiple bridges to the output of a bridge, or the same bridge to
  * the output of different bridges, is not supported.
  *
+ * &drm_bridge, like &drm_panel, aren't &drm_mode_object entities like planes,
+ * CRTCs, encoders or connectors and hence are not visible to userspace. They
+ * just provide additional hooks to get the desired output at the end of the
+ * encoder chain.
+ */
+
+/**
+ * DOC:display driver integration
+ *
  * Display drivers are responsible for linking encoders with the first bridge
  * in the chains. This is done by acquiring the appropriate bridge with
  * of_drm_find_bridge() or drm_of_find_panel_or_bridge(), or creating it for a
@@ -85,11 +94,6 @@
  * helper to create the &drm_connector, or implement it manually on top of the
  * connector-related operations exposed by the bridge (see the overview
  * documentation of bridge operations for more details).
- *
- * &drm_bridge, like &drm_panel, aren't &drm_mode_object entities like planes,
- * CRTCs, encoders or connectors and hence are not visible to userspace. They
- * just provide additional hooks to get the desired output at the end of the
- * encoder chain.
  */
 
 static DEFINE_MUTEX(bridge_lock);
-- 
2.31.1



[Freedreno] [PATCH v4 12/24] drm/bridge: lt9611: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 38 -
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c 
b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 654131aca5ed..d2f45a0f79c8 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -825,26 +825,7 @@ static int lt9611_bridge_attach(struct drm_bridge *bridge,
return ret;
}
 
-   /* Attach primary DSI */
-   lt9611->dsi0 = lt9611_attach_dsi(lt9611, lt9611->dsi0_node);
-   if (IS_ERR(lt9611->dsi0))
-   return PTR_ERR(lt9611->dsi0);
-
-   /* Attach secondary DSI, if specified */
-   if (lt9611->dsi1_node) {
-   lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node);
-   if (IS_ERR(lt9611->dsi1)) {
-   ret = PTR_ERR(lt9611->dsi1);
-   goto err_unregister_dsi0;
-   }
-   }
-
return 0;
-
-err_unregister_dsi0:
-   drm_connector_cleanup(<9611->connector);
-
-   return ret;
 }
 
 static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge,
@@ -1165,10 +1146,29 @@ static int lt9611_probe(struct i2c_client *client,
 
drm_bridge_add(<9611->bridge);
 
+   /* Attach primary DSI */
+   lt9611->dsi0 = lt9611_attach_dsi(lt9611, lt9611->dsi0_node);
+   if (IS_ERR(lt9611->dsi0)) {
+   ret = PTR_ERR(lt9611->dsi0);
+   goto err_remove_bridge;
+   }
+
+   /* Attach secondary DSI, if specified */
+   if (lt9611->dsi1_node) {
+   lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node);
+   if (IS_ERR(lt9611->dsi1)) {
+   ret = PTR_ERR(lt9611->dsi1);
+   goto err_remove_bridge;
+   }
+   }
+
lt9611_enable_hpd_interrupts(lt9611);
 
return lt9611_audio_init(dev, lt9611);
 
+err_remove_bridge:
+   drm_bridge_remove(<9611->bridge);
+
 err_disable_regulators:
regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies);
 
-- 
2.31.1



[Freedreno] [PATCH v4 05/24] drm/bridge: adv7533: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device. This also avoids leaking the device when we detach
the bridge.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h |  1 -
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  2 --
 drivers/gpu/drm/bridge/adv7511/adv7533.c | 20 
 3 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 05e3abb5a0c9..592ecfcf00ca 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -401,7 +401,6 @@ void adv7533_mode_set(struct adv7511 *adv, const struct 
drm_display_mode *mode);
 int adv7533_patch_registers(struct adv7511 *adv);
 int adv7533_patch_cec_registers(struct adv7511 *adv);
 int adv7533_attach_dsi(struct adv7511 *adv);
-void adv7533_detach_dsi(struct adv7511 *adv);
 int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv);
 
 #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 76555ae64e9c..9e3585f23cf1 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1307,8 +1307,6 @@ static int adv7511_remove(struct i2c_client *i2c)
 {
struct adv7511 *adv7511 = i2c_get_clientdata(i2c);
 
-   if (adv7511->type == ADV7533 || adv7511->type == ADV7535)
-   adv7533_detach_dsi(adv7511);
i2c_unregister_device(adv7511->i2c_cec);
clk_disable_unprepare(adv7511->cec_clk);
 
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c 
b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index 59d718bde8c4..eb7579dec40a 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -153,11 +153,10 @@ int adv7533_attach_dsi(struct adv7511 *adv)
return -EPROBE_DEFER;
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
dev_err(dev, "failed to create dsi device\n");
-   ret = PTR_ERR(dsi);
-   goto err_dsi_device;
+   return PTR_ERR(dsi);
}
 
adv->dsi = dsi;
@@ -167,24 +166,13 @@ int adv7533_attach_dsi(struct adv7511 *adv)
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
  MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
dev_err(dev, "failed to attach dsi to host\n");
-   goto err_dsi_attach;
+   return ret;
}
 
return 0;
-
-err_dsi_attach:
-   mipi_dsi_device_unregister(dsi);
-err_dsi_device:
-   return ret;
-}
-
-void adv7533_detach_dsi(struct adv7511 *adv)
-{
-   mipi_dsi_detach(adv->dsi);
-   mipi_dsi_device_unregister(adv->dsi);
 }
 
 int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv)
-- 
2.31.1



[Freedreno] [PATCH v4 24/24] drm/exynos: dsi: Adjust probe order

2021-09-10 Thread Maxime Ripard
Without proper care and an agreement between how DSI hosts and devices
drivers register their MIPI-DSI entities and potential components, we can
end up in a situation where the drivers can never probe.

Most drivers were taking evasive maneuvers to try to workaround this,
but not all of them were following the same conventions, resulting in
various incompatibilities between DSI hosts and devices.

Now that we have a sequence agreed upon and documented, let's convert
exynos to it.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index e39fac889edc..dfda2b259c44 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1529,6 +1529,7 @@ static const struct drm_encoder_helper_funcs 
exynos_dsi_encoder_helper_funcs = {
 
 MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
 
+static const struct component_ops exynos_dsi_component_ops;
 static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
  struct mipi_dsi_device *device)
 {
@@ -1536,6 +1537,7 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host 
*host,
struct drm_encoder *encoder = &dsi->encoder;
struct drm_device *drm = encoder->dev;
struct drm_bridge *out_bridge;
+   struct device *dev = host->dev;
 
out_bridge  = of_drm_find_bridge(device->dev.of_node);
if (out_bridge) {
@@ -1585,7 +1587,7 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host 
*host,
if (drm->mode_config.poll_enabled)
drm_kms_helper_hotplug_event(drm);
 
-   return 0;
+   return component_add(dev, &exynos_dsi_component_ops);
 }
 
 static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
@@ -1593,6 +1595,9 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host 
*host,
 {
struct exynos_dsi *dsi = host_to_dsi(host);
struct drm_device *drm = dsi->encoder.dev;
+   struct device *dev = host->dev;
+
+   component_del(dev, &exynos_dsi_component_ops);
 
if (dsi->panel) {
mutex_lock(&drm->mode_config.mutex);
@@ -1716,7 +1721,7 @@ static int exynos_dsi_bind(struct device *dev, struct 
device *master,
of_node_put(in_bridge_node);
}
 
-   return mipi_dsi_host_register(&dsi->dsi_host);
+   return 0;
 }
 
 static void exynos_dsi_unbind(struct device *dev, struct device *master,
@@ -1726,8 +1731,6 @@ static void exynos_dsi_unbind(struct device *dev, struct 
device *master,
struct drm_encoder *encoder = &dsi->encoder;
 
exynos_dsi_disable(encoder);
-
-   mipi_dsi_host_unregister(&dsi->dsi_host);
 }
 
 static const struct component_ops exynos_dsi_component_ops = {
@@ -1821,7 +1824,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 
pm_runtime_enable(dev);
 
-   ret = component_add(dev, &exynos_dsi_component_ops);
+   ret = mipi_dsi_host_register(&dsi->dsi_host);
if (ret)
goto err_disable_runtime;
 
@@ -1835,10 +1838,12 @@ static int exynos_dsi_probe(struct platform_device 
*pdev)
 
 static int exynos_dsi_remove(struct platform_device *pdev)
 {
+   struct exynos_dsi *dsi = platform_get_drvdata(pdev);
+
+   mipi_dsi_host_unregister(&dsi->dsi_host);
+
pm_runtime_disable(&pdev->dev);
 
-   component_del(&pdev->dev, &exynos_dsi_component_ops);
-
return 0;
 }
 
-- 
2.31.1



[Freedreno] [PATCH v4 02/24] drm/bridge: Document the probe issue with MIPI-DSI bridges

2021-09-10 Thread Maxime Ripard
Interactions between bridges, panels, MIPI-DSI host and the component
framework are not trivial and can lead to probing issues when
implementing a display driver. Let's document the various cases we need
too consider, and the solution to support all the cases.

Signed-off-by: Maxime Ripard 
---
 Documentation/gpu/drm-kms-helpers.rst |  6 +++
 drivers/gpu/drm/drm_bridge.c  | 57 +++
 2 files changed, 63 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index 10f8df7aecc0..ec2f65b31930 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -157,6 +157,12 @@ Display Driver Integration
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
:doc: display driver integration
 
+Special Care with MIPI-DSI bridges
+--
+
+.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
+   :doc: special care dsi
+
 Bridge Operations
 -
 
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index baff74ea4a33..7cc2d2f94ae3 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -96,6 +96,63 @@
  * documentation of bridge operations for more details).
  */
 
+/**
+ * DOC: special care dsi
+ *
+ * The interaction between the bridges and other frameworks involved in
+ * the probing of the upstream driver and the bridge driver can be
+ * challenging. Indeed, there's multiple cases that needs to be
+ * considered:
+ *
+ * - The upstream driver doesn't use the component framework and isn't a
+ *   MIPI-DSI host. In this case, the bridge driver will probe at some
+ *   point and the upstream driver should try to probe again by returning
+ *   EPROBE_DEFER as long as the bridge driver hasn't probed.
+ *
+ * - The upstream driver doesn't use the component framework, but is a
+ *   MIPI-DSI host. The bridge device uses the MIPI-DCS commands to be
+ *   controlled. In this case, the bridge device is a child of the
+ *   display device and when it will probe it's assured that the display
+ *   device (and MIPI-DSI host) is present. The upstream driver will be
+ *   assured that the bridge driver is connected between the
+ *   &mipi_dsi_host_ops.attach and &mipi_dsi_host_ops.detach operations.
+ *   Therefore, it must run mipi_dsi_host_register() in its probe
+ *   function, and then run drm_bridge_attach() in its
+ *   &mipi_dsi_host_ops.attach hook.
+ *
+ * - The upstream driver uses the component framework and is a MIPI-DSI
+ *   host. The bridge device uses the MIPI-DCS commands to be
+ *   controlled. This is the same situation than above, and can run
+ *   mipi_dsi_host_register() in either its probe or bind hooks.
+ *
+ * - The upstream driver uses the component framework and is a MIPI-DSI
+ *   host. The bridge device uses a separate bus (such as I2C) to be
+ *   controlled. In this case, there's no correlation between the probe
+ *   of the bridge and upstream drivers, so care must be taken to avoid
+ *   an endless EPROBE_DEFER loop, with each driver waiting for the
+ *   other to probe.
+ *
+ * The ideal pattern to cover the last item (and all the others in the
+ * MIPI-DSI host driver case) is to split the operations like this:
+ *
+ * - The MIPI-DSI host driver must run mipi_dsi_host_register() in its
+ *   probe hook. It will make sure that the MIPI-DSI host sticks around,
+ *   and that the driver's bind can be called.
+ *
+ * - In its probe hook, the bridge driver must try to find its MIPI-DSI
+ *   host, register as a MIPI-DSI device and attach the MIPI-DSI device
+ *   to its host. The bridge driver is now functional.
+ *
+ * - In its &struct mipi_dsi_host_ops.attach hook, the MIPI-DSI host can
+ *   now add its component. Its bind hook will now be called and since
+ *   the bridge driver is attached and registered, we can now look for
+ *   and attach it.
+ *
+ * At this point, we're now certain that both the upstream driver and
+ * the bridge driver are functional and we can't have a deadlock-like
+ * situation when probing.
+ */
+
 static DEFINE_MUTEX(bridge_lock);
 static LIST_HEAD(bridge_list);
 
-- 
2.31.1



[Freedreno] [PATCH v4 08/24] drm/bridge: anx7625: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c 
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 4adeb2bad03a..d0317651cd75 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1367,12 +1367,6 @@ static int anx7625_bridge_attach(struct drm_bridge 
*bridge,
return -ENODEV;
}
 
-   err = anx7625_attach_dsi(ctx);
-   if (err) {
-   DRM_DEV_ERROR(dev, "Fail to attach to dsi : %d\n", err);
-   return err;
-   }
-
if (ctx->pdata.panel_bridge) {
err = drm_bridge_attach(bridge->encoder,
ctx->pdata.panel_bridge,
@@ -1845,10 +1839,24 @@ static int anx7625_i2c_probe(struct i2c_client *client,
platform->bridge.type = DRM_MODE_CONNECTOR_eDP;
drm_bridge_add(&platform->bridge);
 
+   ret = anx7625_attach_dsi(platform);
+   if (ret) {
+   DRM_DEV_ERROR(dev, "Fail to attach to dsi : %d\n", ret);
+   goto unregister_bridge;
+   }
+
DRM_DEV_DEBUG_DRIVER(dev, "probe done\n");
 
return 0;
 
+unregister_bridge:
+   drm_bridge_remove(&platform->bridge);
+
+   if (!platform->pdata.low_power_mode)
+   pm_runtime_put_sync_suspend(&client->dev);
+
+   anx7625_unregister_i2c_dummy_clients(platform);
+
 free_wq:
if (platform->workqueue)
destroy_workqueue(platform->workqueue);
-- 
2.31.1



[Freedreno] [PATCH v4 07/24] drm/bridge: anx7625: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 20 +---
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c 
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 1a871f6b6822..4adeb2bad03a 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1316,6 +1316,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
.channel = 0,
.node = NULL,
};
+   int ret;
 
DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n");
 
@@ -1325,7 +1326,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
return -EINVAL;
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
DRM_DEV_ERROR(dev, "fail to create dsi device.\n");
return -EINVAL;
@@ -1337,10 +1338,10 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
MIPI_DSI_MODE_VIDEO_SYNC_PULSE  |
MIPI_DSI_MODE_VIDEO_HSE;
 
-   if (mipi_dsi_attach(dsi) < 0) {
+   ret = devm_mipi_dsi_attach(dev, dsi);
+   if (ret) {
DRM_DEV_ERROR(dev, "fail to attach dsi to host.\n");
-   mipi_dsi_device_unregister(dsi);
-   return -EINVAL;
+   return ret;
}
 
ctx->dsi = dsi;
@@ -1350,16 +1351,6 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
return 0;
 }
 
-static void anx7625_bridge_detach(struct drm_bridge *bridge)
-{
-   struct anx7625_data *ctx = bridge_to_anx7625(bridge);
-
-   if (ctx->dsi) {
-   mipi_dsi_detach(ctx->dsi);
-   mipi_dsi_device_unregister(ctx->dsi);
-   }
-}
-
 static int anx7625_bridge_attach(struct drm_bridge *bridge,
 enum drm_bridge_attach_flags flags)
 {
@@ -1624,7 +1615,6 @@ static struct edid *anx7625_bridge_get_edid(struct 
drm_bridge *bridge,
 
 static const struct drm_bridge_funcs anx7625_bridge_funcs = {
.attach = anx7625_bridge_attach,
-   .detach = anx7625_bridge_detach,
.disable = anx7625_bridge_disable,
.mode_valid = anx7625_bridge_mode_valid,
.mode_set = anx7625_bridge_mode_set,
-- 
2.31.1



[Freedreno] [PATCH v4 13/24] drm/bridge: lt9611uxc: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 38 +-
 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c 
b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index 3cac16db970f..e5083bdf4c89 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -257,17 +257,18 @@ static struct mipi_dsi_device 
*lt9611uxc_attach_dsi(struct lt9611uxc *lt9611uxc,
const struct mipi_dsi_device_info info = { "lt9611uxc", 0, NULL };
struct mipi_dsi_device *dsi;
struct mipi_dsi_host *host;
+   struct device *dev = lt9611uxc->dev;
int ret;
 
host = of_find_mipi_dsi_host_by_node(dsi_node);
if (!host) {
-   dev_err(lt9611uxc->dev, "failed to find dsi host\n");
+   dev_err(dev, "failed to find dsi host\n");
return ERR_PTR(-EPROBE_DEFER);
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
-   dev_err(lt9611uxc->dev, "failed to create dsi device\n");
+   dev_err(dev, "failed to create dsi device\n");
return dsi;
}
 
@@ -276,10 +277,9 @@ static struct mipi_dsi_device *lt9611uxc_attach_dsi(struct 
lt9611uxc *lt9611uxc,
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
  MIPI_DSI_MODE_VIDEO_HSE;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
-   dev_err(lt9611uxc->dev, "failed to attach dsi to host\n");
-   mipi_dsi_device_unregister(dsi);
+   dev_err(dev, "failed to attach dsi to host\n");
return ERR_PTR(ret);
}
 
@@ -352,19 +352,6 @@ static int lt9611uxc_connector_init(struct drm_bridge 
*bridge, struct lt9611uxc
return drm_connector_attach_encoder(<9611uxc->connector, 
bridge->encoder);
 }
 
-static void lt9611uxc_bridge_detach(struct drm_bridge *bridge)
-{
-   struct lt9611uxc *lt9611uxc = bridge_to_lt9611uxc(bridge);
-
-   if (lt9611uxc->dsi1) {
-   mipi_dsi_detach(lt9611uxc->dsi1);
-   mipi_dsi_device_unregister(lt9611uxc->dsi1);
-   }
-
-   mipi_dsi_detach(lt9611uxc->dsi0);
-   mipi_dsi_device_unregister(lt9611uxc->dsi0);
-}
-
 static int lt9611uxc_bridge_attach(struct drm_bridge *bridge,
   enum drm_bridge_attach_flags flags)
 {
@@ -385,19 +372,11 @@ static int lt9611uxc_bridge_attach(struct drm_bridge 
*bridge,
/* Attach secondary DSI, if specified */
if (lt9611uxc->dsi1_node) {
lt9611uxc->dsi1 = lt9611uxc_attach_dsi(lt9611uxc, 
lt9611uxc->dsi1_node);
-   if (IS_ERR(lt9611uxc->dsi1)) {
-   ret = PTR_ERR(lt9611uxc->dsi1);
-   goto err_unregister_dsi0;
-   }
+   if (IS_ERR(lt9611uxc->dsi1))
+   return PTR_ERR(lt9611uxc->dsi1);
}
 
return 0;
-
-err_unregister_dsi0:
-   mipi_dsi_detach(lt9611uxc->dsi0);
-   mipi_dsi_device_unregister(lt9611uxc->dsi0);
-
-   return ret;
 }
 
 static enum drm_mode_status
@@ -541,7 +520,6 @@ static struct edid *lt9611uxc_bridge_get_edid(struct 
drm_bridge *bridge,
 
 static const struct drm_bridge_funcs lt9611uxc_bridge_funcs = {
.attach = lt9611uxc_bridge_attach,
-   .detach = lt9611uxc_bridge_detach,
.mode_valid = lt9611uxc_bridge_mode_valid,
.mode_set = lt9611uxc_bridge_mode_set,
.detect = lt9611uxc_bridge_detect,
-- 
2.31.1



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

2021-09-10 Thread Maxime Ripard
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,
Maxime

---

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 
 drivers/gpu/drm/bridge/ti-sn65dsi86.c|  94 -
 drivers/gpu/drm/drm_bridge.c |  69 -
 drivers/gpu/drm/drm_mipi_dsi.c   |  81 +++
 drivers/gpu/drm/exynos/exynos_drm_dsi.c  |  19 ++--
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c |  27 +++--
 include/drm/drm_mipi_dsi.h   |   4 +
 17 files changed, 460 insertions(+), 317 deletions(-)

-- 
2.31.1



[Freedreno] [PATCH v4 19/24] drm/bridge: sn65dsi86: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device. This also avoids leaking the device when we detach
the bridge.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 41d48a393e7f..b5662269ff95 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -674,6 +674,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
struct mipi_dsi_host *host;
struct mipi_dsi_device *dsi;
+   struct device *dev = pdata->dev;
const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
   .channel = 0,
   .node = NULL,
@@ -713,7 +714,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
goto err_dsi_host;
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
DRM_ERROR("failed to create dsi device\n");
ret = PTR_ERR(dsi);
@@ -726,16 +727,16 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
 
/* check if continuous dsi clock is required or not */
-   pm_runtime_get_sync(pdata->dev);
+   pm_runtime_get_sync(dev);
regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
-   pm_runtime_put_autosuspend(pdata->dev);
+   pm_runtime_put_autosuspend(dev);
if (!(val & DPPLL_CLK_SRC_DSICLK))
dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
DRM_ERROR("failed to attach dsi to host\n");
-   goto err_dsi_attach;
+   goto err_dsi_host;
}
pdata->dsi = dsi;
 
@@ -746,14 +747,10 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge,
&pdata->bridge, flags);
if (ret < 0)
-   goto err_dsi_detach;
+   goto err_dsi_host;
 
return 0;
 
-err_dsi_detach:
-   mipi_dsi_detach(dsi);
-err_dsi_attach:
-   mipi_dsi_device_unregister(dsi);
 err_dsi_host:
drm_connector_cleanup(&pdata->connector);
 err_conn_init:
@@ -1236,11 +1233,6 @@ static void ti_sn_bridge_remove(struct auxiliary_device 
*adev)
if (!pdata)
return;
 
-   if (pdata->dsi) {
-   mipi_dsi_detach(pdata->dsi);
-   mipi_dsi_device_unregister(pdata->dsi);
-   }
-
drm_bridge_remove(&pdata->bridge);
 
of_node_put(pdata->host_node);
-- 
2.31.1



[Freedreno] [PATCH v4 22/24] drm/bridge: tc358775: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/tc358775.c | 37 +--
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c 
b/drivers/gpu/drm/bridge/tc358775.c
index 35e66d1b6456..2c76331b251d 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -594,11 +594,26 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
 {
struct tc_data *tc = bridge_to_tc(bridge);
+
+   /* Attach the panel-bridge to the dsi bridge */
+   return drm_bridge_attach(bridge->encoder, tc->panel_bridge,
+&tc->bridge, flags);
+}
+
+static const struct drm_bridge_funcs tc_bridge_funcs = {
+   .attach = tc_bridge_attach,
+   .pre_enable = tc_bridge_pre_enable,
+   .enable = tc_bridge_enable,
+   .mode_valid = tc_mode_valid,
+   .post_disable = tc_bridge_post_disable,
+};
+
+static int tc_attach_host(struct tc_data *tc)
+{
struct device *dev = &tc->i2c->dev;
struct mipi_dsi_host *host;
struct mipi_dsi_device *dsi;
int ret;
-
const struct mipi_dsi_device_info info = { .type = "tc358775",
.channel = 0,
.node = NULL,
@@ -628,19 +643,9 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
return ret;
}
 
-   /* Attach the panel-bridge to the dsi bridge */
-   return drm_bridge_attach(bridge->encoder, tc->panel_bridge,
-&tc->bridge, flags);
+   return 0;
 }
 
-static const struct drm_bridge_funcs tc_bridge_funcs = {
-   .attach = tc_bridge_attach,
-   .pre_enable = tc_bridge_pre_enable,
-   .enable = tc_bridge_enable,
-   .mode_valid = tc_mode_valid,
-   .post_disable = tc_bridge_post_disable,
-};
-
 static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
struct device *dev = &client->dev;
@@ -704,7 +709,15 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
 
i2c_set_clientdata(client, tc);
 
+   ret = tc_attach_host(tc);
+   if (ret)
+   goto err_bridge_remove;
+
return 0;
+
+err_bridge_remove:
+   drm_bridge_remove(&tc->bridge);
+   return ret;
 }
 
 static int tc_remove(struct i2c_client *client)
-- 
2.31.1



[Freedreno] [PATCH v4 06/24] drm/bridge: adv7511: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 9e3585f23cf1..f8e5da148599 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -910,9 +910,6 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
return ret;
}
 
-   if (adv->type == ADV7533 || adv->type == ADV7535)
-   ret = adv7533_attach_dsi(adv);
-
if (adv->i2c_main->irq)
regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0),
 ADV7511_INT0_HPD);
@@ -1288,8 +1285,18 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
drm_bridge_add(&adv7511->bridge);
 
adv7511_audio_init(dev, adv7511);
+
+   if (adv7511->type == ADV7533 || adv7511->type == ADV7535) {
+   ret = adv7533_attach_dsi(adv7511);
+   if (ret)
+   goto err_unregister_audio;
+   }
+
return 0;
 
+err_unregister_audio:
+   adv7511_audio_exit(adv7511);
+   drm_bridge_remove(&adv7511->bridge);
 err_unregister_cec:
i2c_unregister_device(adv7511->i2c_cec);
clk_disable_unprepare(adv7511->cec_clk);
-- 
2.31.1



[Freedreno] [PATCH v4 23/24] drm/kirin: dsi: Adjust probe order

2021-09-10 Thread Maxime Ripard
Without proper care and an agreement between how DSI hosts and devices
drivers register their MIPI-DSI entities and potential components, we can
end up in a situation where the drivers can never probe.

Most drivers were taking evasive maneuvers to try to workaround this,
but not all of them were following the same conventions, resulting in
various incompatibilities between DSI hosts and devices.

Now that we have a sequence agreed upon and documented, let's convert
kirin to it.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 27 +++-
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c 
b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index 952cfdb1961d..be20c2ffe798 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -720,10 +720,13 @@ static int dw_drm_encoder_init(struct device *dev,
return 0;
 }
 
+static const struct component_ops dsi_ops;
 static int dsi_host_attach(struct mipi_dsi_host *host,
   struct mipi_dsi_device *mdsi)
 {
struct dw_dsi *dsi = host_to_dsi(host);
+   struct device *dev = host->dev;
+   int ret;
 
if (mdsi->lanes < 1 || mdsi->lanes > 4) {
DRM_ERROR("dsi device params invalid\n");
@@ -734,13 +737,20 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
dsi->format = mdsi->format;
dsi->mode_flags = mdsi->mode_flags;
 
+   ret = component_add(dev, &dsi_ops);
+   if (ret)
+   return ret;
+
return 0;
 }
 
 static int dsi_host_detach(struct mipi_dsi_host *host,
   struct mipi_dsi_device *mdsi)
 {
-   /* do nothing */
+   struct device *dev = host->dev;
+
+   component_del(dev, &dsi_ops);
+
return 0;
 }
 
@@ -785,10 +795,6 @@ static int dsi_bind(struct device *dev, struct device 
*master, void *data)
if (ret)
return ret;
 
-   ret = dsi_host_init(dev, dsi);
-   if (ret)
-   return ret;
-
ret = dsi_bridge_init(drm_dev, dsi);
if (ret)
return ret;
@@ -859,12 +865,19 @@ static int dsi_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, data);
 
-   return component_add(&pdev->dev, &dsi_ops);
+   ret = dsi_host_init(&pdev->dev, dsi);
+   if (ret)
+   return ret;
+
+   return 0;
 }
 
 static int dsi_remove(struct platform_device *pdev)
 {
-   component_del(&pdev->dev, &dsi_ops);
+   struct dsi_data *data = platform_get_drvdata(pdev);
+   struct dw_dsi *dsi = &data->dsi;
+
+   mipi_dsi_host_unregister(&dsi->host);
 
return 0;
 }
-- 
2.31.1



[Freedreno] [PATCH v4 09/24] drm/bridge: lt8912b: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c 
b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 1b0c7eaf6c84..cc968d65936b 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -472,11 +472,11 @@ static int lt8912_attach_dsi(struct lt8912 *lt)
return -EPROBE_DEFER;
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
ret = PTR_ERR(dsi);
dev_err(dev, "failed to create dsi device (%d)\n", ret);
-   goto err_dsi_device;
+   return ret;
}
 
lt->dsi = dsi;
@@ -489,24 +489,13 @@ static int lt8912_attach_dsi(struct lt8912 *lt)
  MIPI_DSI_MODE_LPM |
  MIPI_DSI_MODE_NO_EOT_PACKET;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
dev_err(dev, "failed to attach dsi to host\n");
-   goto err_dsi_attach;
+   return ret;
}
 
return 0;
-
-err_dsi_attach:
-   mipi_dsi_device_unregister(dsi);
-err_dsi_device:
-   return ret;
-}
-
-static void lt8912_detach_dsi(struct lt8912 *lt)
-{
-   mipi_dsi_detach(lt->dsi);
-   mipi_dsi_device_unregister(lt->dsi);
 }
 
 static int lt8912_bridge_connector_init(struct drm_bridge *bridge)
@@ -573,7 +562,6 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
struct lt8912 *lt = bridge_to_lt8912(bridge);
 
if (lt->is_attached) {
-   lt8912_detach_dsi(lt);
lt8912_hard_power_off(lt);
drm_connector_unregister(<->connector);
drm_connector_cleanup(<->connector);
-- 
2.31.1



[Freedreno] [PATCH v4 18/24] drm/bridge: sn65dsi83: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 80 +++
 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index db4d39082705..f951eb19767b 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -245,40 +245,6 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
 {
struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
-   struct device *dev = ctx->dev;
-   struct mipi_dsi_device *dsi;
-   struct mipi_dsi_host *host;
-   int ret = 0;
-
-   const struct mipi_dsi_device_info info = {
-   .type = "sn65dsi83",
-   .channel = 0,
-   .node = NULL,
-   };
-
-   host = of_find_mipi_dsi_host_by_node(ctx->host_node);
-   if (!host) {
-   dev_err(dev, "failed to find dsi host\n");
-   return -EPROBE_DEFER;
-   }
-
-   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
-   if (IS_ERR(dsi)) {
-   return dev_err_probe(dev, PTR_ERR(dsi),
-"failed to create dsi device\n");
-   }
-
-   ctx->dsi = dsi;
-
-   dsi->lanes = ctx->dsi_lanes;
-   dsi->format = MIPI_DSI_FMT_RGB888;
-   dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
-
-   ret = devm_mipi_dsi_attach(dev, dsi);
-   if (ret < 0) {
-   dev_err(dev, "failed to attach dsi to host\n");
-   return ret;
-   }
 
return drm_bridge_attach(bridge->encoder, ctx->panel_bridge,
 &ctx->bridge, flags);
@@ -646,6 +612,44 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum 
sn65dsi83_model model)
return 0;
 }
 
+static int sn65dsi83_host_attach(struct sn65dsi83 *ctx)
+{
+   struct device *dev = ctx->dev;
+   struct mipi_dsi_device *dsi;
+   struct mipi_dsi_host *host;
+   const struct mipi_dsi_device_info info = {
+   .type = "sn65dsi83",
+   .channel = 0,
+   .node = NULL,
+   };
+   int ret;
+
+   host = of_find_mipi_dsi_host_by_node(ctx->host_node);
+   if (!host) {
+   dev_err(dev, "failed to find dsi host\n");
+   return -EPROBE_DEFER;
+   }
+
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
+   if (IS_ERR(dsi))
+   return dev_err_probe(dev, PTR_ERR(dsi),
+"failed to create dsi device\n");
+
+   ctx->dsi = dsi;
+
+   dsi->lanes = ctx->dsi_lanes;
+   dsi->format = MIPI_DSI_FMT_RGB888;
+   dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
+
+   ret = devm_mipi_dsi_attach(dev, dsi);
+   if (ret < 0) {
+   dev_err(dev, "failed to attach dsi to host: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
 static int sn65dsi83_probe(struct i2c_client *client,
   const struct i2c_device_id *id)
 {
@@ -686,7 +690,15 @@ static int sn65dsi83_probe(struct i2c_client *client,
ctx->bridge.of_node = dev->of_node;
drm_bridge_add(&ctx->bridge);
 
+   ret = sn65dsi83_host_attach(ctx);
+   if (ret)
+   goto err_remove_bridge;
+
return 0;
+
+err_remove_bridge:
+   drm_bridge_remove(&ctx->bridge);
+   return ret;
 }
 
 static int sn65dsi83_remove(struct i2c_client *client)
-- 
2.31.1



[Freedreno] [PATCH v4 14/24] drm/bridge: lt9611uxc: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 31 +-
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c 
b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index e5083bdf4c89..78c4175e0a12 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -364,18 +364,6 @@ static int lt9611uxc_bridge_attach(struct drm_bridge 
*bridge,
return ret;
}
 
-   /* Attach primary DSI */
-   lt9611uxc->dsi0 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi0_node);
-   if (IS_ERR(lt9611uxc->dsi0))
-   return PTR_ERR(lt9611uxc->dsi0);
-
-   /* Attach secondary DSI, if specified */
-   if (lt9611uxc->dsi1_node) {
-   lt9611uxc->dsi1 = lt9611uxc_attach_dsi(lt9611uxc, 
lt9611uxc->dsi1_node);
-   if (IS_ERR(lt9611uxc->dsi1))
-   return PTR_ERR(lt9611uxc->dsi1);
-   }
-
return 0;
 }
 
@@ -955,8 +943,27 @@ static int lt9611uxc_probe(struct i2c_client *client,
 
drm_bridge_add(<9611uxc->bridge);
 
+   /* Attach primary DSI */
+   lt9611uxc->dsi0 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi0_node);
+   if (IS_ERR(lt9611uxc->dsi0)) {
+   ret = PTR_ERR(lt9611uxc->dsi0);
+   goto err_remove_bridge;
+   }
+
+   /* Attach secondary DSI, if specified */
+   if (lt9611uxc->dsi1_node) {
+   lt9611uxc->dsi1 = lt9611uxc_attach_dsi(lt9611uxc, 
lt9611uxc->dsi1_node);
+   if (IS_ERR(lt9611uxc->dsi1)) {
+   ret = PTR_ERR(lt9611uxc->dsi1);
+   goto err_remove_bridge;
+   }
+   }
+
return lt9611uxc_audio_init(dev, lt9611uxc);
 
+err_remove_bridge:
+   drm_bridge_remove(<9611uxc->bridge);
+
 err_disable_regulators:
regulator_bulk_disable(ARRAY_SIZE(lt9611uxc->supplies), 
lt9611uxc->supplies);
 
-- 
2.31.1



[Freedreno] [PATCH v4 17/24] drm/bridge: sn65dsi83: Switch to devm MIPI-DSI helpers

2021-09-10 Thread Maxime Ripard
Let's switch to the new devm MIPI-DSI function to register and attach
our secondary device. This also avoids leaking the device when we detach
the bridge but don't remove its driver.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index a32f70bc68ea..db4d39082705 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -262,7 +262,7 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,
return -EPROBE_DEFER;
}
 
-   dsi = mipi_dsi_device_register_full(host, &info);
+   dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
return dev_err_probe(dev, PTR_ERR(dsi),
 "failed to create dsi device\n");
@@ -274,18 +274,14 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
 
-   ret = mipi_dsi_attach(dsi);
+   ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
dev_err(dev, "failed to attach dsi to host\n");
-   goto err_dsi_attach;
+   return ret;
}
 
return drm_bridge_attach(bridge->encoder, ctx->panel_bridge,
 &ctx->bridge, flags);
-
-err_dsi_attach:
-   mipi_dsi_device_unregister(dsi);
-   return ret;
 }
 
 static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
@@ -697,8 +693,6 @@ static int sn65dsi83_remove(struct i2c_client *client)
 {
struct sn65dsi83 *ctx = i2c_get_clientdata(client);
 
-   mipi_dsi_detach(ctx->dsi);
-   mipi_dsi_device_unregister(ctx->dsi);
drm_bridge_remove(&ctx->bridge);
of_node_put(ctx->host_node);
 
-- 
2.31.1



[Freedreno] [PATCH v4 04/24] drm/mipi-dsi: Create devm device attachment

2021-09-10 Thread Maxime Ripard
MIPI-DSI devices need to call mipi_dsi_attach() when their probe is done
to attach against their host.

However, at removal or when an error occurs, that attachment needs to be
undone through a call to mipi_dsi_detach().

Let's create a device-managed variant of the attachment function that
will automatically detach the device at unbind.

Reviewed-by: Andrzej Hajda 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_mipi_dsi.c | 35 ++
 include/drm/drm_mipi_dsi.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index ddf67463eaa1..18cef04df2f2 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -391,6 +391,41 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
 }
 EXPORT_SYMBOL(mipi_dsi_detach);
 
+static void devm_mipi_dsi_detach(void *arg)
+{
+   struct mipi_dsi_device *dsi = arg;
+
+   mipi_dsi_detach(dsi);
+}
+
+/**
+ * devm_mipi_dsi_attach - Attach a MIPI-DSI device to its DSI Host
+ * @dev: device to tie the MIPI-DSI device attachment lifetime to
+ * @dsi: DSI peripheral
+ *
+ * This is the managed version of mipi_dsi_attach() which automatically
+ * calls mipi_dsi_detach() when @dev is unbound.
+ *
+ * Returns:
+ * 0 on success, a negative error code on failure.
+ */
+int devm_mipi_dsi_attach(struct device *dev,
+struct mipi_dsi_device *dsi)
+{
+   int ret;
+
+   ret = mipi_dsi_attach(dsi);
+   if (ret)
+   return ret;
+
+   ret = devm_add_action_or_reset(dev, devm_mipi_dsi_detach, dsi);
+   if (ret)
+   return ret;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(devm_mipi_dsi_attach);
+
 static ssize_t mipi_dsi_device_transfer(struct mipi_dsi_device *dsi,
struct mipi_dsi_msg *msg)
 {
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index d0032e435e08..147e51b6d241 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -233,6 +233,7 @@ devm_mipi_dsi_device_register_full(struct device *dev, 
struct mipi_dsi_host *hos
 struct mipi_dsi_device *of_find_mipi_dsi_device_by_node(struct device_node 
*np);
 int mipi_dsi_attach(struct mipi_dsi_device *dsi);
 int mipi_dsi_detach(struct mipi_dsi_device *dsi);
+int devm_mipi_dsi_attach(struct device *dev, struct mipi_dsi_device *dsi);
 int mipi_dsi_shutdown_peripheral(struct mipi_dsi_device *dsi);
 int mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi);
 int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi,
-- 
2.31.1



[Freedreno] [PATCH v4 20/24] drm/bridge: sn65dsi86: Register and attach our DSI device at probe

2021-09-10 Thread Maxime Ripard
In order to avoid any probe ordering issue, the best practice is to move
the secondary MIPI-DSI device registration and attachment to the
MIPI-DSI host at probe time. Let's do this.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 74 ++-
 1 file changed, 38 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index b5662269ff95..7f71329536a2 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -667,58 +667,27 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct 
drm_bridge *bridge)
return container_of(bridge, struct ti_sn65dsi86, bridge);
 }
 
-static int ti_sn_bridge_attach(struct drm_bridge *bridge,
-  enum drm_bridge_attach_flags flags)
+static int ti_sn_attach_host(struct ti_sn65dsi86 *pdata)
 {
int ret, val;
-   struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
struct mipi_dsi_host *host;
struct mipi_dsi_device *dsi;
struct device *dev = pdata->dev;
const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
   .channel = 0,
   .node = NULL,
-};
+   };
 
-   if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-   DRM_ERROR("Fix bridge driver to make connector optional!");
-   return -EINVAL;
-   }
-
-   pdata->aux.drm_dev = bridge->dev;
-   ret = drm_dp_aux_register(&pdata->aux);
-   if (ret < 0) {
-   drm_err(bridge->dev, "Failed to register DP AUX channel: %d\n", 
ret);
-   return ret;
-   }
-
-   ret = ti_sn_bridge_connector_init(pdata);
-   if (ret < 0)
-   goto err_conn_init;
-
-   /*
-* TODO: ideally finding host resource and dsi dev registration needs
-* to be done in bridge probe. But some existing DSI host drivers will
-* wait for any of the drm_bridge/drm_panel to get added to the global
-* bridge/panel list, before completing their probe. So if we do the
-* dsi dev registration part in bridge probe, before populating in
-* the global bridge list, then it will cause deadlock as dsi host probe
-* will never complete, neither our bridge probe. So keeping it here
-* will satisfy most of the existing host drivers. Once the host driver
-* is fixed we can move the below code to bridge probe safely.
-*/
host = of_find_mipi_dsi_host_by_node(pdata->host_node);
if (!host) {
DRM_ERROR("failed to find dsi host\n");
-   ret = -ENODEV;
-   goto err_dsi_host;
+   return -ENODEV;
}
 
dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
if (IS_ERR(dsi)) {
DRM_ERROR("failed to create dsi device\n");
-   ret = PTR_ERR(dsi);
-   goto err_dsi_host;
+   return PTR_ERR(dsi);
}
 
/* TODO: setting to 4 MIPI lanes always for now */
@@ -736,10 +705,35 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
ret = devm_mipi_dsi_attach(dev, dsi);
if (ret < 0) {
DRM_ERROR("failed to attach dsi to host\n");
-   goto err_dsi_host;
+   return ret;
}
pdata->dsi = dsi;
 
+   return 0;
+}
+
+static int ti_sn_bridge_attach(struct drm_bridge *bridge,
+  enum drm_bridge_attach_flags flags)
+{
+   struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
+   int ret;
+
+   if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
+   DRM_ERROR("Fix bridge driver to make connector optional!");
+   return -EINVAL;
+   }
+
+   pdata->aux.drm_dev = bridge->dev;
+   ret = drm_dp_aux_register(&pdata->aux);
+   if (ret < 0) {
+   drm_err(bridge->dev, "Failed to register DP AUX channel: %d\n", 
ret);
+   return ret;
+   }
+
+   ret = ti_sn_bridge_connector_init(pdata);
+   if (ret < 0)
+   goto err_conn_init;
+
/* We never want the next bridge to *also* create a connector: */
flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR;
 
@@ -1223,7 +1217,15 @@ static int ti_sn_bridge_probe(struct auxiliary_device 
*adev,
 
drm_bridge_add(&pdata->bridge);
 
+   ret = ti_sn_attach_host(pdata);
+   if (ret)
+   goto err_remove_bridge;
+
return 0;
+
+err_remove_bridge:
+   drm_bridge_remove(&pdata->bridge);
+   return ret;
 }
 
 static void ti_sn_bridge_remove(struct auxiliary_device *adev)
-- 
2.31.1



[Freedreno] [PATCH v4 03/24] drm/mipi-dsi: Create devm device registration

2021-09-10 Thread Maxime Ripard
Devices that take their data through the MIPI-DSI bus but are controlled
through a secondary bus like I2C have to register a secondary device on
the MIPI-DSI bus through the mipi_dsi_device_register_full() function.

At removal or when an error occurs, that device needs to be removed
through a call to mipi_dsi_device_unregister().

Let's create a device-managed variant of the registration function that
will automatically unregister the device at unbind.

Reviewed-by: Andrzej Hajda 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_mipi_dsi.c | 46 ++
 include/drm/drm_mipi_dsi.h |  3 +++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 5dd475e82995..ddf67463eaa1 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -246,6 +246,52 @@ void mipi_dsi_device_unregister(struct mipi_dsi_device 
*dsi)
 }
 EXPORT_SYMBOL(mipi_dsi_device_unregister);
 
+static void devm_mipi_dsi_device_unregister(void *arg)
+{
+   struct mipi_dsi_device *dsi = arg;
+
+   mipi_dsi_device_unregister(dsi);
+}
+
+/**
+ * devm_mipi_dsi_device_register_full - create a managed MIPI DSI device
+ * @dev: device to tie the MIPI-DSI device lifetime to
+ * @host: DSI host to which this device is connected
+ * @info: pointer to template containing DSI device information
+ *
+ * Create a MIPI DSI device by using the device information provided by
+ * mipi_dsi_device_info template
+ *
+ * This is the managed version of mipi_dsi_device_register_full() which
+ * automatically calls mipi_dsi_device_unregister() when @dev is
+ * unbound.
+ *
+ * Returns:
+ * A pointer to the newly created MIPI DSI device, or, a pointer encoded
+ * with an error
+ */
+struct mipi_dsi_device *
+devm_mipi_dsi_device_register_full(struct device *dev,
+  struct mipi_dsi_host *host,
+  const struct mipi_dsi_device_info *info)
+{
+   struct mipi_dsi_device *dsi;
+   int ret;
+
+   dsi = mipi_dsi_device_register_full(host, info);
+   if (IS_ERR(dsi))
+   return dsi;
+
+   ret = devm_add_action_or_reset(dev,
+  devm_mipi_dsi_device_unregister,
+  dsi);
+   if (ret)
+   return ERR_PTR(ret);
+
+   return dsi;
+}
+EXPORT_SYMBOL_GPL(devm_mipi_dsi_device_register_full);
+
 static DEFINE_MUTEX(host_lock);
 static LIST_HEAD(host_list);
 
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index af7ba8071eb0..d0032e435e08 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -227,6 +227,9 @@ struct mipi_dsi_device *
 mipi_dsi_device_register_full(struct mipi_dsi_host *host,
  const struct mipi_dsi_device_info *info);
 void mipi_dsi_device_unregister(struct mipi_dsi_device *dsi);
+struct mipi_dsi_device *
+devm_mipi_dsi_device_register_full(struct device *dev, struct mipi_dsi_host 
*host,
+  const struct mipi_dsi_device_info *info);
 struct mipi_dsi_device *of_find_mipi_dsi_device_by_node(struct device_node 
*np);
 int mipi_dsi_attach(struct mipi_dsi_device *dsi);
 int mipi_dsi_detach(struct mipi_dsi_device *dsi);
-- 
2.31.1