Re: [PATCH v2 2/3] Subject: [PATCH] drm/mediatek/dp: Add HDCP2.x feature for DisplayPort

2024-02-21 Thread 胡俊光


Re: [PATCH v2 2/3] Subject: [PATCH] drm/mediatek/dp: Add HDCP2.x feature for DisplayPort

2024-02-21 Thread 胡俊光


Re: [PATCH] drm/ttm: Fix an invalid freeing on already freed page in error path

2024-02-21 Thread Thomas Hellström
On Wed, 2024-02-21 at 11:26 +0100, Christian König wrote:
> Am 21.02.24 um 08:33 schrieb Thomas Hellström:
> > If caching mode change fails due to, for example, OOM we
> > free the allocated pages in a two-step process. First the pages
> > for which the caching change has already succeeded. Secondly
> > the pages for which a caching change did not succeed.
> > 
> > However the second step was incorrectly freeing the pages already
> > freed in the first step.
> > 
> > Fix.
> > 
> > Signed-off-by: Thomas Hellström 
> > Fixes: 379989e7cbdc ("drm/ttm/pool: Fix ttm_pool_alloc error path")
> > Cc: Christian König 
> > Cc: Dave Airlie 
> > Cc: Christian Koenig 
> > Cc: Huang Rui 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc:  # v6.4+
> 
> You don't know how much time I've spend staring at this line to find
> the 
> bug in it and haven't seen it. Got bug reports about that for month
> as well.


Yeah, sorry about that. We should probably have Kunit tests exercising
OOM in the pool code involving WC pages.

I'll push this to drm-misc-next.

/Thomas

> 
> Reviewed-by: Christian König 
> 
> > ---
> >   drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> > b/drivers/gpu/drm/ttm/ttm_pool.c
> > index b62f420a9f96..112438d965ff 100644
> > --- a/drivers/gpu/drm/ttm/ttm_pool.c
> > +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> > @@ -387,7 +387,7 @@ static void ttm_pool_free_range(struct ttm_pool
> > *pool, struct ttm_tt *tt,
> >     enum ttm_caching caching,
> >     pgoff_t start_page, pgoff_t
> > end_page)
> >   {
> > -   struct page **pages = tt->pages;
> > +   struct page **pages = >pages[start_page];
> >     unsigned int order;
> >     pgoff_t i, nr;
> >   
> 



Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg

2024-02-21 Thread David Gow
On Thu, 22 Feb 2024 at 04:10, 'Justin Stitt' via KUnit Development
 wrote:
>
> Hi,
>
> On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote:
> > The correct format specifier for p - n (both p and n are pointers) is
> > %td, as the type should be ptrdiff_t.
>
> I think %tu is better. d specifies a signed type. I don't doubt that the
> warning is fixed but I think %tu represents the type semantics here.
>

While I agree that this should never be negative, I'd still lean on
this being a signed type, for two reasons:
- I think, if there's a bug in this code, it's easier to debug this if
a 'negative' value were to appear as such.
- While, as I understand it, the C spec does provide for a
ptrdiff_t-sized unsigned printf specifier in '%tu', the difference
between two pointers is always signed:

"When two pointers are subtracted, both shall point to elements of the
same array object,
or one past the last element of the array object; the result is the
difference of the
subscripts of the two array elements. The size of the result is
implementation-defined,
and its type (a signed integer type) is ptrdiff_t defined in the
 header"

(Technically, the kernel's ptrdiff_t type isn't defined in stddef.h,
so a bit of deviation from the spec is happening anyway, though.)

If there's a particularly good reason to make this unsigned in this
case, I'd be happy to change it, of course. But I'd otherwise prefer
to keep it as-is.

Cheers,
-- David


smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test

2024-02-21 Thread Linus Torvalds
On Wed, 21 Feb 2024 at 21:05, Lucas De Marchi  wrote:
>
> this has a potential to cause conflicts with upcoming work, so I think
> it's better to apply this through drm-xe-next. Let me know if you agree.

I disagree. Violently.

For this to be fixed, we need to have the printf format checking enabled.

And we can't enable it until all the problems have been fixed.

Which means that we should *not* have to wait for [N] different trees
to fix their issues separately.

This should get fixed in the Kunit tree, so that the Kunit tree can
just send a pull request to me to enable format checking for the KUnit
tests, together with all the fixes.  Trying to spread those fixes out
to different git branches will only result in pain and pointless
dependencies between different trees.

Honestly, the reason I noticed the problem in the first place was that
the drm tree had a separate bug, that had been apparently noted in
linux-next, and *despite* that it made it into a pull request to me
and caused new build failures in rc5.

So as things are, I am not IN THE LEAST interested in some kind of
"let us fix this in the drm tree separately" garbage.  We're not
making things worse by trying to fix this in different trees.

We're fixing this in the Kunit tree, and I do not want to get *more*
problems from the drm side. I've had enough.

   Linus


Re: [PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test

2024-02-21 Thread Lucas De Marchi

On Wed, Feb 21, 2024 at 05:27:21PM +0800, David Gow wrote:

KUNIT_FAIL() is used to fail the xe_migrate test when an error occurs.
However, there's a mismatch in the format specifier: '%li' is used to
log 'err', which is an 'int'.

Use '%i' instead of '%li', and for the case where we're printing an
error pointer, just use '%pe', instead of extracting the error code
manually with PTR_ERR(). (This also results in a nicer output when the
error code is known.)

Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Signed-off-by: David Gow 



Reviewed-by: Lucas De Marchi 

this has a potential to cause conflicts with upcoming work, so I think
it's better to apply this through drm-xe-next. Let me know if you agree.

thanks
Lucas De Marchi


---
drivers/gpu/drm/xe/tests/xe_migrate.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c 
b/drivers/gpu/drm/xe/tests/xe_migrate.c
index a6523df0f1d3..c347e2c29f81 100644
--- a/drivers/gpu/drm/xe/tests/xe_migrate.c
+++ b/drivers/gpu/drm/xe/tests/xe_migrate.c
@@ -114,21 +114,21 @@ static void test_copy(struct xe_migrate *m, struct xe_bo 
*bo,
   region |
   XE_BO_NEEDS_CPU_ACCESS);
if (IS_ERR(remote)) {
-   KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %li\n",
-  str, PTR_ERR(remote));
+   KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %pe\n",
+  str, remote);
return;
}

err = xe_bo_validate(remote, NULL, false);
if (err) {
-   KUNIT_FAIL(test, "Failed to validate system bo for %s: %li\n",
+   KUNIT_FAIL(test, "Failed to validate system bo for %s: %i\n",
   str, err);
goto out_unlock;
}

err = xe_bo_vmap(remote);
if (err) {
-   KUNIT_FAIL(test, "Failed to vmap system bo for %s: %li\n",
+   KUNIT_FAIL(test, "Failed to vmap system bo for %s: %i\n",
   str, err);
goto out_unlock;
}
--
2.44.0.rc0.258.g7320e95886-goog



Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 05:47, Bjorn Andersson  wrote:
>
> On Thu, Feb 22, 2024 at 01:38:45AM +0200, Dmitry Baryshkov wrote:
> > On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  
> > wrote:
> > >
> > > When upstreamed the SC7280 DP controllers where described as one being
> > > DP and one eDP, but they can infact both be DP or eDP.
> > >
> > > Extend the list of DP controllers to cover both instances, and rely on
> > > the newly introduced mechanism for selecting which mode they should
> > > operate in.
> > >
> > > Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree
> > > will continue to select eDP.
> > >
> > > Signed-off-by: Bjorn Andersson 
> > > ---
> > >  drivers/gpu/drm/msm/dp/dp_display.c | 9 +++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> > > b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index 7b8c695d521a..1792ba9f7259 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = {
> > >  };
> > >
> > >  static const struct msm_dp_desc sc7280_dp_descs[] = {
> > > -   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, 
> > > .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
> > > +   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en 
> > > = true },
> > > +   { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en 
> > > = true },
> >
> > I think we need to keep .connector_type here, don't we?
> >
>
> No, Abel removes the need for that in his patches - and while that logic
> is slightly broken in the RFC I think it looks good.

Let's see v2 first.

>
> Regards,
> Bjorn
>
> > > +   {}
> > > +};
> > > +
> > > +static const struct msm_dp_desc sc7280_edp_descs[] = {
> > > { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, 
> > > .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
> > > {}
> > >  };
> > > @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = 
> > > {
> > >  static const struct of_device_id dp_dt_match[] = {
> > > { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
> > > { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> > > -   { .compatible = "qcom,sc7280-edp", .data = _dp_descs },
> > > +   { .compatible = "qcom,sc7280-edp", .data = _edp_descs },
> > > { .compatible = "qcom,sc8180x-dp", .data = _dp_descs },
> > > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
> > > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
> > >
> > > --
> > > 2.25.1
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry


Re: [PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching

2024-02-21 Thread Bjorn Andersson
On Thu, Feb 22, 2024 at 01:50:12AM +0200, Dmitry Baryshkov wrote:
> On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  
> wrote:
> >
> > With the ADSP remoteproc loaded pmic_glink can be introduced and wired
> > up to provide role and orientation switching signals.
> >
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 
> > +++-
> >  1 file changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
> > b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > index ab498494caea..079bf43b14cc 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > @@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 {
> > };
> > };
> >
> > +   pmic-glink {
> > +   compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink";
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   connector@0 {
> > +   compatible = "usb-c-connector";
> > +   reg = <0>;
> > +   power-role = "dual";
> > +   data-role = "dual";
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   port@0 {
> > +   reg = <0>;
> > +
> > +   pmic_glink_hs_in: endpoint {
> > +   remote-endpoint = 
> > <_1_dwc3_hs>;
> > +   };
> > +   };
> > +
> > +   port@1 {
> > +   reg = <1>;
> > +
> > +   pmic_glink_ss_in: endpoint {
> > +   remote-endpoint = 
> > <_1_dwc3_ss>;
> 
> This should be connected to the QMP PHY rather than to the USB host.
> 

Ahh, you're right, otherwise the orientation-switch below isn't of much
use.

> Also it might be better to squash this patch with the patch 8. Or at
> least to get redriver into the picture in this patch (and keep only
> display-related parts in that patch).
> 

The idea was to only bring in the pmic-glink here and then do the
plumbing between all the components separately, but I guess the
orientation-switch in the redriver means that it should go here as
well...

I'll shuffle this into something that makes sense.

Thanks,
Bjorn

> 
> > +   };
> > +   };
> > +   };
> > +   };
> > +   };
> > +
> > vph_pwr: vph-pwr-regulator {
> > compatible = "regulator-fixed";
> > regulator-name = "vph_pwr";
> > @@ -476,7 +511,16 @@ _1 {
> >  };
> >
> >  _1_dwc3 {
> > -   dr_mode = "peripheral";
> > +   dr_mode = "otg";
> > +   usb-role-switch;
> > +};
> > +
> > +_1_dwc3_hs {
> > +   remote-endpoint = <_glink_hs_in>;
> > +};
> > +
> > +_1_dwc3_ss {
> > +   remote-endpoint = <_glink_ss_in>;
> >  };
> >
> >  _1_hsphy {
> > @@ -491,6 +535,8 @@ _1_qmpphy {
> > vdda-phy-supply = <_l6b_1p2>;
> > vdda-pll-supply = <_l1b_0p912>;
> >
> > +   orientation-switch;
> > +
> > status = "okay";
> >  };
> >
> >
> > --
> > 2.25.1
> >
> 
> 
> --
> With best wishes
> Dmitry


Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280

2024-02-21 Thread Bjorn Andersson
On Thu, Feb 22, 2024 at 01:38:45AM +0200, Dmitry Baryshkov wrote:
> On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  
> wrote:
> >
> > When upstreamed the SC7280 DP controllers where described as one being
> > DP and one eDP, but they can infact both be DP or eDP.
> >
> > Extend the list of DP controllers to cover both instances, and rely on
> > the newly introduced mechanism for selecting which mode they should
> > operate in.
> >
> > Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree
> > will continue to select eDP.
> >
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  drivers/gpu/drm/msm/dp/dp_display.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> > b/drivers/gpu/drm/msm/dp/dp_display.c
> > index 7b8c695d521a..1792ba9f7259 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = {
> >  };
> >
> >  static const struct msm_dp_desc sc7280_dp_descs[] = {
> > -   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, 
> > .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
> > +   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = 
> > true },
> > +   { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = 
> > true },
> 
> I think we need to keep .connector_type here, don't we?
> 

No, Abel removes the need for that in his patches - and while that logic
is slightly broken in the RFC I think it looks good.

Regards,
Bjorn

> > +   {}
> > +};
> > +
> > +static const struct msm_dp_desc sc7280_edp_descs[] = {
> > { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, 
> > .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
> > {}
> >  };
> > @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
> >  static const struct of_device_id dp_dt_match[] = {
> > { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
> > { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> > -   { .compatible = "qcom,sc7280-edp", .data = _dp_descs },
> > +   { .compatible = "qcom,sc7280-edp", .data = _edp_descs },
> > { .compatible = "qcom,sc8180x-dp", .data = _dp_descs },
> > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
> > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
> >
> > --
> > 2.25.1
> >
> 
> 
> -- 
> With best wishes
> Dmitry


Re: [PATCH RFC 3/3] drm/msm/dp: Add support for the X1E80100

2024-02-21 Thread Bjorn Andersson
On Wed, Feb 21, 2024 at 12:50:33AM +0200, Abel Vesa wrote:
> Add the X1E80100 DP descs and compatible. This platform will be using
> a single compatible for both eDP and DP mode. The actual mode will
> be set in devicetree via is-edp flag.
> 
> Signed-off-by: Abel Vesa 
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 9e58285d4ec6..7b8c695d521a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -171,6 +171,14 @@ static const struct msm_dp_desc sm8650_dp_descs[] = {
>   {}
>  };
>  
> +static const struct msm_dp_desc x1e80100_dp_descs[] = {
> + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = 
> true },
> + { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = 
> true },
> + { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_en = 
> true },
> + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .wide_bus_en = 
> true },
> + {}
> +};
> +
>  static const struct of_device_id dp_dt_match[] = {
>   { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
>   { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> @@ -179,6 +187,7 @@ static const struct of_device_id dp_dt_match[] = {
>   { .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
>   { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
>   { .compatible = "qcom,sc8280xp-edp", .data = _edp_descs },
> + { .compatible = "qcom,x1e80100-dp", .data = _dp_descs },

This doesn't look like alphabetical order.

Regards,
Bjorn

>   { .compatible = "qcom,sdm845-dp", .data = _dp_descs },
>   { .compatible = "qcom,sm8350-dp", .data = _dp_descs },
>   { .compatible = "qcom,sm8650-dp", .data = _dp_descs },
> 
> -- 
> 2.34.1
> 


Re: [PATCH RFC 1/3] dt-bindings: display: msm: dp-controller: document X1E80100 compatible

2024-02-21 Thread Bjorn Andersson
On Wed, Feb 21, 2024 at 12:50:31AM +0200, Abel Vesa wrote:
> Add the X1E80100 to the list of compatibles and docoment the is-edp

s/docoment/document/

> flag. This new flag will be used from now on to dictate the mode from

s/from now on//

Perhaps cleaner to spell out that the controllers are expected to
operate in DP mode by default, and this flag can be used to select eDP
mode.

> devicetree, instead of having separate compatibles for eDP and DP.
> 

Regards,
Bjorn

> Signed-off-by: Abel Vesa 
> ---
>  Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml 
> b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index ae53cbfb2193..ed11852e403d 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -27,6 +27,7 @@ properties:
>- qcom,sdm845-dp
>- qcom,sm8350-dp
>- qcom,sm8650-dp
> +  - qcom,x1e80100-dp
>- items:
>- enum:
>- qcom,sm8150-dp
> @@ -73,6 +74,11 @@ properties:
>- description: phy 0 parent
>- description: phy 1 parent
>  
> +  is-edp:
> +$ref: /schemas/types.yaml#/definitions/flag
> +description:
> +  Tells the controller to switch to eDP mode
> +
>phys:
>  maxItems: 1
>  
> 
> -- 
> 2.34.1
> 


Re: [PATCH RFC 2/3] drm/msm/dp: Add support for setting the eDP mode from devicetree

2024-02-21 Thread Bjorn Andersson
On Wed, Feb 21, 2024 at 12:50:32AM +0200, Abel Vesa wrote:
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h 
> b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index fa014cee7e21..a10d1b19d172 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -32,6 +32,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct 
> dp_link *link,
>   struct phy *phy);
>  
>  void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable);
> +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int mode);
>  void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl);
>  void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl);
>  void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index e4433891becb..9e58285d4ec6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1229,6 +1229,7 @@ static int dp_display_probe(struct platform_device 
> *pdev)
>   int rc = 0;
>   struct dp_display_private *dp;
>   const struct msm_dp_desc *desc;
> + bool is_edp = false;
>  
>   if (!pdev || !pdev->dev.of_node) {
>   DRM_ERROR("pdev not found\n");
> @@ -1243,13 +1244,19 @@ static int dp_display_probe(struct platform_device 
> *pdev)
>   if (!desc)
>   return -EINVAL;
>  
> + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP ||

dp is just allocated, and will be 0. You need to check
desc->connector_type here.

Regards,
Bjorn

> + of_property_read_bool(pdev->dev.of_node, "is-edp"))
> + is_edp = true;
> +
> + dp->dp_display.is_edp = is_edp;
>   dp->dp_display.pdev = pdev;
>   dp->name = "drm_dp";
>   dp->id = desc->id;
> - dp->dp_display.connector_type = desc->connector_type;
>   dp->wide_bus_en = desc->wide_bus_en;
> - dp->dp_display.is_edp =
> - (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
> +
> + dp->dp_display.connector_type = is_edp ?
> + DRM_MODE_CONNECTOR_eDP :
> + DRM_MODE_CONNECTOR_DisplayPort;
>  
>   rc = dp_init_sub_modules(dp);
>   if (rc) {
> @@ -1257,6 +1264,12 @@ static int dp_display_probe(struct platform_device 
> *pdev)
>   return -EPROBE_DEFER;
>   }
>  
> + rc = dp_ctrl_phy_set_mode(dp->ctrl, is_edp ? PHY_SUBMODE_EDP : 
> PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("setting PHY submode failed\n");
> + goto err;
> + }
> +
>   /* setup event q */
>   mutex_init(>event_mutex);
>   init_waitqueue_head(>event_q);
> 
> -- 
> 2.34.1
> 


Re: [PATCH] drm: ci: uprev IGT

2024-02-21 Thread Dmitry Baryshkov
On Tue, 20 Feb 2024 at 16:31, Helen Koike  wrote:
>
>
>
> On 20/02/2024 09:17, Dmitry Baryshkov wrote:
> > Bump IGT revision to pick up Rob Clark's fixes for the msm driver:
> >
> > - msm_submit@invalid-duplicate-bo-submit,Fail
> >
> > Signed-off-by: Dmitry Baryshkov 
>
> Do you have a gitlab pipeline link I can check?

Before uprev: https://gitlab.freedesktop.org/drm/msm/-/pipelines/1109455

After uprev: https://gitlab.freedesktop.org/drm/msm/-/pipelines/1109501

-- 
With best wishes
Dmitry


Re: [PATCH 6/6] phy: qcom-qmp-combo: fix type-c switch registration

2024-02-21 Thread Bjorn Andersson
On Sat, Feb 17, 2024 at 04:02:28PM +0100, Johan Hovold wrote:
> Due to a long-standing issue in driver core, drivers may not probe defer
> after having registered child devices to avoid triggering a probe
> deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of
> -EPROBE_DEFER")).
> 
> Move registration of the typec switch to after looking up clocks and
> other resources.
> 
> Note that PHY creation can in theory also trigger a probe deferral when
> a 'phy' supply is used. This does not seem to affect the QMP PHY driver
> but the PHY subsystem should be reworked to address this (i.e. by
> separating initialisation and registration of the PHY).
> 
> Fixes: 2851117f8f42 ("phy: qcom-qmp-combo: Introduce orientation switching")
> Cc: sta...@vger.kernel.org  # 6.5
> Cc: Bjorn Andersson 
> Signed-off-by: Johan Hovold 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index e19d6a084f10..17c4ad7553a5 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3562,10 +3562,6 @@ static int qmp_combo_probe(struct platform_device 
> *pdev)
>   if (ret)
>   return ret;
>  
> - ret = qmp_combo_typec_switch_register(qmp);
> - if (ret)
> - return ret;
> -
>   /* Check for legacy binding with child nodes. */
>   usb_np = of_get_child_by_name(dev->of_node, "usb3-phy");
>   if (usb_np) {
> @@ -3585,6 +3581,10 @@ static int qmp_combo_probe(struct platform_device 
> *pdev)
>   if (ret)
>   goto err_node_put;
>  
> + ret = qmp_combo_typec_switch_register(qmp);
> + if (ret)
> + goto err_node_put;
> +
>   ret = drm_aux_bridge_register(dev);
>   if (ret)
>   goto err_node_put;
> -- 
> 2.43.0
> 


Re: [PATCH 5/6] phy: qcom-qmp-combo: fix drm bridge registration

2024-02-21 Thread Bjorn Andersson
On Sat, Feb 17, 2024 at 04:02:27PM +0100, Johan Hovold wrote:
> Due to a long-standing issue in driver core, drivers may not probe defer
> after having registered child devices to avoid triggering a probe
> deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of
> -EPROBE_DEFER")).
> 
> This could potentially also trigger a bug in the DRM bridge
> implementation which does not expect bridges to go away even if device
> links may avoid triggering this (when enabled).
> 
> Move registration of the DRM aux bridge to after looking up clocks and
> other resources.
> 
> Note that PHY creation can in theory also trigger a probe deferral when
> a 'phy' supply is used. This does not seem to affect the QMP PHY driver
> but the PHY subsystem should be reworked to address this (i.e. by
> separating initialisation and registration of the PHY).
> 
> Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE")
> Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge")
> Cc: sta...@vger.kernel.org  # 6.5
> Cc: Bjorn Andersson 
> Cc: Dmitry Baryshkov 
> Signed-off-by: Johan Hovold 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 1ad10110dd25..e19d6a084f10 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3566,10 +3566,6 @@ static int qmp_combo_probe(struct platform_device 
> *pdev)
>   if (ret)
>   return ret;
>  
> - ret = drm_aux_bridge_register(dev);
> - if (ret)
> - return ret;
> -
>   /* Check for legacy binding with child nodes. */
>   usb_np = of_get_child_by_name(dev->of_node, "usb3-phy");
>   if (usb_np) {
> @@ -3589,6 +3585,10 @@ static int qmp_combo_probe(struct platform_device 
> *pdev)
>   if (ret)
>   goto err_node_put;
>  
> + ret = drm_aux_bridge_register(dev);
> + if (ret)
> + goto err_node_put;
> +
>   pm_runtime_set_active(dev);
>   ret = devm_pm_runtime_enable(dev);
>   if (ret)
> -- 
> 2.43.0
> 


Re: [PATCH 4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m

2024-02-21 Thread Bjorn Andersson
On Sat, Feb 17, 2024 at 04:02:26PM +0100, Johan Hovold wrote:
> From: Rob Clark 
> 
> We need to bail out before adding/removing devices if we are going to
> -EPROBE_DEFER. Otherwise boot can get stuck in a probe deferral loop due
> to a long-standing issue in driver core (see fbc35b45f9f6 ("Add
> documentation on meaning of -EPROBE_DEFER")).
> 
> Deregistering the altmode child device can potentially also trigger bugs
> in the DRM bridge implementation, which does not expect bridges to go
> away.
> 
> Suggested-by: Dmitry Baryshkov 
> Signed-off-by: Rob Clark 
> Link: https://lore.kernel.org/r/20231213210644.8702-1-robdcl...@gmail.com
> [ johan: rebase on 6.8-rc4, amend commit message and mention DRM ]
> Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK 
> driver")
> Cc: sta...@vger.kernel.org  # 6.3
> Cc: Bjorn Andersson 
> Signed-off-by: Johan Hovold 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/soc/qcom/pmic_glink.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index f4bfd24386f1..f913e9bd57ed 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -265,10 +265,17 @@ static int pmic_glink_probe(struct platform_device 
> *pdev)
>  
>   pg->client_mask = *match_data;
>  
> + pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
> + if (IS_ERR(pg->pdr)) {
> + ret = dev_err_probe(>dev, PTR_ERR(pg->pdr),
> + "failed to initialize pdr\n");
> + return ret;
> + }
> +
>   if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) {
>   ret = pmic_glink_add_aux_device(pg, >ucsi_aux, "ucsi");
>   if (ret)
> - return ret;
> + goto out_release_pdr_handle;
>   }
>   if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) {
>   ret = pmic_glink_add_aux_device(pg, >altmode_aux, 
> "altmode");
> @@ -281,17 +288,11 @@ static int pmic_glink_probe(struct platform_device 
> *pdev)
>   goto out_release_altmode_aux;
>   }
>  
> - pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
> - if (IS_ERR(pg->pdr)) {
> - ret = dev_err_probe(>dev, PTR_ERR(pg->pdr), "failed to 
> initialize pdr\n");
> - goto out_release_aux_devices;
> - }
> -
>   service = pdr_add_lookup(pg->pdr, "tms/servreg", "msm/adsp/charger_pd");
>   if (IS_ERR(service)) {
>   ret = dev_err_probe(>dev, PTR_ERR(service),
>   "failed adding pdr lookup for 
> charger_pd\n");
> - goto out_release_pdr_handle;
> + goto out_release_aux_devices;
>   }
>  
>   mutex_lock(&__pmic_glink_lock);
> @@ -300,8 +301,6 @@ static int pmic_glink_probe(struct platform_device *pdev)
>  
>   return 0;
>  
> -out_release_pdr_handle:
> - pdr_handle_release(pg->pdr);
>  out_release_aux_devices:
>   if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
>   pmic_glink_del_aux_device(pg, >ps_aux);
> @@ -311,6 +310,8 @@ static int pmic_glink_probe(struct platform_device *pdev)
>  out_release_ucsi_aux:
>   if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
>   pmic_glink_del_aux_device(pg, >ucsi_aux);
> +out_release_pdr_handle:
> + pdr_handle_release(pg->pdr);
>  
>   return ret;
>  }
> -- 
> 2.43.0
> 


Re: [PATCH 3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free

2024-02-21 Thread Bjorn Andersson
On Sat, Feb 17, 2024 at 04:02:25PM +0100, Johan Hovold wrote:
> A recent DRM series purporting to simplify support for "transparent
> bridges" and handling of probe deferrals ironically exposed a
> use-after-free issue on pmic_glink_altmode probe deferral.
> 
> This has manifested itself as the display subsystem occasionally failing
> to initialise and NULL-pointer dereferences during boot of machines like
> the Lenovo ThinkPad X13s.
> 
> Specifically, the dp-hpd bridge is currently registered before all
> resources have been acquired which means that it can also be
> deregistered on probe deferrals.
> 
> In the meantime there is a race window where the new aux bridge driver
> (or PHY driver previously) may have looked up the dp-hpd bridge and
> stored a (non-reference-counted) pointer to the bridge which is about to
> be deallocated.
> 
> When the display controller is later initialised, this triggers a
> use-after-free when attaching the bridges:
> 
>   dp -> aux -> dp-hpd (freed)
> 
> which may, for example, result in the freed bridge failing to attach:
> 
>   [drm:drm_bridge_attach [drm]] *ERROR* failed to attach bridge 
> /soc@0/phy@88eb000 to encoder TMDS-31: -16
> 
> or a NULL-pointer dereference:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 
> 
>   ...
>   Call trace:
> drm_bridge_attach+0x70/0x1a8 [drm]
> drm_aux_bridge_attach+0x24/0x38 [aux_bridge]
> drm_bridge_attach+0x80/0x1a8 [drm]
> dp_bridge_init+0xa8/0x15c [msm]
> msm_dp_modeset_init+0x28/0xc4 [msm]
> 
> The DRM bridge implementation is clearly fragile and implicitly built on
> the assumption that bridges may never go away. In this case, the fix is
> to move the bridge registration in the pmic_glink_altmode driver to
> after all resources have been looked up.
> 
> Incidentally, with the new dp-hpd bridge implementation, which registers
> child devices, this is also a requirement due to a long-standing issue
> in driver core that can otherwise lead to a probe deferral loop (see
> fbc35b45f9f6 ("Add documentation on meaning of -EPROBE_DEFER")).
> 
> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
> Fixes: 2bcca96abfbf ("soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE")
> Cc: sta...@vger.kernel.org  # 6.3
> Cc: Bjorn Andersson 
> Cc: Dmitry Baryshkov 
> Signed-off-by: Johan Hovold 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/soc/qcom/pmic_glink_altmode.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c 
> b/drivers/soc/qcom/pmic_glink_altmode.c
> index 5fcd0fdd2faa..b3808fc24c69 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
> @@ -76,7 +76,7 @@ struct pmic_glink_altmode_port {
>  
>   struct work_struct work;
>  
> - struct device *bridge;
> + struct auxiliary_device *bridge;
>  
>   enum typec_orientation orientation;
>   u16 svid;
> @@ -230,7 +230,7 @@ static void pmic_glink_altmode_worker(struct work_struct 
> *work)
>   else
>   pmic_glink_altmode_enable_usb(altmode, alt_port);
>  
> - drm_aux_hpd_bridge_notify(alt_port->bridge,
> + drm_aux_hpd_bridge_notify(_port->bridge->dev,
> alt_port->hpd_state ?
> connector_status_connected :
> connector_status_disconnected);
> @@ -454,7 +454,7 @@ static int pmic_glink_altmode_probe(struct 
> auxiliary_device *adev,
>   alt_port->index = port;
>   INIT_WORK(_port->work, pmic_glink_altmode_worker);
>  
> - alt_port->bridge = drm_dp_hpd_bridge_register(dev, 
> to_of_node(fwnode));
> + alt_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, 
> to_of_node(fwnode));
>   if (IS_ERR(alt_port->bridge)) {
>   fwnode_handle_put(fwnode);
>   return PTR_ERR(alt_port->bridge);
> @@ -510,6 +510,16 @@ static int pmic_glink_altmode_probe(struct 
> auxiliary_device *adev,
>   }
>   }
>  
> + for (port = 0; port < ARRAY_SIZE(altmode->ports); port++) {
> + alt_port = >ports[port];
> + if (!alt_port->bridge)
> + continue;
> +
> + ret = devm_drm_dp_hpd_bridge_add(dev, alt_port->bridge);
> + if (ret)
> + return ret;
> + }
> +
>   altmode->client = devm_pmic_glink_register_client(dev,
> altmode->owner_id,
> 
> pmic_glink_altmode_callback,
> -- 
> 2.43.0
> 


Re: [PATCH 2/6] drm/bridge: aux-hpd: separate allocation and registration

2024-02-21 Thread Bjorn Andersson
On Sat, Feb 17, 2024 at 04:02:24PM +0100, Johan Hovold wrote:
> diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c 
> b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
[..]
> +/**
> + * devm_drm_dp_hpd_bridge_add - register a HDP DisplayPort bridge

kernel-doc wants () after function names.

> + * @dev: struct device to tie registration lifetime to
> + * @adev: bridge auxiliary device to be registered
> + *
> + * Returns: zero on success or a negative errno

and "Return:" without the 's'.

This could however be done in a separate patch, as the file is already
wrong in this regard.

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn


linux-next: build failure after merge of the drm-misc tree

2024-02-21 Thread Stephen Rothwell
Hi all,

After merging the drm-misc tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:47:6: error: redefinition of 
'rzg2l_du_vsp_enable'
   47 | void rzg2l_du_vsp_enable(struct rzg2l_du_crtc *crtc)
  |  ^~~
In file included from drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h:18,
 from drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:30:
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:72:20: note: previous definition 
of 'rzg2l_du_vsp_enable' with type 'void(struct rzg2l_du_crtc *)'
   72 | static inline void rzg2l_du_vsp_enable(struct rzg2l_du_crtc *crtc) { };
  |^~~
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:61:6: error: redefinition of 
'rzg2l_du_vsp_disable'
   61 | void rzg2l_du_vsp_disable(struct rzg2l_du_crtc *crtc)
  |  ^~~~
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:73:20: note: previous definition 
of 'rzg2l_du_vsp_disable' with type 'void(struct rzg2l_du_crtc *)'
   73 | static inline void rzg2l_du_vsp_disable(struct rzg2l_du_crtc *crtc) { };
  |^~~~
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:66:6: error: redefinition of 
'rzg2l_du_vsp_atomic_flush'
   66 | void rzg2l_du_vsp_atomic_flush(struct rzg2l_du_crtc *crtc)
  |  ^
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:74:20: note: previous definition 
of 'rzg2l_du_vsp_atomic_flush' with type 'void(struct rzg2l_du_crtc *)'
   74 | static inline void rzg2l_du_vsp_atomic_flush(struct rzg2l_du_crtc 
*crtc) { };
  |^
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:76:19: error: redefinition of 
'rzg2l_du_vsp_get_drm_plane'
   76 | struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct rzg2l_du_crtc *crtc,
  |   ^~
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:75:33: note: previous definition 
of 'rzg2l_du_vsp_get_drm_plane' with type 'struct drm_plane *(struct 
rzg2l_du_crtc *, unsigned int)'
   75 | static inline struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct 
rzg2l_du_crtc *crtc,
  | ^~
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:302:5: error: redefinition of 
'rzg2l_du_vsp_init'
  302 | int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct device_node *np,
  | ^
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:66:19: note: previous definition 
of 'rzg2l_du_vsp_init' with type 'int(struct rzg2l_du_vsp *, struct device_node 
*, unsigned int)'
   66 | static inline int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct 
device_node *np,
  |   ^

Caused by commit

  768e9e61b3b9 ("drm: renesas: Add RZ/G2L DU Support")

I have used the drm-misc tree from next-20240221 for today.

-- 
Cheers,
Stephen Rothwell


pgpPN1_TME7xx.pgp
Description: OpenPGP digital signature


Re: [PATCH 1/6] drm/bridge: aux-hpd: fix OF node leaks

2024-02-21 Thread Bjorn Andersson
On Sat, Feb 17, 2024 at 04:02:23PM +0100, Johan Hovold wrote:
> The two device node references taken during allocation need to be
> dropped when the auxiliary device is freed.
> 
> Fixes: 6914968a0b52 ("drm/bridge: properly refcount DT nodes in aux bridge 
> drivers")
> Cc: Dmitry Baryshkov 
> Cc: Neil Armstrong 
> Signed-off-by: Johan Hovold 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/gpu/drm/bridge/aux-hpd-bridge.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c 
> b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> index bb55f697a181..9e71daf95bde 100644
> --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> @@ -25,6 +25,7 @@ static void drm_aux_hpd_bridge_release(struct device *dev)
>   ida_free(_aux_hpd_bridge_ida, adev->id);
>  
>   of_node_put(adev->dev.platform_data);
> + of_node_put(adev->dev.of_node);
>  
>   kfree(adev);
>  }
> @@ -74,6 +75,8 @@ struct device *drm_dp_hpd_bridge_register(struct device 
> *parent,
>  
>   ret = auxiliary_device_init(adev);
>   if (ret) {
> + of_node_put(adev->dev.platform_data);
> + of_node_put(adev->dev.of_node);
>   ida_free(_aux_hpd_bridge_ida, adev->id);
>   kfree(adev);
>   return ERR_PTR(ret);
> -- 
> 2.43.0
> 


[PATCH v2] drivers/i915/intel_bios: Fix parsing backlight BDB data

2024-02-21 Thread Karthikeyan Ramasubramanian
Starting BDB version 239, hdr_dpcd_refresh_timeout is introduced to
backlight BDB data. Commit 700034566d68 ("drm/i915/bios: Define more BDB
contents") updated the backlight BDB data accordingly. This broke the
parsing of backlight BDB data in VBT for versions 236 - 238 (both
inclusive) and hence the backlight controls are not responding on units
with the concerned BDB version.

backlight_control information has been present in backlight BDB data
from at least BDB version 191 onwards, if not before. Hence this patch
extracts the backlight_control information for BDB version 191 or newer.
Tested on Chromebooks using Jasperlake SoC (reports bdb->version = 236).
Tested on Chromebooks using Raptorlake SoC (reports bdb->version = 251).

Fixes: 700034566d68 ("drm/i915/bios: Define more BDB contents")
Cc: sta...@vger.kernel.org
Cc: Jani Nikula 
Cc: Ville Syrjälä 
Signed-off-by: Karthikeyan Ramasubramanian 
---

Changes in v2:
- removed checking the block size of the backlight BDB data

 drivers/gpu/drm/i915/display/intel_bios.c | 19 ---
 drivers/gpu/drm/i915/display/intel_vbt_defs.h |  5 -
 2 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
b/drivers/gpu/drm/i915/display/intel_bios.c
index aa169b0055e97..8c1eb05fe77d2 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1042,22 +1042,11 @@ parse_lfp_backlight(struct drm_i915_private *i915,
panel->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI;
panel->vbt.backlight.controller = 0;
if (i915->display.vbt.version >= 191) {
-   size_t exp_size;
+   const struct lfp_backlight_control_method *method;
 
-   if (i915->display.vbt.version >= 236)
-   exp_size = sizeof(struct bdb_lfp_backlight_data);
-   else if (i915->display.vbt.version >= 234)
-   exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_234;
-   else
-   exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_191;
-
-   if (get_blocksize(backlight_data) >= exp_size) {
-   const struct lfp_backlight_control_method *method;
-
-   method = _data->backlight_control[panel_type];
-   panel->vbt.backlight.type = method->type;
-   panel->vbt.backlight.controller = method->controller;
-   }
+   method = _data->backlight_control[panel_type];
+   panel->vbt.backlight.type = method->type;
+   panel->vbt.backlight.controller = method->controller;
}
 
panel->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h 
b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index a9f44abfc9fc2..b50cd0dcabda9 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -897,11 +897,6 @@ struct lfp_brightness_level {
u16 reserved;
 } __packed;
 
-#define EXP_BDB_LFP_BL_DATA_SIZE_REV_191 \
-   offsetof(struct bdb_lfp_backlight_data, brightness_level)
-#define EXP_BDB_LFP_BL_DATA_SIZE_REV_234 \
-   offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits)
-
 struct bdb_lfp_backlight_data {
u8 entry_size;
struct lfp_backlight_data_entry data[16];
-- 
2.44.0.rc0.258.g7320e95886-goog



[PATCH] gpu: host1x: Skip reset assert on Tegra186

2024-02-21 Thread Mikko Perttunen
From: Mikko Perttunen 

On Tegra186, secure world applications may need to access host1x
during suspend/resume, and rely on the kernel to keep Host1x out
of reset during the suspend cycle. As such, as a quirk,
skip asserting Host1x's reset on Tegra186.

We don't need to keep the clocks enabled, as BPMP ensures the clock
stays on while Host1x is being used. On newer SoC's, the reset line
is inaccessible, so there is no need for the quirk.

Signed-off-by: Mikko Perttunen 
---
 drivers/gpu/host1x/dev.c | 15 +--
 drivers/gpu/host1x/dev.h |  6 ++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 42fd504abbcd..89983d7d73ca 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -169,6 +169,7 @@ static const struct host1x_info host1x06_info = {
.num_sid_entries = ARRAY_SIZE(tegra186_sid_table),
.sid_table = tegra186_sid_table,
.reserve_vblank_syncpts = false,
+   .skip_reset_assert = true,
 };
 
 static const struct host1x_sid_entry tegra194_sid_table[] = {
@@ -680,13 +681,15 @@ static int __maybe_unused host1x_runtime_suspend(struct 
device *dev)
host1x_intr_stop(host);
host1x_syncpt_save(host);
 
-   err = reset_control_bulk_assert(host->nresets, host->resets);
-   if (err) {
-   dev_err(dev, "failed to assert reset: %d\n", err);
-   goto resume_host1x;
-   }
+   if (!host->info->skip_reset_assert) {
+   err = reset_control_bulk_assert(host->nresets, host->resets);
+   if (err) {
+   dev_err(dev, "failed to assert reset: %d\n", err);
+   goto resume_host1x;
+   }
 
-   usleep_range(1000, 2000);
+   usleep_range(1000, 2000);
+   }
 
clk_disable_unprepare(host->clk);
reset_control_bulk_release(host->nresets, host->resets);
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index c8e302de7625..6143c2a61d70 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -116,6 +116,12 @@ struct host1x_info {
 * the display driver disables VBLANK increments.
 */
bool reserve_vblank_syncpts;
+   /*
+* On T186, secure world applications may require access to host1x
+* during suspend/resume. To allow this, we need to leave host1x
+* not in reset.
+*/
+   bool skip_reset_assert;
 };
 
 struct host1x {
-- 
2.42.0



Re: [PATCH v2 1/3] drm/syncobj: call drm_syncobj_fence_add_wait when WAIT_AVAILABLE flag is set

2024-02-21 Thread Dave Airlie
Can someone pick these up into misc?

Dave.

On Thu, 22 Feb 2024 at 04:48, Erik Kurzinger  wrote:
>
> It looks these these patches have still not been merged after one month, is 
> there anything more that needs to be done for this to happen?
>
> On 1/25/24 10:12, Daniel Vetter wrote:
> > On Fri, Jan 19, 2024 at 08:32:06AM -0800, Erik Kurzinger wrote:
> >> When waiting for a syncobj timeline point whose fence has not yet been
> >> submitted with the WAIT_FOR_SUBMIT flag, a callback is registered using
> >> drm_syncobj_fence_add_wait and the thread is put to sleep until the
> >> timeout expires. If the fence is submitted before then,
> >> drm_syncobj_add_point will wake up the sleeping thread immediately which
> >> will proceed to wait for the fence to be signaled.
> >>
> >> However, if the WAIT_AVAILABLE flag is used instead,
> >> drm_syncobj_fence_add_wait won't get called, meaning the waiting thread
> >> will always sleep for the full timeout duration, even if the fence gets
> >> submitted earlier. If it turns out that the fence *has* been submitted
> >> by the time it eventually wakes up, it will still indicate to userspace
> >> that the wait completed successfully (it won't return -ETIME), but it
> >> will have taken much longer than it should have.
> >>
> >> To fix this, we must call drm_syncobj_fence_add_wait if *either* the
> >> WAIT_FOR_SUBMIT flag or the WAIT_AVAILABLE flag is set. The only
> >> difference being that with WAIT_FOR_SUBMIT we will also wait for the
> >> fence to be signaled after it has been submitted while with
> >> WAIT_AVAILABLE we will return immediately.
> >>
> >> IGT test patch: 
> >> https://lists.freedesktop.org/archives/igt-dev/2024-January/067537.html
> >>
> >> v1 -> v2: adjust lockdep_assert_none_held_once condition
> >>
> >> Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8")
> >> Signed-off-by: Erik Kurzinger 
> >
> > Yeah I think this series catches now all the corner cases I spotted in v1.
> > On the series:
> >
> > Reviewed-by: Daniel Vetter 
> >> ---
> >>  drivers/gpu/drm/drm_syncobj.c | 6 --
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> >> index 94ebc71e5be5..97be8b140599 100644
> >> --- a/drivers/gpu/drm/drm_syncobj.c
> >> +++ b/drivers/gpu/drm/drm_syncobj.c
> >> @@ -1058,7 +1058,8 @@ static signed long 
> >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> >>  uint64_t *points;
> >>  uint32_t signaled_count, i;
> >>
> >> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)
> >> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> >> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE))
> >>  lockdep_assert_none_held_once();
> >>
> >>  points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
> >> @@ -1127,7 +1128,8 @@ static signed long 
> >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> >>   * fallthough and try a 0 timeout wait!
> >>   */
> >>
> >> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
> >> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> >> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) {
> >>  for (i = 0; i < count; ++i)
> >>  drm_syncobj_fence_add_wait(syncobjs[i], [i]);
> >>  }
> >> --
> >> 2.43.0
> >>
> >
>


Re: [PATCH 7/9] arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> The RB3gen2 has a USB redriver on APPS_I2C, enable the bus and introduce
> the redriver. The plumbing with other components is kept separate for
> clarity.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 14 ++
>  1 file changed, 14 insertions(+)

Reviewed-by: Dmitry Baryshkov 




-- 
With best wishes
Dmitry


Re: [PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> With the ADSP remoteproc loaded pmic_glink can be introduced and wired
> up to provide role and orientation switching signals.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 
> +++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
> b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index ab498494caea..079bf43b14cc 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 {
> };
> };
>
> +   pmic-glink {
> +   compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink";
> +
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   connector@0 {
> +   compatible = "usb-c-connector";
> +   reg = <0>;
> +   power-role = "dual";
> +   data-role = "dual";
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   port@0 {
> +   reg = <0>;
> +
> +   pmic_glink_hs_in: endpoint {
> +   remote-endpoint = 
> <_1_dwc3_hs>;
> +   };
> +   };
> +
> +   port@1 {
> +   reg = <1>;
> +
> +   pmic_glink_ss_in: endpoint {
> +   remote-endpoint = 
> <_1_dwc3_ss>;

This should be connected to the QMP PHY rather than to the USB host.

Also it might be better to squash this patch with the patch 8. Or at
least to get redriver into the picture in this patch (and keep only
display-related parts in that patch).


> +   };
> +   };
> +   };
> +   };
> +   };
> +
> vph_pwr: vph-pwr-regulator {
> compatible = "regulator-fixed";
> regulator-name = "vph_pwr";
> @@ -476,7 +511,16 @@ _1 {
>  };
>
>  _1_dwc3 {
> -   dr_mode = "peripheral";
> +   dr_mode = "otg";
> +   usb-role-switch;
> +};
> +
> +_1_dwc3_hs {
> +   remote-endpoint = <_glink_hs_in>;
> +};
> +
> +_1_dwc3_ss {
> +   remote-endpoint = <_glink_ss_in>;
>  };
>
>  _1_hsphy {
> @@ -491,6 +535,8 @@ _1_qmpphy {
> vdda-phy-supply = <_l6b_1p2>;
> vdda-pll-supply = <_l1b_0p912>;
>
> +   orientation-switch;
> +
> status = "okay";
>  };
>
>
> --
> 2.25.1
>


--
With best wishes
Dmitry


Re: [PATCH 4/9] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> The RB3Gen2 board comes with a mini DP connector, describe this, enable
> MDSS, DP controller and the PHY that drives this.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++
>  1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
> b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index ac4579119d3b..32313f47602a 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -430,6 +430,23 @@  {
>;
>  };
>
> + {
> +   status = "okay";
> +};
> +
> +_edp {
> +   status = "okay";
> +};
> +
> +_edp_out {
> +   data-lanes = <0 1 2 3>;
> +   link-frequencies = /bits/ 64 <162000 27 54 
> 81>;
> +};

Please add a corresponding dp-connector device and link it to the mdss_edp_out.

> +
> +_edp_phy {
> +   status = "okay";
> +};
> +
>  _id_0 {
> status = "okay";
>  };
> @@ -470,3 +487,9 @@ _1_qmpphy {
>   {
> memory-region = <_fw_mem>;
>  };
> +
> +/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
> +
> +_hot_plug_det {
> +   bias-disable;
> +};
>
> --
> 2.25.1
>


--
With best wishes
Dmitry


Re: [PATCH 9/9] arm64: defconfig: Enable sc7280 display and gpu clock controllers

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> Enable the SC7280 display and gpu clock controllers to enable display
> support on the QCS6490 RB3gen2.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/configs/defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
Reviewed-by: Dmitry Baryshkov 


-- 
With best wishes
Dmitry


Re: [PATCH 2/9] arm64: dts: qcom: sc7280: Make eDP/DP controller default DP

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> The newly introduced mechanism for selecting eDP mode allow us to make a
> DisplayPort controller operate in eDP mode, but not the other way
> around. The qcom,sc7280-edp compatible is obviously tied to eDP, so this
> would not allow us to select DisplayPort-mode.
>
> Switch the compatible of the mdss_edp instance and make it eDP for the
> SC7280 qcard.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 ++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi   | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Dmitry Baryshkov 


-- 
With best wishes
Dmitry


Re: [PATCH 5/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> Define firmware paths and enable the ADSP and CDSP remoteprocs.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
> b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 32313f47602a..ab498494caea 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -451,6 +451,16 @@ _id_0 {
> status = "okay";
>  };
>
> +_adsp {
> +   firmware-name = "qcom/qcs6490/rb3gen2/adsp.mbn";

This should be either

   firmware-name = "qcom/qcs6490/adsp.mbn";

or

   firmware-name = "qcom/qcs6490/Vendor/rb3gen2/adsp.mbn";


> +   status = "okay";
> +};
> +
> +_cdsp {
> +   firmware-name = "qcom/qcs6490/rb3gen2/cdsp.mbn";
> +   status = "okay";
> +};
> +
>   {
> gpio-reserved-ranges = <32 2>, /* ADSP */
><48 4>; /* NFC */
>
> --
> 2.25.1
>


-- 
With best wishes
Dmitry


Re: [PATCH 3/9] arm64: dts: qcom: sc7280: Enable MDP turbo mode

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> The max frequency listed in the DPU opp-table is 506MHz, this is not
> sufficient to drive a 4k@60 display, resulting in constant underrun.
>
> Add the missing MDP_CLK turbo frequency of 608MHz to the opp-table to
> fix this.

I think we might want to keep this disabled for ChromeOS devices. Doug?

>
> Signed-off-by: Bjorn Andersson 
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index a19c278ebec9..a2a6717c6c87 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -4417,6 +4417,11 @@ opp-50667 {
> opp-hz = /bits/ 64 
> <50667>;
> required-opps = 
> <_opp_nom>;
> };
> +
> +   opp-60800 {
> +   opp-hz = /bits/ 64 
> <60800>;
> +   required-opps = 
> <_opp_turbo>;
> +   };
> };
> };
>
>
> --
> 2.25.1
>


-- 
With best wishes
Dmitry


Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280

2024-02-21 Thread Dmitry Baryshkov
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson  wrote:
>
> When upstreamed the SC7280 DP controllers where described as one being
> DP and one eDP, but they can infact both be DP or eDP.
>
> Extend the list of DP controllers to cover both instances, and rely on
> the newly introduced mechanism for selecting which mode they should
> operate in.
>
> Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree
> will continue to select eDP.
>
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 7b8c695d521a..1792ba9f7259 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = {
>  };
>
>  static const struct msm_dp_desc sc7280_dp_descs[] = {
> -   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type 
> = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
> +   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = 
> true },
> +   { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = 
> true },

I think we need to keep .connector_type here, don't we?

> +   {}
> +};
> +
> +static const struct msm_dp_desc sc7280_edp_descs[] = {
> { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type 
> = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
> {}
>  };
> @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
>  static const struct of_device_id dp_dt_match[] = {
> { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
> { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> -   { .compatible = "qcom,sc7280-edp", .data = _dp_descs },
> +   { .compatible = "qcom,sc7280-edp", .data = _edp_descs },
> { .compatible = "qcom,sc8180x-dp", .data = _dp_descs },
> { .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
> { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
>
> --
> 2.25.1
>


-- 
With best wishes
Dmitry


[PATCH 4/9] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output

2024-02-21 Thread Bjorn Andersson
The RB3Gen2 board comes with a mini DP connector, describe this, enable
MDSS, DP controller and the PHY that drives this.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index ac4579119d3b..32313f47602a 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -430,6 +430,23 @@  {
   ;
 };
 
+ {
+   status = "okay";
+};
+
+_edp {
+   status = "okay";
+};
+
+_edp_out {
+   data-lanes = <0 1 2 3>;
+   link-frequencies = /bits/ 64 <162000 27 54 
81>;
+};
+
+_edp_phy {
+   status = "okay";
+};
+
 _id_0 {
status = "okay";
 };
@@ -470,3 +487,9 @@ _1_qmpphy {
  {
memory-region = <_fw_mem>;
 };
+
+/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
+
+_hot_plug_det {
+   bias-disable;
+};

-- 
2.25.1



[PATCH 9/9] arm64: defconfig: Enable sc7280 display and gpu clock controllers

2024-02-21 Thread Bjorn Andersson
Enable the SC7280 display and gpu clock controllers to enable display
support on the QCS6490 RB3gen2.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b8adb28185ad..193d504041dd 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1283,6 +1283,7 @@ CONFIG_QCM_DISPCC_2290=m
 CONFIG_QCS_GCC_404=y
 CONFIG_QDU_GCC_1000=y
 CONFIG_SC_CAMCC_8280XP=m
+CONFIG_SC_DISPCC_7280=m
 CONFIG_SC_DISPCC_8280XP=m
 CONFIG_SA_GCC_8775P=y
 CONFIG_SA_GPUCC_8775P=m
@@ -1290,6 +1291,7 @@ CONFIG_SC_GCC_7180=y
 CONFIG_SC_GCC_7280=y
 CONFIG_SC_GCC_8180X=y
 CONFIG_SC_GCC_8280XP=y
+CONFIG_SC_GPUCC_7280=m
 CONFIG_SC_GPUCC_8280XP=m
 CONFIG_SC_LPASSCC_8280XP=m
 CONFIG_SDM_CAMCC_845=m

-- 
2.25.1



[PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280

2024-02-21 Thread Bjorn Andersson
When upstreamed the SC7280 DP controllers where described as one being
DP and one eDP, but they can infact both be DP or eDP.

Extend the list of DP controllers to cover both instances, and rely on
the newly introduced mechanism for selecting which mode they should
operate in.

Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree
will continue to select eDP.

Signed-off-by: Bjorn Andersson 
---
 drivers/gpu/drm/msm/dp/dp_display.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 7b8c695d521a..1792ba9f7259 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = {
 };
 
 static const struct msm_dp_desc sc7280_dp_descs[] = {
-   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type = 
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
+   { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = 
true },
+   { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = 
true },
+   {}
+};
+
+static const struct msm_dp_desc sc7280_edp_descs[] = {
{ .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type = 
DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
{}
 };
@@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
 static const struct of_device_id dp_dt_match[] = {
{ .compatible = "qcom,sc7180-dp", .data = _dp_descs },
{ .compatible = "qcom,sc7280-dp", .data = _dp_descs },
-   { .compatible = "qcom,sc7280-edp", .data = _dp_descs },
+   { .compatible = "qcom,sc7280-edp", .data = _edp_descs },
{ .compatible = "qcom,sc8180x-dp", .data = _dp_descs },
{ .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
{ .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },

-- 
2.25.1



[PATCH 5/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp

2024-02-21 Thread Bjorn Andersson
Define firmware paths and enable the ADSP and CDSP remoteprocs.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 32313f47602a..ab498494caea 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -451,6 +451,16 @@ _id_0 {
status = "okay";
 };
 
+_adsp {
+   firmware-name = "qcom/qcs6490/rb3gen2/adsp.mbn";
+   status = "okay";
+};
+
+_cdsp {
+   firmware-name = "qcom/qcs6490/rb3gen2/cdsp.mbn";
+   status = "okay";
+};
+
  {
gpio-reserved-ranges = <32 2>, /* ADSP */
   <48 4>; /* NFC */

-- 
2.25.1



[PATCH 8/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display

2024-02-21 Thread Bjorn Andersson
With MDSS, pmic_glink, and the redriver in place, wire up the various
components to enable USB Type-C display on the RB3gen2.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 63 +++-
 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 171ed979d55f..4bf1c6351467 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -149,7 +149,15 @@ port@1 {
reg = <1>;
 
pmic_glink_ss_in: endpoint {
-   remote-endpoint = 
<_1_dwc3_ss>;
+   remote-endpoint = 
<_usb_con_ss>;
+   };
+   };
+
+   port@2 {
+   reg = <2>;
+
+   pmic_glink_sbu_in: endpoint {
+   remote-endpoint = 
<_usb_con_sbu>;
};
};
};
@@ -476,6 +484,36 @@ typec-mux@1c {
 
retimer-switch;
orientation-switch;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   redriver_usb_con_ss: endpoint {
+   remote-endpoint = <_glink_ss_in>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   redriver_phy_con_ss: endpoint {
+   remote-endpoint = <_dp_qmpphy_out>;
+   data-lanes = <0 1 2 3>;
+   };
+   };
+
+   port@2 {
+   reg = <2>;
+
+   redriver_usb_con_sbu: endpoint {
+   remote-endpoint = <_glink_sbu_in>;
+   };
+   };
+   };
};
 };
 
@@ -483,6 +521,15 @@  {
status = "okay";
 };
 
+_dp {
+   status = "okay";
+};
+
+_dp_out {
+   data-lanes = <0 1>;
+   remote-endpoint = <_dp_qmpphy_dp_in>;
+};
+
 _edp {
status = "okay";
 };
@@ -534,7 +581,7 @@ _1_dwc3_hs {
 };
 
 _1_dwc3_ss {
-   remote-endpoint = <_glink_ss_in>;
+   remote-endpoint = <_dp_qmpphy_usb_ss_in>;
 };
 
 _1_hsphy {
@@ -554,6 +601,18 @@ _1_qmpphy {
status = "okay";
 };
 
+_dp_qmpphy_out {
+   remote-endpoint = <_phy_con_ss>;
+};
+
+_dp_qmpphy_usb_ss_in {
+   remote-endpoint = <_1_dwc3_ss>;
+};
+
+_dp_qmpphy_dp_in {
+   remote-endpoint = <_dp_out>;
+};
+
  {
memory-region = <_fw_mem>;
 };

-- 
2.25.1



[PATCH 2/9] arm64: dts: qcom: sc7280: Make eDP/DP controller default DP

2024-02-21 Thread Bjorn Andersson
The newly introduced mechanism for selecting eDP mode allow us to make a
DisplayPort controller operate in eDP mode, but not the other way
around. The qcom,sc7280-edp compatible is obviously tied to eDP, so this
would not allow us to select DisplayPort-mode.

Switch the compatible of the mdss_edp instance and make it eDP for the
SC7280 qcard.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 ++
 arch/arm64/boot/dts/qcom/sc7280.dtsi   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi 
b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
index f9b96bd2477e..e339b181a9ac 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
@@ -348,6 +348,8 @@ _va_macro {
 
 /* NOTE: Not all Qcards have eDP connector stuffed */
 _edp {
+   is-edp;
+
aux-bus {
edp_panel: panel {
compatible = "edp-panel";
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 581818676a4c..a19c278ebec9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -4513,7 +4513,7 @@ mdss_dsi_phy: phy@ae94400 {
};
 
mdss_edp: edp@aea {
-   compatible = "qcom,sc7280-edp";
+   compatible = "qcom,sc7280-dp";
pinctrl-names = "default";
pinctrl-0 = <_hot_plug_det>;
 

-- 
2.25.1



[PATCH 7/9] arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver

2024-02-21 Thread Bjorn Andersson
The RB3gen2 has a USB redriver on APPS_I2C, enable the bus and introduce
the redriver. The plumbing with other components is kept separate for
clarity.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 079bf43b14cc..171ed979d55f 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -465,6 +465,20 @@  {
   ;
 };
 
+ {
+   status = "okay";
+
+   typec-mux@1c {
+   compatible = "onnn,nb7vpq904m";
+   reg = <0x1c>;
+
+   vcc-supply = <_l18b_1p8>;
+
+   retimer-switch;
+   orientation-switch;
+   };
+};
+
  {
status = "okay";
 };

-- 
2.25.1



[PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching

2024-02-21 Thread Bjorn Andersson
With the ADSP remoteproc loaded pmic_glink can be introduced and wired
up to provide role and orientation switching signals.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts 
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index ab498494caea..079bf43b14cc 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 {
};
};
 
+   pmic-glink {
+   compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink";
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   connector@0 {
+   compatible = "usb-c-connector";
+   reg = <0>;
+   power-role = "dual";
+   data-role = "dual";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   pmic_glink_hs_in: endpoint {
+   remote-endpoint = 
<_1_dwc3_hs>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   pmic_glink_ss_in: endpoint {
+   remote-endpoint = 
<_1_dwc3_ss>;
+   };
+   };
+   };
+   };
+   };
+
vph_pwr: vph-pwr-regulator {
compatible = "regulator-fixed";
regulator-name = "vph_pwr";
@@ -476,7 +511,16 @@ _1 {
 };
 
 _1_dwc3 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
+   usb-role-switch;
+};
+
+_1_dwc3_hs {
+   remote-endpoint = <_glink_hs_in>;
+};
+
+_1_dwc3_ss {
+   remote-endpoint = <_glink_ss_in>;
 };
 
 _1_hsphy {
@@ -491,6 +535,8 @@ _1_qmpphy {
vdda-phy-supply = <_l6b_1p2>;
vdda-pll-supply = <_l1b_0p912>;
 
+   orientation-switch;
+
status = "okay";
 };
 

-- 
2.25.1



[PATCH 3/9] arm64: dts: qcom: sc7280: Enable MDP turbo mode

2024-02-21 Thread Bjorn Andersson
The max frequency listed in the DPU opp-table is 506MHz, this is not
sufficient to drive a 4k@60 display, resulting in constant underrun.

Add the missing MDP_CLK turbo frequency of 608MHz to the opp-table to
fix this.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index a19c278ebec9..a2a6717c6c87 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -4417,6 +4417,11 @@ opp-50667 {
opp-hz = /bits/ 64 <50667>;
required-opps = 
<_opp_nom>;
};
+
+   opp-60800 {
+   opp-hz = /bits/ 64 <60800>;
+   required-opps = 
<_opp_turbo>;
+   };
};
};
 

-- 
2.25.1



[PATCH 0/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable two displays

2024-02-21 Thread Bjorn Andersson
RB3Gen2 is capable of producing DisplayPort output on a dedicated
mini-DP connector and USB Type-C.

Utilize Abel's work for DP vs eDP selection to allow configuring both
controllers in DP-mode, then enable the two output paths.

Tested by driving fbcon to 4k@60 + 4k@30 concurrently.

Depends on 
https://lore.kernel.org/linux-arm-msm/20240220-x1e80100-display-v4-0-971afd9de...@linaro.org/

Signed-off-by: Bjorn Andersson 
---
Bjorn Andersson (9):
  drm/msm/dp: Add DP support to combo instance in SC7280
  arm64: dts: qcom: sc7280: Make eDP/DP controller default DP
  arm64: dts: qcom: sc7280: Enable MDP turbo mode
  arm64: dts: qcom: qcs6490-rb3gen2: Add DP output
  arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp
  arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching
  arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver
  arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display
  arm64: defconfig: Enable sc7280 display and gpu clock controllers

 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 154 ++-
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi   |   2 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi |   7 +-
 arch/arm64/configs/defconfig |   2 +
 drivers/gpu/drm/msm/dp/dp_display.c  |   9 +-
 5 files changed, 170 insertions(+), 4 deletions(-)
---
base-commit: aba508318eec7acad2373296279d6447fd35f83f
change-id: 20240209-rb3gen2-dp-connector-bddfb892ff20

Best regards,
-- 
Bjorn Andersson 



Re: [PATCH] drm/amd/display: Use kcalloc() instead of kzalloc()

2024-02-21 Thread Rodrigo Siqueira Jordao




On 1/28/24 02:04, Lenko Donchev wrote:

We are trying to get rid of all multiplications from allocation
functions to prevent integer overflows. Here the multiplication is
obviously safe, but using kcalloc() is more appropriate and improves
readability. This patch has no effect on runtime behavior.

Link: 
https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
Link: https://github.com/KSPP/linux/issues/162

Signed-off-by: Lenko Donchev 
---
  drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c 
b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c
index 5c9a30211c10..b67cd78e7c58 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c
@@ -164,7 +164,7 @@ static void dpcd_extend_address_range(
if (new_addr_range.start != in_address || new_addr_range.end != 
end_address) {
*out_address = new_addr_range.start;
*out_size = ADDRESS_RANGE_SIZE(new_addr_range.start, 
new_addr_range.end);
-   *out_data = kzalloc(*out_size * sizeof(**out_data), GFP_KERNEL);
+   *out_data = kcalloc(*out_size, sizeof(**out_data), GFP_KERNEL);
}
  }
  


lgtm,

Reviewed-by: Rodrigo Siqueira 


Re: Future handling of complex RGB devices on Linux v2

2024-02-21 Thread Pavel Machek
Hi!

> so after more feedback from the OpenRGB maintainers I came up with an even
> more generic proposal:
> https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1753072869

> >evaluate-set-command ioctl taking:
> >{
> >    enum command                /* one of supported_commands */
> >    union data
> >    {
> >        char raw[3072],
> >        {
> >            
> >        },

Yeah, so ... this is not a interface. This is a backdoor to pass
arbitrary data. That's not going to fly.

For keyboards, we don't need complete new interface; we reasonable
extensions over existing display APIs -- keyboards are clearly 2D.

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature


Re: [PATCH] phy: constify of_phandle_args in xlate

2024-02-21 Thread Florian Fainelli

On 2/17/24 01:39, Krzysztof Kozlowski wrote:

The xlate callbacks are supposed to translate of_phandle_args to proper
provider without modifying the of_phandle_args.  Make the argument
pointer to const for code safety and readability.

Signed-off-by: Krzysztof Kozlowski 


Acked-by: Florian Fainelli  #Broadcom
--
Florian



Re: [PATCH] phy: constify of_phandle_args in xlate

2024-02-21 Thread Linus Walleij
On Sat, Feb 17, 2024 at 10:39 AM Krzysztof Kozlowski
 wrote:

> The xlate callbacks are supposed to translate of_phandle_args to proper
> provider without modifying the of_phandle_args.  Make the argument
> pointer to const for code safety and readability.
>
> Signed-off-by: Krzysztof Kozlowski 
(...)
>  drivers/pinctrl/tegra/pinctrl-tegra-xusb.c |  2 +-

Acked-by: Linus Walleij 

Yours,
Linus Walleij


Re: [PATCH] drm/xe: skip building debugfs code for CONFIG_DEBUG_FS=n

2024-02-21 Thread Stephen Rothwell
Hi Lucas,

On Wed, 21 Feb 2024 13:45:35 -0600 Lucas De Marchi  
wrote:
>
> I don't think it's needed since drm-xe-next covers what is in
> drm-xe-fixes. Please add other maintainers and mailing list:
> 
> M:  Oded Gabbay 
> M:  Thomas Hellström 
> L:  intel...@lists.freedesktop.org

Added.

-- 
Cheers,
Stephen Rothwell


pgpjHbRARM1Ui.pgp
Description: OpenPGP digital signature


Re: Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

2024-02-21 Thread Lucas De Marchi

On Wed, Feb 21, 2024 at 11:04:22PM +0200, Andy Shevchenko wrote:

On Wed, Feb 21, 2024 at 10:30:02PM +0200, Dmitry Baryshkov wrote:

On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi  wrote:


...


> +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)


Can sizeof() be used in assembly?

...


> -#define __GENMASK(h, l) \
> -   (((~UL(0)) - (UL(1) << (l)) + 1) & \
> -(~UL(0) >> (BITS_PER_LONG - 1 - (h
> -#define GENMASK(h, l) \
> -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))



> +#define __GENMASK(t, h, l) \
> +   (GENMASK_INPUT_CHECK(h, l) + \
> +(((t)~0ULL - ((t)(1) << (l)) + 1) & \
> +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)


Nevertheless, the use ~0ULL is not proper assembly, this broke initial
implementation using UL() / ULL().


indeed.





> -#define __GENMASK_ULL(h, l) \
> -   (((~ULL(0)) - (ULL(1) << (l)) + 1) & \
> -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h
> -#define GENMASK_ULL(h, l) \
> -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))


Ditto.


problem here seems actually because of the cast to the final type. My
previous impl was avoiding that, but was too verbose compared to this.

I will look at reverting this.

Lucas De Marchi




> +#define GENMASK(h, l)  __GENMASK(unsigned long,  h, l)
> +#define GENMASK_ULL(h, l)  __GENMASK(unsigned long long, h, l)
> +#define GENMASK_U8(h, l)   __GENMASK(u8,  h, l)
> +#define GENMASK_U16(h, l)  __GENMASK(u16, h, l)
> +#define GENMASK_U32(h, l)  __GENMASK(u32, h, l)
> +#define GENMASK_U64(h, l)  __GENMASK(u64, h, l)

This breaks drm-tip on arm64 architecture:

arch/arm64/kernel/entry-fpsimd.S: Assembler messages:
465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters
following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned
long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned
long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)'
474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
480arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
481arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
482arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
483arch/arm64/kernel/entry-fpsimd.S:282: Error: unexpected characters
following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned
long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned
long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)'
484arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here


--
With Best Regards,
Andy Shevchenko




Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

2024-02-21 Thread Matthias Kaehlcke
On Wed, Feb 21, 2024 at 10:33:53PM +0100, Javier Carrasco wrote:
> On 21.02.24 22:18, Matthias Kaehlcke wrote:
>  +/*
>  + * Fallback supply names for backwards compatibility. If the device 
>  requires
>  + * more than the currently supported supplies, add a new one here, and 
>  if
>  + * possible, the real name supplies to the device-specific data.
>  + */
>  +static const char * const generic_supply_names[] = {
>  +"vdd",
>  +"vdd2",
>  +};
>  +
>  +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names)
> >>>
> >>> This will have to change when support for a device with more than 2 
> >>> non-generic
> >>> supply names gets added. Please use a literal value for MAX_SUPPLIES 
> >>> instead of
> >>> ARRAY_SIZE. If the literal is 2 it would still need to change for future 
> >>> devices
> >>> with more supplies, but that change would be more straighforward.
> >>>
> >>
> >> I am not completely sure about this. Someone could increase MAX_SUPPLIES
> >> without adding a generic name.
> > 
> > That's perfectly fine and intended. MAX_SUPPLIES is a max, any list
> > shorther than that is valid. Any longer list will result in probe()
> > being aborted with a clear error message.
> > 
> >> Actually two modifications will be necessary for every addition (name
> >> and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required,
> >> and MAX_SUPPLIES is automatically increased.
> > 
> > As per above it's not necessary to add a new name when MAX_SUPPLIES is
> > increased to support more non-generic names. It would only be necessary
> > if more generic names were added, my understanding is that this
> > should not happen because any newly supported onboard devices are
> > supposed to use device specific supply names. I don't like to idea of
> > adding unused pseudo supply names to the list, just for the sake of
> > using ARRAY_SIZE.
> > 
> >> I understand that the whole point of this is getting rid of the generic
> >> names, but we still have to provide generic names for every extra
> >> supply, at least for code consistency and to avoid size mismatches
> >> between real an generic supply names.
> > 
> > Please let me know if you still think the extra names are needed.
> 
> Not really, the only case I could come up is if an existing device that
> uses generic names might end up requiring a third supply, which would
> also be generic. But even such an unlikely event would be cover without
> ARRAY_SIZE.
> 
> Actually one could argue that every existing device could have "vdd" and
> "vdd2" as their supply names and remove checks and the generic array.

Sounds good to me. Another similar option would be to assign
'generic_supply_names' to '.supply_names'. I don't have a strong
preference.


Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

2024-02-21 Thread Javier Carrasco
On 21.02.24 22:18, Matthias Kaehlcke wrote:
 +/*
 + * Fallback supply names for backwards compatibility. If the device 
 requires
 + * more than the currently supported supplies, add a new one here, and if
 + * possible, the real name supplies to the device-specific data.
 + */
 +static const char * const generic_supply_names[] = {
 +  "vdd",
 +  "vdd2",
 +};
 +
 +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names)
>>>
>>> This will have to change when support for a device with more than 2 
>>> non-generic
>>> supply names gets added. Please use a literal value for MAX_SUPPLIES 
>>> instead of
>>> ARRAY_SIZE. If the literal is 2 it would still need to change for future 
>>> devices
>>> with more supplies, but that change would be more straighforward.
>>>
>>
>> I am not completely sure about this. Someone could increase MAX_SUPPLIES
>> without adding a generic name.
> 
> That's perfectly fine and intended. MAX_SUPPLIES is a max, any list
> shorther than that is valid. Any longer list will result in probe()
> being aborted with a clear error message.
> 
>> Actually two modifications will be necessary for every addition (name
>> and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required,
>> and MAX_SUPPLIES is automatically increased.
> 
> As per above it's not necessary to add a new name when MAX_SUPPLIES is
> increased to support more non-generic names. It would only be necessary
> if more generic names were added, my understanding is that this
> should not happen because any newly supported onboard devices are
> supposed to use device specific supply names. I don't like to idea of
> adding unused pseudo supply names to the list, just for the sake of
> using ARRAY_SIZE.
> 
>> I understand that the whole point of this is getting rid of the generic
>> names, but we still have to provide generic names for every extra
>> supply, at least for code consistency and to avoid size mismatches
>> between real an generic supply names.
> 
> Please let me know if you still think the extra names are needed.

Not really, the only case I could come up is if an existing device that
uses generic names might end up requiring a third supply, which would
also be generic. But even such an unlikely event would be cover without
ARRAY_SIZE.

Actually one could argue that every existing device could have "vdd" and
"vdd2" as their supply names and remove checks and the generic array.

Best regards,
Javier Carrasco



Re: [PATCH 7/9] drm: tests: Fix invalid printf format specifiers in KUnit tests

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:20PM +0800, David Gow wrote:
> The drm_buddy_test's alloc_contiguous test used a u64 for the page size,
> which was then updated to be an 'unsigned long' to avoid 64-bit
> multiplication division helpers.
>
> However, the variable is logged by some KUNIT_ASSERT_EQ_MSG() using the
> '%d' or '%llu' format specifiers, the former of which is always wrong,
> and the latter is no longer correct now that ps is no longer a u64. Fix
> these to all use '%lu'.
>
> Also, drm_mm_test calls KUNIT_FAIL() with an empty string as the
> message. gcc warns if a printf format string is empty (apparently), so

clang does too; under -Wformat-zero-length

> give these some more detailed error messages, which should be more
> useful anyway.
>
> Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
> Fixes: fca7526b7d89 ("drm/tests/drm_buddy: fix build failure on 32-bit 
> targets")
> Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
> Signed-off-by: David Gow 

Reviewed-by: Justin Stitt 
> ---
>  drivers/gpu/drm/tests/drm_buddy_test.c | 14 +++---
>  drivers/gpu/drm/tests/drm_mm_test.c|  6 +++---
>  2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c 
> b/drivers/gpu/drm/tests/drm_buddy_test.c
> index 8a464f7f4c61..3dbfa3078449 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -55,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit 
> *test)
>   KUNIT_ASSERT_FALSE_MSG(test,
>  drm_buddy_alloc_blocks(, 0, mm_size,
> ps, ps, list, 0),
> -"buddy_alloc hit an error size=%d\n",
> +"buddy_alloc hit an error size=%lu\n",
>  ps);
>   } while (++i < n_pages);
>
>   KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
>  3 * ps, ps, 
> ,
>  
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%d\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
>
>   drm_buddy_free_list(, );
>   KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
>  3 * ps, ps, 
> ,
>  
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
>   KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
>  2 * ps, ps, 
> ,
>  
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 2 * ps);
> +"buddy_alloc didn't error size=%lu\n", 2 * ps);
>
>   drm_buddy_free_list(, );
>   KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
>  3 * ps, ps, 
> ,
>  
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
>   /*
>* At this point we should have enough contiguous space for 2 blocks,
>* however they are never buddies (since we freed middle and right) so
> @@ -87,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit 
> *test)
>   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
>   2 * ps, ps, 
> ,
>   
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc hit an error size=%d\n", 2 * ps);
> +"buddy_alloc hit an error size=%lu\n", 2 * ps);
>
>   drm_buddy_free_list(, );
>   KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
>   3 * ps, ps, 
> ,
>   
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc hit an error size=%d\n", 3 * ps);
> +"buddy_alloc hit an error size=%lu\n", 3 * ps);
>
>   total = 0;
>   list_for_each_entry(block, , link)
> diff --git a/drivers/gpu/drm/tests/drm_mm_test.c 
> b/drivers/gpu/drm/tests/drm_mm_test.c
> index 1eb0c304f960..f37c0d765865 100644
> --- 

Re: [PATCH] drm/i915/guc: Add Compute context hint

2024-02-21 Thread Rodrigo Vivi
On Wed, Feb 21, 2024 at 09:42:34AM +, Tvrtko Ursulin wrote:
> 
> On 21/02/2024 00:14, Vinay Belgaumkar wrote:
> > Allow user to provide a context hint. When this is set, KMD will
> > send a hint to GuC which results in special handling for this
> > context. SLPC will ramp the GT frequency aggressively every time
> > it switches to this context. The down freq threshold will also be
> > lower so GuC will ramp down the GT freq for this context more slowly.
> > We also disable waitboost for this context as that will interfere with
> > the strategy.
> > 
> > We need to enable the use of Compute strategy during SLPC init, but
> > it will apply only to contexts that set this bit during context
> > creation.
> > 
> > Userland can check whether this feature is supported using a new param-
> > I915_PARAM_HAS_COMPUTE_CONTEXT. This flag is true for all guc submission
> > enabled platforms since they use SLPC for freq management.
> > 
> > The Mesa usage model for this flag is here -
> > https://gitlab.freedesktop.org/sushmave/mesa/-/commits/compute_hint
> 
> This allows for setting it for the whole application, correct? Upsides,
> downsides? Are there any plans for per context?

Currently there's no extension on a high level API (Vulkan/OpenGL/OpenCL/etc)
that would allow the application to hint for power/freq/latency. So Mesa cannot
decide when to hint. So their solution was to use .drirc and make 
per-application
decision.

I would prefer a high level extension for a more granular and informative
decision. We need to work with that goal, but for now I don't see any
cons on this approach.

> 
> > Cc: Rodrigo Vivi 
> > Signed-off-by: Vinay Belgaumkar 
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  8 +++
> >   .../gpu/drm/i915/gem/i915_gem_context_types.h |  1 +
> >   drivers/gpu/drm/i915/gt/intel_rps.c   |  8 +++
> >   .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h | 21 +++
> >   drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   | 17 +++
> >   drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |  1 +
> >   .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  7 +++
> >   drivers/gpu/drm/i915/i915_getparam.c  | 11 ++
> >   include/uapi/drm/i915_drm.h   | 15 +
> >   9 files changed, 89 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
> > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index dcbfe32fd30c..ceab7dbe9b47 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -879,6 +879,7 @@ static int set_proto_ctx_param(struct 
> > drm_i915_file_private *fpriv,
> >struct i915_gem_proto_context *pc,
> >struct drm_i915_gem_context_param *args)
> >   {
> > +   struct drm_i915_private *i915 = fpriv->i915;
> > int ret = 0;
> > switch (args->param) {
> > @@ -904,6 +905,13 @@ static int set_proto_ctx_param(struct 
> > drm_i915_file_private *fpriv,
> > pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> > break;
> > +   case I915_CONTEXT_PARAM_IS_COMPUTE:
> > +   if (!intel_uc_uses_guc_submission(_gt(i915)->uc))
> > +   ret = -EINVAL;
> > +   else
> > +   pc->user_flags |= BIT(UCONTEXT_COMPUTE);
> > +   break;
> > +
> > case I915_CONTEXT_PARAM_RECOVERABLE:
> > if (args->size)
> > ret = -EINVAL;
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h 
> > b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > index 03bc7f9d191b..db86d6f6245f 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > @@ -338,6 +338,7 @@ struct i915_gem_context {
> >   #define UCONTEXT_BANNABLE 2
> >   #define UCONTEXT_RECOVERABLE  3
> >   #define UCONTEXT_PERSISTENCE  4
> > +#define UCONTEXT_COMPUTE   5
> 
> What is the GuC behaviour when SLPC_CTX_FREQ_REQ_IS_COMPUTE is set for
> non-compute engines? Wondering if per intel_context is what we want instead.
> (Which could then be the i915_context_param_engines extension to mark
> individual contexts as compute strategy.)

Perhaps we should rename this? This is a freq-decision-strategy inside
GuC that is there mostly targeting compute workloads that needs lower
latency with short burst execution. But the engine itself doesn't matter.
It can be applied to any engine.

> 
> > /**
> >  * @flags: small set of booleans
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c 
> > b/drivers/gpu/drm/i915/gt/intel_rps.c
> > index 4feef874e6d6..1ed40cd61b70 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > @@ -24,6 +24,7 @@
> >   #include "intel_pcode.h"
> >   #include "intel_rps.h"
> >   #include "vlv_sideband.h"
> > +#include "../gem/i915_gem_context.h"
> >   

Re: [PATCH 6/9] net: test: Fix printf format specifier in skb_segment kunit test

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:19PM +0800, David Gow wrote:
> KUNIT_FAIL() accepts a printf-style format string, but previously did
> not let gcc validate it with the __printf() attribute. The use of %lld
> for the result of PTR_ERR() is not correct.
>
> Instead, use %pe and pass the actual error pointer. printk() will format
> it correctly (and give a symbolic name rather than a number if
> available, which should make the output more readable, too).
>
> Fixes: b3098d32ed6e ("net: add skb_segment kunit test")
> Signed-off-by: David Gow 

Looks good.

For those wondering, %pe has a special meaning in the kernel which can
be seen in lib/vsprintf.c.

Reviewed-by: Justin Stitt 
> ---
>  net/core/gso_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/gso_test.c b/net/core/gso_test.c
> index 4c2e77bd12f4..358c44680d91 100644
> --- a/net/core/gso_test.c
> +++ b/net/core/gso_test.c
> @@ -225,7 +225,7 @@ static void gso_test_func(struct kunit *test)
>
>   segs = skb_segment(skb, features);
>   if (IS_ERR(segs)) {
> - KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
> + KUNIT_FAIL(test, "segs error %pe", segs);
>   goto free_gso_skb;
>   } else if (!segs) {
>   KUNIT_FAIL(test, "no segments");
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

2024-02-21 Thread Matthias Kaehlcke
Hi Javier,

On Wed, Feb 21, 2024 at 09:40:38PM +0100, Javier Carrasco wrote:
> On 21.02.24 21:25, Matthias Kaehlcke wrote:
> > On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote:
> >> The current mechanism uses generic names for the power supplies, which
> >> conflicts with proper name definitions in the device bindings.
> >>
> >> Add a per-device property to include real supply names and keep generic
> >> names as a fallback mechanism for backward compatibility.
> >>
> >> Signed-off-by: Javier Carrasco 
> >> ---
> >>  drivers/usb/misc/onboard_usb_dev.c | 54 
> >> --
> >>  drivers/usb/misc/onboard_usb_dev.h | 23 
> >>  2 files changed, 52 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/usb/misc/onboard_usb_dev.c 
> >> b/drivers/usb/misc/onboard_usb_dev.c
> >> index f43130a6786f..e66fcac93006 100644
> >> --- a/drivers/usb/misc/onboard_usb_dev.c
> >> +++ b/drivers/usb/misc/onboard_usb_dev.c
> >> @@ -29,18 +29,6 @@
> >>  
> >>  #include "onboard_usb_dev.h"
> >>  
> >> -/*
> >> - * Use generic names, as the actual names might differ between devices. 
> >> If a new
> >> - * device requires more than the currently supported supplies, add a new 
> >> one
> >> - * here.
> >> - */
> >> -static const char * const supply_names[] = {
> >> -  "vdd",
> >> -  "vdd2",
> >> -};
> >> -
> >> -#define MAX_SUPPLIES ARRAY_SIZE(supply_names)
> >> -
> >>  static void onboard_dev_attach_usb_driver(struct work_struct *work);
> >>  
> >>  static struct usb_device_driver onboard_dev_usbdev_driver;
> >> @@ -66,6 +54,33 @@ struct onboard_dev {
> >>struct clk *clk;
> >>  };
> >>  
> >> +static int onboard_dev_get_regulator_bulk(struct device *dev,
> >> +struct onboard_dev *onboard_dev)
> >> +{
> >> +  unsigned int i;
> >> +  int err;
> >> +
> >> +  const char * const *supply_names = onboard_dev->pdata->supply_names;
> >> +
> >> +  if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES)
> >> +  return dev_err_probe(dev, -EINVAL, "max %zu supplies 
> >> supported!\n",
> >> +   MAX_SUPPLIES);
> >> +
> >> +  if (!supply_names[0])
> >> +  supply_names = generic_supply_names;
> > 
> > Please change to 'if (!supply_names)' and omit the initialization of
> > .supply_names for devices that use the generic supply names.
> > 
> 
> That looks much cleaner, I will apply it to the next version.
> 
> >> diff --git a/drivers/usb/misc/onboard_usb_dev.h 
> >> b/drivers/usb/misc/onboard_usb_dev.h
> >> index ebe83e19d818..59dced6bd339 100644
> >> --- a/drivers/usb/misc/onboard_usb_dev.h
> >> +++ b/drivers/usb/misc/onboard_usb_dev.h
> >> @@ -6,63 +6,86 @@
> >>  #ifndef _USB_MISC_ONBOARD_USB_DEV_H
> >>  #define _USB_MISC_ONBOARD_USB_DEV_H
> >>  
> >> +/*
> >> + * Fallback supply names for backwards compatibility. If the device 
> >> requires
> >> + * more than the currently supported supplies, add a new one here, and if
> >> + * possible, the real name supplies to the device-specific data.
> >> + */
> >> +static const char * const generic_supply_names[] = {
> >> +  "vdd",
> >> +  "vdd2",
> >> +};
> >> +
> >> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names)
> > 
> > This will have to change when support for a device with more than 2 
> > non-generic
> > supply names gets added. Please use a literal value for MAX_SUPPLIES 
> > instead of
> > ARRAY_SIZE. If the literal is 2 it would still need to change for future 
> > devices
> > with more supplies, but that change would be more straighforward.
> > 
> 
> I am not completely sure about this. Someone could increase MAX_SUPPLIES
> without adding a generic name.

That's perfectly fine and intended. MAX_SUPPLIES is a max, any list
shorther than that is valid. Any longer list will result in probe()
being aborted with a clear error message.

> Actually two modifications will be necessary for every addition (name
> and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required,
> and MAX_SUPPLIES is automatically increased.

As per above it's not necessary to add a new name when MAX_SUPPLIES is
increased to support more non-generic names. It would only be necessary
if more generic names were added, my understanding is that this
should not happen because any newly supported onboard devices are
supposed to use device specific supply names. I don't like to idea of
adding unused pseudo supply names to the list, just for the sake of
using ARRAY_SIZE.

> I understand that the whole point of this is getting rid of the generic
> names, but we still have to provide generic names for every extra
> supply, at least for code consistency and to avoid size mismatches
> between real an generic supply names.

Please let me know if you still think the extra names are needed.


Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

2024-02-21 Thread Andy Shevchenko
On Wed, Feb 21, 2024 at 11:06:10PM +0200, Andy Shevchenko wrote:
> On Wed, Feb 21, 2024 at 10:37:30PM +0200, Dmitry Baryshkov wrote:
> > On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov
> >  wrote:

...

> > Excuse me, it seems a c from gitlab didn't work as expected.
> 
> No problem, it's clear that the patch has not been properly tested and
> obviously wrong. Has to be dropped. If somebody wants that, it will
> be material for v6.10 cycle.

...which makes me think that bitmap(bitops) lack of assembly test case(s).

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH 5/9] rtc: test: Fix invalid format specifier.

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:18PM +0800, David Gow wrote:
> 'days' is a s64 (from div_s64), and so should use a %lld specifier.
>
> This was found by extending KUnit's assertion macros to use gcc's
> __printf attribute.
>
> Fixes: 1d1bb12a8b18 ("rtc: Improve performance of rtc_time64_to_tm(). Add 
> tests.")
> Signed-off-by: David Gow 

Reviewed-by: Justin Stitt 
> ---
>  drivers/rtc/lib_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/lib_test.c b/drivers/rtc/lib_test.c
> index d5caf36c56cd..225c859d6da5 100644
> --- a/drivers/rtc/lib_test.c
> +++ b/drivers/rtc/lib_test.c
> @@ -54,7 +54,7 @@ static void rtc_time64_to_tm_test_date_range(struct kunit 
> *test)
>
>   days = div_s64(secs, 86400);
>
> - #define FAIL_MSG "%d/%02d/%02d (%2d) : %ld", \
> + #define FAIL_MSG "%d/%02d/%02d (%2d) : %lld", \
>   year, month, mday, yday, days
>
>   KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year, 
> FAIL_MSG);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: [PATCH 4/9] time: test: Fix incorrect format specifier

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:17PM +0800, David Gow wrote:
> 'days' is a s64 (from div_s64), and so should use a %lld specifier.
>
> This was found by extending KUnit's assertion macros to use gcc's
> __printf attribute.
>
> Fixes: 276010551664 ("time: Improve performance of time64_to_tm()")
> Signed-off-by: David Gow 

Reviewed-by: Justin Stitt 
> ---
>  kernel/time/time_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/time_test.c b/kernel/time/time_test.c
> index ca058c8af6ba..3e5d422dd15c 100644
> --- a/kernel/time/time_test.c
> +++ b/kernel/time/time_test.c
> @@ -73,7 +73,7 @@ static void time64_to_tm_test_date_range(struct kunit *test)
>
>   days = div_s64(secs, 86400);
>
> - #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %ld", \
> + #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %lld", \
>   year, month, mdday, yday, days
>
>   KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year, 
> FAIL_MSG);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

2024-02-21 Thread Andy Shevchenko
On Wed, Feb 21, 2024 at 10:37:30PM +0200, Dmitry Baryshkov wrote:
> On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov
>  wrote:

...

> Excuse me, it seems a c from gitlab didn't work as expected.

No problem, it's clear that the patch has not been properly tested and
obviously wrong. Has to be dropped. If somebody wants that, it will
be material for v6.10 cycle.

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH 3/9] lib: memcpy_kunit: Fix an invalid format specifier in an assertion msg

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:16PM +0800, David Gow wrote:
> The 'i' passed as an assertion message is a size_t, so should use '%zu',
> not '%d'.
>
> This was found by annotating the _MSG() variants of KUnit's assertions
> to let gcc validate the format strings.
>
> Fixes: bb95ebbe89a7 ("lib: Introduce CONFIG_MEMCPY_KUNIT_TEST")
> Signed-off-by: David Gow 
> ---

Reviewed-by: Justin Stitt 
>  lib/memcpy_kunit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c
> index 440aee705ccc..30e00ef0bf2e 100644
> --- a/lib/memcpy_kunit.c
> +++ b/lib/memcpy_kunit.c
> @@ -32,7 +32,7 @@ struct some_bytes {
>   BUILD_BUG_ON(sizeof(instance.data) != 32);  \
>   for (size_t i = 0; i < sizeof(instance.data); i++) {\
>   KUNIT_ASSERT_EQ_MSG(test, instance.data[i], v, \
> - "line %d: '%s' not initialized to 0x%02x @ %d (saw 
> 0x%02x)\n", \
> + "line %d: '%s' not initialized to 0x%02x @ %zu (saw 
> 0x%02x)\n", \
>   __LINE__, #instance, v, i, instance.data[i]);   \
>   }   \
>  } while (0)
> @@ -41,7 +41,7 @@ struct some_bytes {
>   BUILD_BUG_ON(sizeof(one) != sizeof(two)); \
>   for (size_t i = 0; i < sizeof(one); i++) {  \
>   KUNIT_EXPECT_EQ_MSG(test, one.data[i], two.data[i], \
> - "line %d: %s.data[%d] (0x%02x) != %s.data[%d] 
> (0x%02x)\n", \
> + "line %d: %s.data[%zu] (0x%02x) != %s.data[%zu] 
> (0x%02x)\n", \
>   __LINE__, #one, i, one.data[i], #two, i, two.data[i]); \
>   }   \
>   kunit_info(test, "ok: " TEST_OP "() " name "\n");   \
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

2024-02-21 Thread Andy Shevchenko
On Wed, Feb 21, 2024 at 10:30:02PM +0200, Dmitry Baryshkov wrote:
> On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi  wrote:

...

> > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)

Can sizeof() be used in assembly?

...

> > -#define __GENMASK(h, l) \
> > -   (((~UL(0)) - (UL(1) << (l)) + 1) & \
> > -(~UL(0) >> (BITS_PER_LONG - 1 - (h
> > -#define GENMASK(h, l) \
> > -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))

> > +#define __GENMASK(t, h, l) \
> > +   (GENMASK_INPUT_CHECK(h, l) + \
> > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \
> > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)

Nevertheless, the use ~0ULL is not proper assembly, this broke initial
implementation using UL() / ULL().


> > -#define __GENMASK_ULL(h, l) \
> > -   (((~ULL(0)) - (ULL(1) << (l)) + 1) & \
> > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h
> > -#define GENMASK_ULL(h, l) \
> > -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))

Ditto.

> > +#define GENMASK(h, l)  __GENMASK(unsigned long,  h, l)
> > +#define GENMASK_ULL(h, l)  __GENMASK(unsigned long long, h, l)
> > +#define GENMASK_U8(h, l)   __GENMASK(u8,  h, l)
> > +#define GENMASK_U16(h, l)  __GENMASK(u16, h, l)
> > +#define GENMASK_U32(h, l)  __GENMASK(u32, h, l)
> > +#define GENMASK_U64(h, l)  __GENMASK(u64, h, l)
> 
> This breaks drm-tip on arm64 architecture:
> 
> arch/arm64/kernel/entry-fpsimd.S: Assembler messages:
> 465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
> 466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
> 467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
> 468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
> 469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
> 470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
> 471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
> 472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
> 473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters
> following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned
> long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned
> long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)'
> 474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
> 475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
> 476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
> 477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
> 478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
> 479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
> 480arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
> 481arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
> 482arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
> 483arch/arm64/kernel/entry-fpsimd.S:282: Error: unexpected characters
> following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned
> long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned
> long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)'
> 484arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v2 2/2] drm/i915/gt: Enable only one CCS for compute workload

2024-02-21 Thread Matt Roper
On Wed, Feb 21, 2024 at 01:12:18AM +0100, Andi Shyti wrote:
> Hi Matt,
> 
> thanks a lot for looking into this.
> 
> On Tue, Feb 20, 2024 at 03:39:18PM -0800, Matt Roper wrote:
> > On Tue, Feb 20, 2024 at 03:35:26PM +0100, Andi Shyti wrote:
> 
> [...]
> 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c 
> > > b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> > > index 833987015b8b..7041acc77810 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> > > @@ -243,6 +243,15 @@ void intel_engines_driver_register(struct 
> > > drm_i915_private *i915)
> > >   if (engine->uabi_class == I915_NO_UABI_CLASS)
> > >   continue;
> > >  
> > > + /*
> > > +  * Do not list and do not count CCS engines other than the first
> > > +  */
> > > + if (engine->uabi_class == I915_ENGINE_CLASS_COMPUTE &&
> > > + engine->uabi_instance > 0) {
> > > + i915->engine_uabi_class_count[engine->uabi_class]--;
> > > + continue;
> > > + }
> > 
> > Wouldn't it be simpler to just add a workaround to the end of
> > engine_mask_apply_compute_fuses() if we want to ensure only a single
> > compute engine gets exposed?  Then both the driver internals and uapi
> > will agree that's there's just one CCS (and on which one there is).
> > 
> > If we want to do something fancy with "hotplugging" a new engine later
> > on or whatever, that can be handled in the future series (although as
> > noted on the previous patch, it sounds like these changes might not
> > actually be aligned with the workaround we were trying to address).
> 
> The hotplugging capability is one of the features I was looking
> for, actually.
> 
> I have done some more refactoring in this piece of code in
> upcoming patches.
> 
> Will check, though, if I can do something with compute_fuses(),
> even though, the other cslices are not really fused off (read
> below).
> 
> > > +
> > >   rb_link_node(>uabi_node, prev, p);
> > >   rb_insert_color(>uabi_node, >uabi_engines);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c 
> > > b/drivers/gpu/drm/i915/gt/intel_gt.c
> > > index a425db5ed3a2..e19df4ef47f6 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > > @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> > >   }
> > >  }
> > >  
> > > +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> > > +{
> > > + if (!IS_DG2(gt->i915))
> > > + return;
> > > +
> > > + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> > 
> > This doesn't look right to me.  A value of 0 means every cslice gets
> > associated with CCS0.
> 
> Yes, that's what I'm trying to do. The behavior I'm looking for
> is this one:
> 
>/*
> ...
>   * With 1 engine (ccs0):
>   *   slice 0, 1, 2, 3: ccs0
>   *
>   * With 2 engines (ccs0, ccs1):
>   *   slice 0, 2: ccs0
>   *   slice 1, 3: ccs1
>   *
>   * With 4 engines (ccs0, ccs1, ccs2, ccs3):
>   *   slice 0: ccs0
>   *   slice 1: ccs1
>   *   slice 2: ccs2
>   *   slice 3: ccs3
> ...
> */
> 
> where the user can configure runtime the mode, making sure that
> no client is connected to i915.
> 
> But, this needs to be written 
> 
> As we are now forcing mode '1', then all cslices are connected
> with ccs0.

Right --- and that's what I'm pointing out as illegal.  I think that
code comment above was taken out of context from a different RFC series;
that's not an accurate description of the behavior we want here.

First, the above comment is using ccs# to refer to userspace engines,
not hardware engines.  As a simple example, DG2-G11 only ever has a
single CCS which userspace sees as "instance 0" but which is actually
CCS1 at the hardware level.  If you try to follow the comment above when
programming CCS_MODE, you've assigned all of the cslices to a
non-existent engine and assigned no cslices to the CCS engine that
actually exists.  For DG2-G10 (and I think DG2-G12), there are different
combinations of fused-off / not-fused-off engines that will always show
up in userspace as CCS0-CCSn, even if those don't match the hardware
IDs.

Second, the above comment is assuming that you have a part with a
maximum fusing config (i.e., all cslices present).  Using DG2-G11 again
as an example, there's also only a single cslice (cslice1), so if you
tell CCS1 that it's allowed to use EUs from non-existent cslice0,
cslice2, and cslice3, you might not get the behavior you were hoping
for.

> 
> > On a DG2-G11 platform, that will flat out break
> > compute since CCS0 is never present (G11 only has a single CCS and it's
> > always the hardware's CCS1).  Even on a G10 or G12 this could also break
> > things depending on the fusing of your card if the hardware CCS0 happens
> > 

Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

2024-02-21 Thread Javier Carrasco
On 21.02.24 21:25, Matthias Kaehlcke wrote:
> On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote:
>> The current mechanism uses generic names for the power supplies, which
>> conflicts with proper name definitions in the device bindings.
>>
>> Add a per-device property to include real supply names and keep generic
>> names as a fallback mechanism for backward compatibility.
>>
>> Signed-off-by: Javier Carrasco 
>> ---
>>  drivers/usb/misc/onboard_usb_dev.c | 54 
>> --
>>  drivers/usb/misc/onboard_usb_dev.h | 23 
>>  2 files changed, 52 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/usb/misc/onboard_usb_dev.c 
>> b/drivers/usb/misc/onboard_usb_dev.c
>> index f43130a6786f..e66fcac93006 100644
>> --- a/drivers/usb/misc/onboard_usb_dev.c
>> +++ b/drivers/usb/misc/onboard_usb_dev.c
>> @@ -29,18 +29,6 @@
>>  
>>  #include "onboard_usb_dev.h"
>>  
>> -/*
>> - * Use generic names, as the actual names might differ between devices. If 
>> a new
>> - * device requires more than the currently supported supplies, add a new one
>> - * here.
>> - */
>> -static const char * const supply_names[] = {
>> -"vdd",
>> -"vdd2",
>> -};
>> -
>> -#define MAX_SUPPLIES ARRAY_SIZE(supply_names)
>> -
>>  static void onboard_dev_attach_usb_driver(struct work_struct *work);
>>  
>>  static struct usb_device_driver onboard_dev_usbdev_driver;
>> @@ -66,6 +54,33 @@ struct onboard_dev {
>>  struct clk *clk;
>>  };
>>  
>> +static int onboard_dev_get_regulator_bulk(struct device *dev,
>> +  struct onboard_dev *onboard_dev)
>> +{
>> +unsigned int i;
>> +int err;
>> +
>> +const char * const *supply_names = onboard_dev->pdata->supply_names;
>> +
>> +if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES)
>> +return dev_err_probe(dev, -EINVAL, "max %zu supplies 
>> supported!\n",
>> + MAX_SUPPLIES);
>> +
>> +if (!supply_names[0])
>> +supply_names = generic_supply_names;
> 
> Please change to 'if (!supply_names)' and omit the initialization of
> .supply_names for devices that use the generic supply names.
> 

That looks much cleaner, I will apply it to the next version.

>> diff --git a/drivers/usb/misc/onboard_usb_dev.h 
>> b/drivers/usb/misc/onboard_usb_dev.h
>> index ebe83e19d818..59dced6bd339 100644
>> --- a/drivers/usb/misc/onboard_usb_dev.h
>> +++ b/drivers/usb/misc/onboard_usb_dev.h
>> @@ -6,63 +6,86 @@
>>  #ifndef _USB_MISC_ONBOARD_USB_DEV_H
>>  #define _USB_MISC_ONBOARD_USB_DEV_H
>>  
>> +/*
>> + * Fallback supply names for backwards compatibility. If the device requires
>> + * more than the currently supported supplies, add a new one here, and if
>> + * possible, the real name supplies to the device-specific data.
>> + */
>> +static const char * const generic_supply_names[] = {
>> +"vdd",
>> +"vdd2",
>> +};
>> +
>> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names)
> 
> This will have to change when support for a device with more than 2 
> non-generic
> supply names gets added. Please use a literal value for MAX_SUPPLIES instead 
> of
> ARRAY_SIZE. If the literal is 2 it would still need to change for future 
> devices
> with more supplies, but that change would be more straighforward.
> 

I am not completely sure about this. Someone could increase MAX_SUPPLIES
without adding a generic name. Actually two modifications will be
necessary for every addition (name and MAX_SUPPLIES). If ARRAY_SIZE is
used, only new names are required, and MAX_SUPPLIES is automatically
increased.

I understand that the whole point of this is getting rid of the generic
names, but we still have to provide generic names for every extra
supply, at least for code consistency and to avoid size mismatches
between real an generic supply names.

>> +
>>  struct onboard_dev_pdata {
>>  unsigned long reset_us; /* reset pulse width in us */
>>  unsigned int num_supplies;  /* number of supplies */
>>  bool is_hub;
>> +const char * const supply_names[MAX_SUPPLIES];
>> +
>>  };
>>  
>>  static const struct onboard_dev_pdata microchip_usb424_data = {
>>  .reset_us = 1,
>>  .num_supplies = 1,
>> +.supply_names = { NULL },
>>  .is_hub = true,
>>  };
>>
>> ...

Thanks again for your feedback.

Best regards,
Javier Carrasco



Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

2024-02-21 Thread Dmitry Baryshkov
On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov
 wrote:
>
> On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi  wrote:
> >
> > From: Yury Norov 
> >
> > Generalize __GENMASK() to support different types, and implement
> > fixed-types versions of GENMASK() based on it. The fixed-type version
> > allows more strict checks to the min/max values accepted, which is
> > useful for defining registers like implemented by i915 and xe drivers
> > with their REG_GENMASK*() macros.
> >
> > The strict checks rely on shift-count-overflow compiler check to
> > fail the build if a number outside of the range allowed is passed.
> > Example:
> >
> > #define FOO_MASK GENMASK_U32(33, 4)
> >
> > will generate a warning like:
> >
> > ../include/linux/bits.h:41:31: error: left shift count >= width of 
> > type [-Werror=shift-count-overflow]
> >41 |  (((t)~0ULL - ((t)(1) << (l)) + 1) & \
> >   |   ^~
> >
> > Signed-off-by: Yury Norov 
> > Signed-off-by: Lucas De Marchi 
> > Acked-by: Jani Nikula 
> > ---
> >  include/linux/bitops.h |  1 -
> >  include/linux/bits.h   | 32 ++--
> >  2 files changed, 22 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index 2ba557e067fe..1db50c69cfdb 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -15,7 +15,6 @@
> >  #  define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n)))
> >  #endif
> >
> > -#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)
> >  #define BITS_TO_LONGS(nr)  __KERNEL_DIV_ROUND_UP(nr, 
> > BITS_PER_TYPE(long))
> >  #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, 
> > BITS_PER_TYPE(u64))
> >  #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, 
> > BITS_PER_TYPE(u32))
> > diff --git a/include/linux/bits.h b/include/linux/bits.h
> > index 7c0cf5031abe..bd56f32de44e 100644
> > --- a/include/linux/bits.h
> > +++ b/include/linux/bits.h
> > @@ -6,6 +6,8 @@
> >  #include 
> >  #include 
> >
> > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)
> > +
> >  #define BIT_MASK(nr)   (UL(1) << ((nr) % BITS_PER_LONG))
> >  #define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
> >  #define BIT_ULL_MASK(nr)   (ULL(1) << ((nr) % BITS_PER_LONG_LONG))
> > @@ -30,16 +32,26 @@
> >  #define GENMASK_INPUT_CHECK(h, l) 0
> >  #endif
> >
> > -#define __GENMASK(h, l) \
> > -   (((~UL(0)) - (UL(1) << (l)) + 1) & \
> > -(~UL(0) >> (BITS_PER_LONG - 1 - (h
> > -#define GENMASK(h, l) \
> > -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
> > +/*
> > + * Generate a mask for the specified type @t. Additional checks are made to
> > + * guarantee the value returned fits in that type, relying on
> > + * shift-count-overflow compiler check to detect incompatible arguments.
> > + * For example, all these create build errors or warnings:
> > + *
> > + * - GENMASK(15, 20): wrong argument order
> > + * - GENMASK(72, 15): doesn't fit unsigned long
> > + * - GENMASK_U32(33, 15): doesn't fit in a u32
> > + */
> > +#define __GENMASK(t, h, l) \
> > +   (GENMASK_INPUT_CHECK(h, l) + \
> > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \
> > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)
> >
> > -#define __GENMASK_ULL(h, l) \
> > -   (((~ULL(0)) - (ULL(1) << (l)) + 1) & \
> > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h
> > -#define GENMASK_ULL(h, l) \
> > -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
> > +#define GENMASK(h, l)  __GENMASK(unsigned long,  h, l)
> > +#define GENMASK_ULL(h, l)  __GENMASK(unsigned long long, h, l)
> > +#define GENMASK_U8(h, l)   __GENMASK(u8,  h, l)
> > +#define GENMASK_U16(h, l)  __GENMASK(u16, h, l)
> > +#define GENMASK_U32(h, l)  __GENMASK(u32, h, l)
> > +#define GENMASK_U64(h, l)  __GENMASK(u64, h, l)
>
> This breaks drm-tip on arm64 architecture:
>

Excuse me, it seems a c from gitlab didn't work as expected.

arch/arm64/kernel/entry-fpsimd.S: Assembler messages:
arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
arch/arm64/kernel/entry-fpsimd.S:66:  Info: macro invoked from here
arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
arch/arm64/kernel/entry-fpsimd.S:66:  Info: macro invoked from here
arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
arch/arm64/kernel/entry-fpsimd.S:66:  Info: macro invoked from here
arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
arch/arm64/kernel/entry-fpsimd.S:66:  Info: macro invoked from here
arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters
following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned
long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned
long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)'
arch/arm64/kernel/entry-fpsimd.S:66:  Info: macro invoked from here
arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', 

Re: [PATCH] drm/amd/display: Add prefix to functions inside dnc10_cm_common.h

2024-02-21 Thread Alex Deucher
On Wed, Feb 21, 2024 at 2:31 PM Joao Paulo Pereira da Silva
 wrote:
>
> While debugging with ftrace, it's useful to create filters to search
> within the codebase. However, since some function names lack prefixes,
> creating a good filter may become more difficult.
>
> Because of this, add prefix to functions declared inside the header
> dcn10/dcn10_cm_common.h to facilitate creating good filters for the
> functions declared inside the file.
>

I think in this case, these functions are not dcn10 specific, they are
common to all DCN families, hence, the lack of a dcn10 prefix, they
just happen to date back to the original dcn10 code.  Might be better
to split them out into a separate file rather than renaming them.

Alex

> Signed-off-by: Joao Paulo Pereira da Silva 
> ---
>  .../drm/amd/display/dc/dcn10/dcn10_cm_common.c | 12 ++--
>  .../drm/amd/display/dc/dcn10/dcn10_cm_common.h |  8 
>  .../drm/amd/display/dc/dcn10/dcn10_dpp_cm.c| 18 +-
>  .../drm/amd/display/dc/dcn20/dcn20_dpp_cm.c|  8 
>  .../gpu/drm/amd/display/dc/dcn20/dcn20_mpc.c   |  8 
>  .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c   |  2 +-
>  .../drm/amd/display/dc/dcn30/dcn30_dpp_cm.c|  4 ++--
>  .../drm/amd/display/dc/dcn30/dcn30_dwb_cm.c|  6 +++---
>  .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c   |  8 
>  .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c|  2 +-
>  .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c|  6 +++---
>  .../amd/display/dc/hwss/dcn30/dcn30_hwseq.c|  2 +-
>  .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c|  2 +-
>  13 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c 
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> index 3538973bd0c6..3878b78faf89 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> @@ -38,7 +38,7 @@
>  #define FN(reg_name, field_name) \
> reg->shifts.field_name, reg->masks.field_name
>
> -void cm_helper_program_color_matrices(
> +void dcn10_cm_common_helper_program_color_matrices(
> struct dc_context *ctx,
> const uint16_t *regval,
> const struct color_matrices_reg *reg)
> @@ -62,7 +62,7 @@ void cm_helper_program_color_matrices(
>
>  }
>
> -void cm_helper_program_xfer_func(
> +void dcn10_cm_common_helper_program_xfer_func(
> struct dc_context *ctx,
> const struct pwl_params *params,
> const struct xfer_func_reg *reg)
> @@ -125,7 +125,7 @@ void cm_helper_program_xfer_func(
>
>
>
> -bool cm_helper_convert_to_custom_float(
> +bool dcn10_cm_common_helper_convert_to_custom_float(
> struct pwl_result_data *rgb_resulted,
> struct curve_points3 *corner_points,
> uint32_t hw_points_num,
> @@ -311,7 +311,7 @@ bool cm_helper_convert_to_custom_float(
>  #define DC_LOGGER \
> ctx->logger
>
> -bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx,
> +bool dcn10_cm_common_helper_translate_curve_to_hw_format(struct dc_context 
> *ctx,
> const struct dc_transfer_func *output_tf,
> struct pwl_params *lut_params, bool fixpoint)
>  {
> @@ -507,7 +507,7 @@ bool cm_helper_translate_curve_to_hw_format(struct 
> dc_context *ctx,
> ++rgb;
> ++i;
> }
> -   cm_helper_convert_to_custom_float(rgb_resulted,
> +   dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted,
> lut_params->corner_points,
> hw_points, fixpoint);
>
> @@ -653,7 +653,7 @@ bool cm_helper_translate_curve_to_degamma_hw_format(
> ++rgb;
> ++i;
> }
> -   cm_helper_convert_to_custom_float(rgb_resulted,
> +   dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted,
> lut_params->corner_points,
> hw_points, false);
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h 
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h
> index 0a68b63d6126..0622dbdbe84b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h
> @@ -89,23 +89,23 @@ struct color_matrices_reg{
> uint32_t csc_c33_c34;
>  };
>
> -void cm_helper_program_color_matrices(
> +void dcn10_cm_common_helper_program_color_matrices(
> struct dc_context *ctx,
> const uint16_t *regval,
> const struct color_matrices_reg *reg);
>
> -void cm_helper_program_xfer_func(
> +void dcn10_cm_common_helper_program_xfer_func(
> struct dc_context *ctx,
> const struct pwl_params 

Re: [PATCH 0/3] drm/amdgpu: Use KMEM_CACHE instead of kmem_cache_create

2024-02-21 Thread Alex Deucher
Applied.  Thanks!

On Wed, Feb 21, 2024 at 6:08 AM Christian König
 wrote:
>
> Am 21.02.24 um 10:59 schrieb Kunwu Chan:
> > For where the cache name and the structure name match.
> > Use the new KMEM_CACHE() macro instead of direct kmem_cache_create
> > to simplify the creation of SLAB caches.
>
> Reviewed-by: Christian König  for the entire
> series.
>
> >
> > Kunwu Chan (3):
> >drm/amdgpu: Simplify the allocation of fence slab caches
> >drm/amdgpu: Simplify the allocation of mux_chunk slab caches
> >drm/amdgpu: Simplify the allocation of sync slab caches
> >
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c| 4 +---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c | 4 +---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 4 +---
> >   3 files changed, 3 insertions(+), 9 deletions(-)
> >
>


Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

2024-02-21 Thread Dmitry Baryshkov
On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi  wrote:
>
> From: Yury Norov 
>
> Generalize __GENMASK() to support different types, and implement
> fixed-types versions of GENMASK() based on it. The fixed-type version
> allows more strict checks to the min/max values accepted, which is
> useful for defining registers like implemented by i915 and xe drivers
> with their REG_GENMASK*() macros.
>
> The strict checks rely on shift-count-overflow compiler check to
> fail the build if a number outside of the range allowed is passed.
> Example:
>
> #define FOO_MASK GENMASK_U32(33, 4)
>
> will generate a warning like:
>
> ../include/linux/bits.h:41:31: error: left shift count >= width of 
> type [-Werror=shift-count-overflow]
>41 |  (((t)~0ULL - ((t)(1) << (l)) + 1) & \
>   |   ^~
>
> Signed-off-by: Yury Norov 
> Signed-off-by: Lucas De Marchi 
> Acked-by: Jani Nikula 
> ---
>  include/linux/bitops.h |  1 -
>  include/linux/bits.h   | 32 ++--
>  2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index 2ba557e067fe..1db50c69cfdb 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -15,7 +15,6 @@
>  #  define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n)))
>  #endif
>
> -#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)
>  #define BITS_TO_LONGS(nr)  __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
>  #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, 
> BITS_PER_TYPE(u64))
>  #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, 
> BITS_PER_TYPE(u32))
> diff --git a/include/linux/bits.h b/include/linux/bits.h
> index 7c0cf5031abe..bd56f32de44e 100644
> --- a/include/linux/bits.h
> +++ b/include/linux/bits.h
> @@ -6,6 +6,8 @@
>  #include 
>  #include 
>
> +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)
> +
>  #define BIT_MASK(nr)   (UL(1) << ((nr) % BITS_PER_LONG))
>  #define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
>  #define BIT_ULL_MASK(nr)   (ULL(1) << ((nr) % BITS_PER_LONG_LONG))
> @@ -30,16 +32,26 @@
>  #define GENMASK_INPUT_CHECK(h, l) 0
>  #endif
>
> -#define __GENMASK(h, l) \
> -   (((~UL(0)) - (UL(1) << (l)) + 1) & \
> -(~UL(0) >> (BITS_PER_LONG - 1 - (h
> -#define GENMASK(h, l) \
> -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
> +/*
> + * Generate a mask for the specified type @t. Additional checks are made to
> + * guarantee the value returned fits in that type, relying on
> + * shift-count-overflow compiler check to detect incompatible arguments.
> + * For example, all these create build errors or warnings:
> + *
> + * - GENMASK(15, 20): wrong argument order
> + * - GENMASK(72, 15): doesn't fit unsigned long
> + * - GENMASK_U32(33, 15): doesn't fit in a u32
> + */
> +#define __GENMASK(t, h, l) \
> +   (GENMASK_INPUT_CHECK(h, l) + \
> +(((t)~0ULL - ((t)(1) << (l)) + 1) & \
> +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)
>
> -#define __GENMASK_ULL(h, l) \
> -   (((~ULL(0)) - (ULL(1) << (l)) + 1) & \
> -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h
> -#define GENMASK_ULL(h, l) \
> -   (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
> +#define GENMASK(h, l)  __GENMASK(unsigned long,  h, l)
> +#define GENMASK_ULL(h, l)  __GENMASK(unsigned long long, h, l)
> +#define GENMASK_U8(h, l)   __GENMASK(u8,  h, l)
> +#define GENMASK_U16(h, l)  __GENMASK(u16, h, l)
> +#define GENMASK_U32(h, l)  __GENMASK(u32, h, l)
> +#define GENMASK_U64(h, l)  __GENMASK(u64, h, l)

This breaks drm-tip on arm64 architecture:

arch/arm64/kernel/entry-fpsimd.S: Assembler messages:
465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')'
472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters
following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned
long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned
long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)'
474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here
475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')'
478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here
479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', 

Re: [PATCH 1/9] kunit: test: Log the correct filter string in executor_test

2024-02-21 Thread Daniel Latypov
On Wed, Feb 21, 2024 at 1:28 AM David Gow  wrote:
>
> KUnit's executor_test logs the filter string in KUNIT_ASSERT_EQ_MSG(),
> but passed a random character from the filter, rather than the whole
> string.

Note: it's worse than that, afaict.

It's printing from a random bit of memory.
I was curious about this, so I found under UML, the string I got was
always "efault)" if I make it fail for j=0.

>
> This was found by annotating KUNIT_ASSERT_EQ_MSG() to let gcc validate
> the format string.
>
> Fixes: 76066f93f1df ("kunit: add tests for filtering attributes")
> Signed-off-by: David Gow 

Reviewed-by: Daniel Latypov 

> ---
>  lib/kunit/executor_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> index 22d4ee86dbed..3f7f967e3688 100644
> --- a/lib/kunit/executor_test.c
> +++ b/lib/kunit/executor_test.c
> @@ -129,7 +129,7 @@ static void parse_filter_attr_test(struct kunit *test)
> GFP_KERNEL);
> for (j = 0; j < filter_count; j++) {
> parsed_filters[j] = kunit_next_attr_filter(, );
> -   KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter 
> '%s'", filters[j]);
> +   KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter 
> from '%s'", filters);

note: if there is a v2, it might be nice to include `j` in the message.


Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

2024-02-21 Thread Matthias Kaehlcke
On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote:
> The current mechanism uses generic names for the power supplies, which
> conflicts with proper name definitions in the device bindings.
> 
> Add a per-device property to include real supply names and keep generic
> names as a fallback mechanism for backward compatibility.
> 
> Signed-off-by: Javier Carrasco 
> ---
>  drivers/usb/misc/onboard_usb_dev.c | 54 
> --
>  drivers/usb/misc/onboard_usb_dev.h | 23 
>  2 files changed, 52 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/usb/misc/onboard_usb_dev.c 
> b/drivers/usb/misc/onboard_usb_dev.c
> index f43130a6786f..e66fcac93006 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
> @@ -29,18 +29,6 @@
>  
>  #include "onboard_usb_dev.h"
>  
> -/*
> - * Use generic names, as the actual names might differ between devices. If a 
> new
> - * device requires more than the currently supported supplies, add a new one
> - * here.
> - */
> -static const char * const supply_names[] = {
> - "vdd",
> - "vdd2",
> -};
> -
> -#define MAX_SUPPLIES ARRAY_SIZE(supply_names)
> -
>  static void onboard_dev_attach_usb_driver(struct work_struct *work);
>  
>  static struct usb_device_driver onboard_dev_usbdev_driver;
> @@ -66,6 +54,33 @@ struct onboard_dev {
>   struct clk *clk;
>  };
>  
> +static int onboard_dev_get_regulator_bulk(struct device *dev,
> +   struct onboard_dev *onboard_dev)
> +{
> + unsigned int i;
> + int err;
> +
> + const char * const *supply_names = onboard_dev->pdata->supply_names;
> +
> + if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES)
> + return dev_err_probe(dev, -EINVAL, "max %zu supplies 
> supported!\n",
> +  MAX_SUPPLIES);
> +
> + if (!supply_names[0])
> + supply_names = generic_supply_names;

Please change to 'if (!supply_names)' and omit the initialization of
.supply_names for devices that use the generic supply names.

> diff --git a/drivers/usb/misc/onboard_usb_dev.h 
> b/drivers/usb/misc/onboard_usb_dev.h
> index ebe83e19d818..59dced6bd339 100644
> --- a/drivers/usb/misc/onboard_usb_dev.h
> +++ b/drivers/usb/misc/onboard_usb_dev.h
> @@ -6,63 +6,86 @@
>  #ifndef _USB_MISC_ONBOARD_USB_DEV_H
>  #define _USB_MISC_ONBOARD_USB_DEV_H
>  
> +/*
> + * Fallback supply names for backwards compatibility. If the device requires
> + * more than the currently supported supplies, add a new one here, and if
> + * possible, the real name supplies to the device-specific data.
> + */
> +static const char * const generic_supply_names[] = {
> + "vdd",
> + "vdd2",
> +};
> +
> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names)

This will have to change when support for a device with more than 2 non-generic
supply names gets added. Please use a literal value for MAX_SUPPLIES instead of
ARRAY_SIZE. If the literal is 2 it would still need to change for future devices
with more supplies, but that change would be more straighforward.

> +
>  struct onboard_dev_pdata {
>   unsigned long reset_us; /* reset pulse width in us */
>   unsigned int num_supplies;  /* number of supplies */
>   bool is_hub;
> + const char * const supply_names[MAX_SUPPLIES];
> +
>  };
>  
>  static const struct onboard_dev_pdata microchip_usb424_data = {
>   .reset_us = 1,
>   .num_supplies = 1,
> + .supply_names = { NULL },
>   .is_hub = true,
>  };
>
> ...


Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote:
> The correct format specifier for p - n (both p and n are pointers) is
> %td, as the type should be ptrdiff_t.

I think %tu is better. d specifies a signed type. I don't doubt that the
warning is fixed but I think %tu represents the type semantics here.

>
> This was discovered by annotating KUnit assertion macros with gcc's
> printf specifier, but note that gcc incorrectly suggested a %d or %ld
> specifier (depending on the pointer size of the architecture being
> built).
>
> Fixes: 0ea09083116d ("lib/cmdline: Allow get_options() to take 0 to validate 
> the input")
> Signed-off-by: David Gow 


> ---
>  lib/cmdline_kunit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
> index d4572dbc9145..705b82736be0 100644
> --- a/lib/cmdline_kunit.c
> +++ b/lib/cmdline_kunit.c
> @@ -124,7 +124,7 @@ static void cmdline_do_one_range_test(struct kunit *test, 
> const char *in,
>   n, e[0], r[0]);
>
>   p = memchr_inv([1], 0, sizeof(r) - sizeof(r[0]));
> - KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %u out of bound", 
> n, p - r);
> + KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %td out of 
> bound", n, p - r);
>  }
>
>  static void cmdline_test_range(struct kunit *test)
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: [PATCH 1/9] kunit: test: Log the correct filter string in executor_test

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:14PM +0800, David Gow wrote:
> KUnit's executor_test logs the filter string in KUNIT_ASSERT_EQ_MSG(),
> but passed a random character from the filter, rather than the whole
> string.
>
> This was found by annotating KUNIT_ASSERT_EQ_MSG() to let gcc validate
> the format string.
>
> Fixes: 76066f93f1df ("kunit: add tests for filtering attributes")
> Signed-off-by: David Gow 

Reviewed-by: Justin Stitt 
> ---
>  lib/kunit/executor_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> index 22d4ee86dbed..3f7f967e3688 100644
> --- a/lib/kunit/executor_test.c
> +++ b/lib/kunit/executor_test.c
> @@ -129,7 +129,7 @@ static void parse_filter_attr_test(struct kunit *test)
>   GFP_KERNEL);
>   for (j = 0; j < filter_count; j++) {
>   parsed_filters[j] = kunit_next_attr_filter(, );
> - KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter 
> '%s'", filters[j]);
> + KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter from 
> '%s'", filters);
>   }
>
>   KUNIT_EXPECT_STREQ(test, kunit_attr_filter_name(parsed_filters[0]), 
> "speed");
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: [PATCH 9/9] kunit: Annotate _MSG assertion variants with gnu printf specifiers

2024-02-21 Thread Justin Stitt
Hi,

On Wed, Feb 21, 2024 at 05:27:22PM +0800, David Gow wrote:
> KUnit's assertion macros have variants which accept a printf format
> string, to allow tests to specify a more detailed message on failure.
> These (and the related KUNIT_FAIL() macro) ultimately wrap the
> __kunit_do_failed_assertion() function, which accepted a printf format
> specifier, but did not have the __printf attribute, so gcc couldn't warn
> on incorrect agruments.
>
> It turns out there were quite a few tests with such incorrect arguments.
>
> Add the __printf() specifier now that we've fixed these errors, to
> prevent them from recurring.
>
> Suggested-by: Linus Torvalds 
> Signed-off-by: David Gow 

Reviewed-by: Justin Stitt 
> ---
>  include/kunit/test.h | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index fcb4a4940ace..61637ef32302 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -579,12 +579,12 @@ void __printf(2, 3) kunit_log_append(struct 
> string_stream *log, const char *fmt,
>
>  void __noreturn __kunit_abort(struct kunit *test);
>
> -void __kunit_do_failed_assertion(struct kunit *test,
> -const struct kunit_loc *loc,
> -enum kunit_assert_type type,
> -const struct kunit_assert *assert,
> -assert_format_t assert_format,
> -const char *fmt, ...);
> +void __printf(6, 7) __kunit_do_failed_assertion(struct kunit *test,
> + const struct kunit_loc *loc,
> + enum kunit_assert_type type,
> + const struct kunit_assert 
> *assert,
> + assert_format_t assert_format,
> + const char *fmt, ...);
>
>  #define _KUNIT_FAILED(test, assert_type, assert_class, assert_format, 
> INITIALIZER, fmt, ...) do { \
>   static const struct kunit_loc __loc = KUNIT_CURRENT_LOC;   \
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Thanks
Justin


Re: Re: [PATCH] drm/xe: skip building debugfs code for CONFIG_DEBUG_FS=n

2024-02-21 Thread Lucas De Marchi

On Wed, Feb 21, 2024 at 05:21:17PM +1100, Stephen Rothwell wrote:

Hi Lucas,

On Tue, 20 Feb 2024 23:29:54 -0600 Lucas De Marchi  
wrote:


Looking at 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2d5c7b7eb345249cb34d42cbc2b97b4c57ea944e
it seems we still don't have drm-xe-next branch in linux-next.

Stephen, could you please add it? Or do I have to do something on my end
before that?  This is the branch:

https://gitlab.freedesktop.org/drm/xe/kernel drm-xe-next


Added from tomorrow.  Currently the only contact is yourself.  Do you
want anyone else (or a mailing list) as well?

Do you want the drm-xe-fixes branch included as well?


I don't think it's needed since drm-xe-next covers what is in
drm-xe-fixes. Please add other maintainers and mailing list:

M:  Oded Gabbay 
M:  Thomas Hellström 
L:  intel...@lists.freedesktop.org

Looking at drm-intel and drm-misc, they are using a special
for-linux-next. We may eventually adopt the same workflow, but I will
have to check with other maintainers.

thanks
Lucas De Marchi



Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window.

You will need to ensure that the patches/commits in your tree/series have
been:
* submitted under GPL v2 (or later) and include the Contributor's
   Signed-off-by,
* posted to the relevant mailing list,
* reviewed by you (or another maintainer of your subsystem tree),
* successfully unit tested, and
* destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

--
Cheers,
Stephen Rothwell
s...@canb.auug.org.au





[PATCH] drm/amd/display: Add prefix to functions inside dnc10_cm_common.h

2024-02-21 Thread Joao Paulo Pereira da Silva
While debugging with ftrace, it's useful to create filters to search
within the codebase. However, since some function names lack prefixes,
creating a good filter may become more difficult.

Because of this, add prefix to functions declared inside the header
dcn10/dcn10_cm_common.h to facilitate creating good filters for the
functions declared inside the file.

Signed-off-by: Joao Paulo Pereira da Silva 
---
 .../drm/amd/display/dc/dcn10/dcn10_cm_common.c | 12 ++--
 .../drm/amd/display/dc/dcn10/dcn10_cm_common.h |  8 
 .../drm/amd/display/dc/dcn10/dcn10_dpp_cm.c| 18 +-
 .../drm/amd/display/dc/dcn20/dcn20_dpp_cm.c|  8 
 .../gpu/drm/amd/display/dc/dcn20/dcn20_mpc.c   |  8 
 .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c   |  2 +-
 .../drm/amd/display/dc/dcn30/dcn30_dpp_cm.c|  4 ++--
 .../drm/amd/display/dc/dcn30/dcn30_dwb_cm.c|  6 +++---
 .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c   |  8 
 .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c|  2 +-
 .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c|  6 +++---
 .../amd/display/dc/hwss/dcn30/dcn30_hwseq.c|  2 +-
 .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c|  2 +-
 13 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
index 3538973bd0c6..3878b78faf89 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
@@ -38,7 +38,7 @@
 #define FN(reg_name, field_name) \
reg->shifts.field_name, reg->masks.field_name
 
-void cm_helper_program_color_matrices(
+void dcn10_cm_common_helper_program_color_matrices(
struct dc_context *ctx,
const uint16_t *regval,
const struct color_matrices_reg *reg)
@@ -62,7 +62,7 @@ void cm_helper_program_color_matrices(
 
 }
 
-void cm_helper_program_xfer_func(
+void dcn10_cm_common_helper_program_xfer_func(
struct dc_context *ctx,
const struct pwl_params *params,
const struct xfer_func_reg *reg)
@@ -125,7 +125,7 @@ void cm_helper_program_xfer_func(
 
 
 
-bool cm_helper_convert_to_custom_float(
+bool dcn10_cm_common_helper_convert_to_custom_float(
struct pwl_result_data *rgb_resulted,
struct curve_points3 *corner_points,
uint32_t hw_points_num,
@@ -311,7 +311,7 @@ bool cm_helper_convert_to_custom_float(
 #define DC_LOGGER \
ctx->logger
 
-bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx,
+bool dcn10_cm_common_helper_translate_curve_to_hw_format(struct dc_context 
*ctx,
const struct dc_transfer_func *output_tf,
struct pwl_params *lut_params, bool fixpoint)
 {
@@ -507,7 +507,7 @@ bool cm_helper_translate_curve_to_hw_format(struct 
dc_context *ctx,
++rgb;
++i;
}
-   cm_helper_convert_to_custom_float(rgb_resulted,
+   dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted,
lut_params->corner_points,
hw_points, fixpoint);
 
@@ -653,7 +653,7 @@ bool cm_helper_translate_curve_to_degamma_hw_format(
++rgb;
++i;
}
-   cm_helper_convert_to_custom_float(rgb_resulted,
+   dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted,
lut_params->corner_points,
hw_points, false);
 
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h
index 0a68b63d6126..0622dbdbe84b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h
@@ -89,23 +89,23 @@ struct color_matrices_reg{
uint32_t csc_c33_c34;
 };
 
-void cm_helper_program_color_matrices(
+void dcn10_cm_common_helper_program_color_matrices(
struct dc_context *ctx,
const uint16_t *regval,
const struct color_matrices_reg *reg);
 
-void cm_helper_program_xfer_func(
+void dcn10_cm_common_helper_program_xfer_func(
struct dc_context *ctx,
const struct pwl_params *params,
const struct xfer_func_reg *reg);
 
-bool cm_helper_convert_to_custom_float(
+bool dcn10_cm_common_helper_convert_to_custom_float(
struct pwl_result_data *rgb_resulted,
struct curve_points3 *corner_points,
uint32_t hw_points_num,
bool fixpoint);
 
-bool cm_helper_translate_curve_to_hw_format(
+bool dcn10_cm_common_helper_translate_curve_to_hw_format(
struct dc_context *ctx,
const struct dc_transfer_func *output_tf,

Re: [PATCH v4 2/8] usb: misc: onboard_dev: add support for non-hub devices

2024-02-21 Thread Matthias Kaehlcke
On Tue, Feb 20, 2024 at 03:05:46PM +0100, Javier Carrasco wrote:
> Most of the functionality this driver provides can be used by non-hub
> devices as well.
> 
> To account for the hub-specific code, add a flag to the device data
> structure and check its value for hub-specific code.

Please mention that the driver doesn't power off non-hub devices
during system suspend.

> Signed-off-by: Javier Carrasco 
> ---
>  drivers/usb/misc/onboard_usb_dev.c |  3 ++-
>  drivers/usb/misc/onboard_usb_dev.h | 10 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/misc/onboard_usb_dev.c 
> b/drivers/usb/misc/onboard_usb_dev.c
> index 2103af2cb2a6..f43130a6786f 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
> @@ -129,7 +129,8 @@ static int __maybe_unused onboard_dev_suspend(struct 
> device *dev)
>   if (!device_may_wakeup(node->udev->bus->controller))
>   continue;
>  
> - if (usb_wakeup_enabled_descendants(node->udev)) {
> + if (usb_wakeup_enabled_descendants(node->udev) ||
> + !onboard_dev->pdata->is_hub) {


This check isn't dependent on characteristics of the USB devices processed
in this loop, therefore it can be performed at function entry. Please combine
it with the check of 'always_powered_in_suspend'. It's also an option to
omit the check completely, 'always_powered_in_suspend' will never be set for
non-hub devices (assuming the sysfs attribute isn't added).

>   power_off = false;
>   break;
>   }

Without code context: please omit the creation of the 
'always_powered_in_suspend'
attribute for non-hub devices. As per above we don't plan to hone it, so it
shouldn't exist.


Re: [PATCH v4 1/8] usb: misc: onboard_hub: rename to onboard_dev

2024-02-21 Thread Matthias Kaehlcke
On Tue, Feb 20, 2024 at 03:05:45PM +0100, Javier Carrasco wrote:
> This patch prepares onboad_hub to support non-hub devices by renaming
> the driver files and their content, the headers and their references.
> 
> The comments and descriptions have been slightly modified to keep
> coherence and account for the specific cases that only affect onboard
> hubs (e.g. peer-hub).
> 
> The "hub" variables in functions where "dev" (and similar names) variables
> already exist have been renamed to onboard_dev for clarity, which adds a
> few lines in cases where more than 80 characters are used.
> 
> No new functionality has been added.
> 
> Signed-off-by: Javier Carrasco 
> ---
>  ...-usb-hub => sysfs-bus-platform-onboard-usb-dev} |   4 +-
>  MAINTAINERS|   4 +-
>  drivers/usb/core/Makefile  |   4 +-
>  drivers/usb/core/hub.c |   8 +-
>  drivers/usb/core/hub.h |   2 +-
>  drivers/usb/misc/Kconfig   |  16 +-
>  drivers/usb/misc/Makefile  |   2 +-
>  drivers/usb/misc/onboard_usb_dev.c | 518 
> +
>  .../misc/{onboard_usb_hub.h => onboard_usb_dev.h}  |  28 +-
>  ...ard_usb_hub_pdevs.c => onboard_usb_dev_pdevs.c} |  47 +-
>  drivers/usb/misc/onboard_usb_hub.c | 501 
>  include/linux/usb/onboard_dev.h|  18 +
>  include/linux/usb/onboard_hub.h|  18 -
>  13 files changed, 594 insertions(+), 576 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub 
> b/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
> similarity index 67%
> rename from Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
> rename to Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
> index 42deb0552065..cd31f76362e7 100644
> --- a/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
> +++ b/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev
> @@ -4,5 +4,5 @@ KernelVersion:5.20
>  Contact: Matthias Kaehlcke 
>   linux-...@vger.kernel.org
>  Description:
> - (RW) Controls whether the USB hub remains always powered
> - during system suspend or not.
> \ No newline at end of file
> + (RW) Controls whether the USB device remains always powered
> + during system suspend or not.

With patch "[2/8] usb: misc: onboard_dev: add support for non-hub devices"
this attribute isn't honed for non-hub devices. With that I'd say leave
the existing comment unchanged, but add a note that this attribute only
exists for hubs. That will also require a change in the patch mentioned
above to omit the creation of the attribute for non-hub devices.

> diff --git a/drivers/usb/misc/onboard_usb_dev.c 
> b/drivers/usb/misc/onboard_usb_dev.c
> new file mode 100644
> index ..2103af2cb2a6
> --- /dev/null
> +++ b/drivers/usb/misc/onboard_usb_dev.c
>
> ...
>
> +/*
> + * Returns the onboard_dev platform device that is associated with the USB
> + * device passed as parameter.
> + */
> +static struct onboard_dev *_find_onboard_dev(struct device *dev)
> +{
> + struct platform_device *pdev;
> + struct device_node *np;
> + struct onboard_dev *onboard_dev;
> +
> + pdev = of_find_device_by_node(dev->of_node);
> + if (!pdev) {
> + np = of_parse_phandle(dev->of_node, "peer-hub", 0);
> + if (!np) {
> + dev_err(dev, "failed to find device node for peer 
> hub\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pdev = of_find_device_by_node(np);
> + of_node_put(np);
> +
> + if (!pdev)
> + return ERR_PTR(-ENODEV);
> + }
> +
> + onboard_dev = dev_get_drvdata(>dev);
> + put_device(>dev);
> +
> + /*
> +  * The presence of drvdata ('hub') indicates that the platform driver

drop "('hub')"

> diff --git a/drivers/usb/misc/onboard_usb_hub_pdevs.c 
> b/drivers/usb/misc/onboard_usb_dev_pdevs.c
> similarity index 69%
> rename from drivers/usb/misc/onboard_usb_hub_pdevs.c
> rename to drivers/usb/misc/onboard_usb_dev_pdevs.c
> index ed22a18f4ab7..ca56f67393f1 100644
> --- a/drivers/usb/misc/onboard_usb_hub_pdevs.c
> +++ b/drivers/usb/misc/onboard_usb_dev_pdevs.c
>
> ...
>
>  /**
> - * onboard_hub_create_pdevs -- create platform devices for onboard USB hubs
> - * @parent_hub   : parent hub to scan for connected onboard hubs
> - * @pdev_list: list of onboard hub platform devices owned by the 
> parent hub
> + * onboard_dev_create_pdevs -- create platform devices for onboard USB 
> devices
> + * @parent_hub   : parent hub to scan for connected onboard devices
> + * @pdev_list: list of onboard platform devices owned by the parent 
> hub
>   *
> - * Creates a platform device for each 

Re: [PATCH] drm/msm/a6xx: specify UBWC config for sc7180

2024-02-21 Thread Connor Abbott
On Tue, Feb 20, 2024 at 5:12 PM Dmitry Baryshkov
 wrote:
>
> Historically the Adreno driver has not been updating memory
> configuration registers on a618 (SC7180 platform) implying that the
> default configuration is fine. After the rework performed in the commit
> 8814455a0e54 ("drm/msm: Refactor UBWC config setting") the function
> a6xx_calc_ubwc_config() still contained this shortcut and did not
> calculate UBWC configuration. However the function which now actually
> updates hardware registers, a6xx_set_ubwc_config(), doesn't contain such
> check.
>
> Rather than adding the check to a6xx_set_ubwc_config(), fill in the
> UBWC config for a618 (based on readings from SC7180).
>
> Reported-by: Leonard Lausen 
> Link: https://gitlab.freedesktop.org/drm/msm/-/issues/49
> Fixes: 8814455a0e54 ("drm/msm: Refactor UBWC config setting")
> Cc: Connor Abbott 
> Signed-off-by: Dmitry Baryshkov 

Thanks!

Reviewed-by: Connor Abbott 

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index c9c55e2ea584..dc80e5940f51 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1292,9 +1292,8 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu 
> *gpu)
> gpu->ubwc_config.ubwc_mode = 1;
> }
>
> -   /* a618 is using the hw default values */
> if (adreno_is_a618(gpu))
> -   return;
> +   gpu->ubwc_config.highest_bank_bit = 14;
>
> if (adreno_is_a619_holi(gpu))
> gpu->ubwc_config.highest_bank_bit = 13;
>
> ---
> base-commit: 41c177cf354126a22443b5c80cec9fdd313e67e1
> change-id: 20240220-fd-sc7180-explicit-ubwc-40953fa55947
>
> Best regards,
> --
> Dmitry Baryshkov 
>


[PATCH RESEND v2] drm/syncobj: handle NULL fence in syncobj_eventfd_entry_func

2024-02-21 Thread Erik Kurzinger
During syncobj_eventfd_entry_func, dma_fence_chain_find_seqno may set
the fence to NULL if the given seqno is signaled and a later seqno has
already been submitted. In that case, the eventfd should be signaled
immediately which currently does not happen.

This is a similar issue to the one addressed by b19926d4f3a6
("drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence")

As a fix, if the return value of dma_fence_chain_find_seqno indicates
success but it sets the fence to NULL, we will assign a stub fence to
ensure the following code still signals the eventfd.

v1 -> v2: assign a stub fence instead of signaling the eventfd

Signed-off-by: Erik Kurzinger 
Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
---
 drivers/gpu/drm/drm_syncobj.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 6c82138b2c70..cb5b5838ccf3 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1441,10 +1441,20 @@ syncobj_eventfd_entry_func(struct drm_syncobj *syncobj,
 
/* This happens inside the syncobj lock */
fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1));
+   if (!fence)
+   return;
+
ret = dma_fence_chain_find_seqno(, entry->point);
-   if (ret != 0 || !fence) {
+   if (ret != 0) {
+   /* The given seqno has not been submitted yet. */
dma_fence_put(fence);
return;
+   } else if (!fence) {
+   /* If dma_fence_chain_find_seqno returns 0 but sets the fence
+* to NULL, it implies that the given seqno is signaled and a
+* later seqno has already been submitted. Assign a stub fence
+* so that the eventfd still gets signaled below. */
+   fence = dma_fence_get_stub();
}
 
list_del_init(>node);
-- 
2.43.2



Re: [PATCH v7 3/3] drm/buddy: Add defragmentation support

2024-02-21 Thread Matthew Auld

On 21/02/2024 12:18, Arunpravin Paneer Selvam wrote:

Add a function to support defragmentation.

v1:
   - Defragment the memory beginning from min_order
 till the required memory space is available.

v2(Matthew):
   - add amdgpu user for defragmentation
   - add a warning if the two blocks are incompatible on
 defragmentation
   - call full defragmentation in the fini() function
   - place a condition to test if min_order is equal to 0
   - replace the list with safe_reverse() variant as we might
 remove the block from the list.

Signed-off-by: Arunpravin Paneer Selvam 
Suggested-by: Matthew Auld 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 17 +++-
  drivers/gpu/drm/drm_buddy.c  | 93 +---
  include/drm/drm_buddy.h  |  3 +
  3 files changed, 97 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index e494f5bf136a..cff8a526c622 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -533,8 +533,21 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager 
*man,
   min_block_size,
   >blocks,
   vres->flags);
-   if (unlikely(r))
-   goto error_free_blocks;
+   if (unlikely(r)) {
+   if (r == -ENOSPC) {
+   drm_buddy_defrag(mm, min_block_size);
+   r = drm_buddy_alloc_blocks(mm, fpfn,
+  lpfn,
+  size,
+  min_block_size,
+  >blocks,
+  vres->flags);
+   if (unlikely(r))
+   goto error_free_blocks;
+   } else {
+   goto error_free_blocks;
+   }
+   }
  
  		if (size > remaining_size)

remaining_size = 0;
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 18e004fa39d3..56bd1560fbcd 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -203,6 +203,8 @@ void drm_buddy_fini(struct drm_buddy *mm)
drm_block_free(mm, mm->roots[i]);
}
  
+	drm_buddy_defrag(mm, mm->chunk_size << mm->max_order);


I think this needs to be called higher up, otherwise we blow up with the 
WARN, plus we just freed the root(s). There is also the case with 
non-power-of-two VRAM size, in which case you get multiple roots and 
max_order is just the largest root and not entire address space. I guess 
do this in the loop above and use the root order instead?


Also this should be done as part of the first patch and then in this 
patch it is just a case of exporting it. Every commit should ideally be 
functional by itself.



+
WARN_ON(mm->avail != mm->size);
  
  	kfree(mm->roots);

@@ -276,25 +278,39 @@ drm_get_buddy(struct drm_buddy_block *block)
  }
  EXPORT_SYMBOL(drm_get_buddy);
  
-static void __drm_buddy_free(struct drm_buddy *mm,

-struct drm_buddy_block *block)
+static unsigned int __drm_buddy_free(struct drm_buddy *mm,
+struct drm_buddy_block *block,
+bool defrag)
  {
+   unsigned int order, block_order;
struct drm_buddy_block *parent;
  
+	block_order = drm_buddy_block_order(block);

+
while ((parent = block->parent)) {
-   struct drm_buddy_block *buddy;
+   struct drm_buddy_block *buddy = NULL;
  
  		buddy = __get_buddy(block);
  
  		if (!drm_buddy_block_is_free(buddy))

break;
  
-		if (drm_buddy_block_is_clear(block) !=

-   drm_buddy_block_is_clear(buddy))
-   break;
+   if (!defrag) {
+   /*
+* Check the block and its buddy clear state and exit
+* the loop if they both have the dissimilar state.
+*/
+   if (drm_buddy_block_is_clear(block) !=
+   drm_buddy_block_is_clear(buddy))
+   break;
  
-		if (drm_buddy_block_is_clear(block))

-   mark_cleared(parent);
+   if (drm_buddy_block_is_clear(block))
+   mark_cleared(parent);
+   }
+
+   WARN_ON(defrag &&
+   (drm_buddy_block_is_clear(block) ==
+drm_buddy_block_is_clear(buddy)));
  
  		

Re: [PATCH v2 1/3] drm/syncobj: call drm_syncobj_fence_add_wait when WAIT_AVAILABLE flag is set

2024-02-21 Thread Erik Kurzinger
It looks these these patches have still not been merged after one month, is 
there anything more that needs to be done for this to happen?

On 1/25/24 10:12, Daniel Vetter wrote:
> On Fri, Jan 19, 2024 at 08:32:06AM -0800, Erik Kurzinger wrote:
>> When waiting for a syncobj timeline point whose fence has not yet been
>> submitted with the WAIT_FOR_SUBMIT flag, a callback is registered using
>> drm_syncobj_fence_add_wait and the thread is put to sleep until the
>> timeout expires. If the fence is submitted before then,
>> drm_syncobj_add_point will wake up the sleeping thread immediately which
>> will proceed to wait for the fence to be signaled.
>>
>> However, if the WAIT_AVAILABLE flag is used instead,
>> drm_syncobj_fence_add_wait won't get called, meaning the waiting thread
>> will always sleep for the full timeout duration, even if the fence gets
>> submitted earlier. If it turns out that the fence *has* been submitted
>> by the time it eventually wakes up, it will still indicate to userspace
>> that the wait completed successfully (it won't return -ETIME), but it
>> will have taken much longer than it should have.
>>
>> To fix this, we must call drm_syncobj_fence_add_wait if *either* the
>> WAIT_FOR_SUBMIT flag or the WAIT_AVAILABLE flag is set. The only
>> difference being that with WAIT_FOR_SUBMIT we will also wait for the
>> fence to be signaled after it has been submitted while with
>> WAIT_AVAILABLE we will return immediately.
>>
>> IGT test patch: 
>> https://lists.freedesktop.org/archives/igt-dev/2024-January/067537.html
>>
>> v1 -> v2: adjust lockdep_assert_none_held_once condition
>>
>> Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8")
>> Signed-off-by: Erik Kurzinger 
> 
> Yeah I think this series catches now all the corner cases I spotted in v1.
> On the series:
> 
> Reviewed-by: Daniel Vetter 
>> ---
>>  drivers/gpu/drm/drm_syncobj.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index 94ebc71e5be5..97be8b140599 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -1058,7 +1058,8 @@ static signed long 
>> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>>  uint64_t *points;
>>  uint32_t signaled_count, i;
>>  
>> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)
>> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE))
>>  lockdep_assert_none_held_once();
>>  
>>  points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
>> @@ -1127,7 +1128,8 @@ static signed long 
>> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>>   * fallthough and try a 0 timeout wait!
>>   */
>>  
>> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
>> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) {
>>  for (i = 0; i < count; ++i)
>>  drm_syncobj_fence_add_wait(syncobjs[i], [i]);
>>  }
>> -- 
>> 2.43.0
>>
> 



[linux-next:master] BUILD REGRESSION 4893c639cc3659cefaa675bf1e59f4e7571afb5c

2024-02-21 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 4893c639cc3659cefaa675bf1e59f4e7571afb5c  Add linux-next specific 
files for 20240221

Error/Warning: (recently discovered and may have been fixed)

ld.lld: error: undefined symbol: ipt_alloc_initial_table
ld.lld: error: undefined symbol: ipt_do_table
ld.lld: error: undefined symbol: ipt_register_table
ld.lld: error: undefined symbol: ipt_unregister_table_exit

Unverified Error/Warning (likely false positive, please contact us if 
interested):

fs/pidfs.c:98 pidfd_show_fdinfo() error: 'pid' dereferencing possible ERR_PTR()
mm/userfaultfd.c:740 mfill_atomic() warn: inconsistent returns 
'>map_changing_lock'.

Error/Warning ids grouped by kconfigs:

gcc_recent_errors
|-- alpha-allyesconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- alpha-randconfig-r121-20240221
|   `-- io_uring-io_uring.c:sparse:sparse:cast-to-restricted-io_req_flags_t
|-- arc-allmodconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- arc-allyesconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- arm-allmodconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- arm-allyesconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- arm64-defconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- arm64-randconfig-002-20240221
|   `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size
|-- arm64-randconfig-003-20240221
|   `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size
|-- csky-allmodconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- csky-allyesconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- csky-randconfig-002-20240221
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- i386-allmodconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- i386-allyesconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- i386-buildonly-randconfig-001-20240221
|   `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size
|-- i386-randconfig-001-20240221
|   `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size
|-- i386-randconfig-011-20240221
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- i386-randconfig-013-20240221
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- i386-randconfig-051-20240221
|   `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size
|-- i386-randconfig-062-20240221
|   |-- io_uring-io_uring.c:sparse:sparse:cast-to-restricted-io_req_flags_t
|   |-- 
sound-core-sound_kunit.c:sparse:sparse:incorrect-type-in-argument-(different-base-types)-expected-restricted-snd_pcm_format_t-usertype-format-got-int
|   `-- 
sound-core-sound_kunit.c:sparse:sparse:restricted-snd_pcm_format_t-degrades-to-integer
|-- i386-randconfig-141-20240221
|   |-- 
drivers-bluetooth-btintel.c-btintel_read_version()-warn:passing-zero-to-PTR_ERR
|   `-- 
mm-page_owner.c-stack_print()-warn:unsigned-nr_entries-is-never-less-than-zero.
|-- loongarch-allmodconfig
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- loongarch-randconfig-r064-20240221
|   `-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|-- loongarch-randconfig-r111-20240221
|   |-- 
drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg
|   |-- 
drivers-leds-flash-leds-ktd2692.c:sparse:sparse:symbol-ktd2692_timing-was-not-declared.-Should-it-be-static
|   |-- 
include-trace-..-..-drivers-bus-mhi-host-trace.h:sparse:sparse:cast-to-restricted-__le32
|   |-- 
include-trace-..-..-drivers-bus-mhi-host-trace.h:sparse:sparse:cast-to-restricted-__le64
|   |-- 
include-trace-..-..-drivers-bus-mhi-host-trace.h:sparse:sparse:restricted-__l

Re: [PATCH v1] drivers/i915/intel_bios: Fix parsing backlight BDB data

2024-02-21 Thread Ville Syrjälä
On Tue, Feb 20, 2024 at 02:12:57PM -0700, Karthikeyan Ramasubramanian wrote:
> Starting BDB version 239, hdr_dpcd_refresh_timeout is introduced to
> backlight BDB data. Commit 700034566d68 ("drm/i915/bios: Define more BDB
> contents") updated the backlight BDB data accordingly. This broke the
> parsing of backlight BDB data in VBT for versions 236 - 238 (both
> inclusive) and hence the backlight controls are not responding on units
> with the concerned BDB version.
> 
> backlight_control information has been present in backlight BDB data
> from at least BDB version 191 onwards, if not before. Hence this patch
> extracts the backlight_control information if the block size of
> backlight BDB is >= version 191 backlight BDB block size.
> Tested on Chromebooks using Jasperlake SoC (reports bdb->version = 236).
> Tested on Chromebooks using Raptorlake SoC (reports bdb->version = 251).
> 
> Fixes: 700034566d68 ("drm/i915/bios: Define more BDB contents")
> Cc: sta...@vger.kernel.org
> Cc: Jani Nikula 
> Cc: Ville Syrjälä 
> Signed-off-by: Karthikeyan Ramasubramanian 
> ---
> 
>  drivers/gpu/drm/i915/display/intel_bios.c | 22 +--
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h |  2 --
>  2 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index aa169b0055e97..4ec50903b9e64 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1041,23 +1041,13 @@ parse_lfp_backlight(struct drm_i915_private *i915,
>  
>   panel->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI;
>   panel->vbt.backlight.controller = 0;
> - if (i915->display.vbt.version >= 191) {
> - size_t exp_size;
> + if (i915->display.vbt.version >= 191 &&
> + get_blocksize(backlight_data) >= EXP_BDB_LFP_BL_DATA_SIZE_REV_191) {

The size checks looks like nonsense to me. I guess maybe
we needed it before we were guaranteed to have the full
struct's worth of memory. But there should be no need for
this anymore.

> + const struct lfp_backlight_control_method *method;
>  
> - if (i915->display.vbt.version >= 236)
> - exp_size = sizeof(struct bdb_lfp_backlight_data);
> - else if (i915->display.vbt.version >= 234)
> - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_234;
> - else
> - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_191;
> -
> - if (get_blocksize(backlight_data) >= exp_size) {
> - const struct lfp_backlight_control_method *method;
> -
> - method = _data->backlight_control[panel_type];
> - panel->vbt.backlight.type = method->type;
> - panel->vbt.backlight.controller = method->controller;
> - }
> + method = _data->backlight_control[panel_type];
> + panel->vbt.backlight.type = method->type;
> + panel->vbt.backlight.controller = method->controller;
>   }
>  
>   panel->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h 
> b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index a9f44abfc9fc2..aeea5635a37ff 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -899,8 +899,6 @@ struct lfp_brightness_level {
>  
>  #define EXP_BDB_LFP_BL_DATA_SIZE_REV_191 \
>   offsetof(struct bdb_lfp_backlight_data, brightness_level)
> -#define EXP_BDB_LFP_BL_DATA_SIZE_REV_234 \
> - offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits)
>  
>  struct bdb_lfp_backlight_data {
>   u8 entry_size;
> -- 
> 2.44.0.rc0.258.g7320e95886-goog

-- 
Ville Syrjälä
Intel


Re: [PATCH] drm/amd/display: clean unnecessary braces

2024-02-21 Thread Rodrigo Siqueira Jordao

Hi Túlio,

First of all thanks for your patch. See my comments inline.

On 2/17/24 13:20, Túlio Fernandes wrote:

Clean unnecessary braces in dc/dcn32/dcn32_resource_helpers.c and 
dc/dcn32/dcn201_link_encoder.c


Did you identify this issue with checkpatch? If so, I recommend you 
paste the error message in the commit message. Iirc, checkpatch provides 
the file and the function name, which can make this commit message more 
informative. Also, wrap the commit message under 70 characters.




Signed-off-by: Túlio Fernandes 
---
  .../display/dc/dcn32/dcn32_resource_helpers.c| 16 +++-
  1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c 
b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c
index 87760600e154..e179dea148e7 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c
@@ -110,14 +110,12 @@ uint32_t dcn32_helper_calculate_num_ways_for_subvp(
struct dc_state *context)
  {
if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) {
-   if (dc->debug.force_subvp_num_ways) {
+   if (dc->debug.force_subvp_num_ways)
return dc->debug.force_subvp_num_ways;
-   } else {
+   else
return dcn32_helper_mall_bytes_to_ways(dc, 
context->bw_ctx.bw.dcn.mall_subvp_size_bytes);
-   }
-   } else {
+   } else


Actually, we want to keep the braces around the else part to keep the 
braces balanced with the if condition.


Thanks
Siqueira


return 0;
-   }
  }
  
  void dcn32_merge_pipes_for_subvp(struct dc *dc,

@@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe)
  {
bool psr_capable = false;
  
-	if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) {

+   if (pipe->stream && pipe->stream->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED)
psr_capable = true;
-   }
+
return psr_capable;
  }
  
@@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc *dc, struct dc_state *context, uint

if (pipe_ctx->stream && pipe_ctx->plane_state && 
dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) {
if (dcn32_allow_subvp_high_refresh_rate(dc, context, 
pipe_ctx)) {
  
-if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) {

+   if (pipe_ctx->stream->timing.v_addressable == 1080 && 
pipe_ctx->stream->timing.h_addressable == 1920)
fhd_count++;
-   }
+
subvp_high_refresh_count++;
}
}




Re: [PATCH v6 1/3] drm/buddy: Implement tracking clear page feature

2024-02-21 Thread Matthew Auld

On 21/02/2024 12:40, Paneer Selvam, Arunpravin wrote:


On 2/16/2024 5:33 PM, Matthew Auld wrote:

On 08/02/2024 15:49, Arunpravin Paneer Selvam wrote:

- Add tracking clear page feature.

- Driver should enable the DRM_BUDDY_CLEARED flag if it
   successfully clears the blocks in the free path. On the otherhand,
   DRM buddy marks each block as cleared.

- Track the available cleared pages size

- If driver requests cleared memory we prefer cleared memory
   but fallback to uncleared if we can't find the cleared blocks.
   when driver requests uncleared memory we try to use uncleared but
   fallback to cleared memory if necessary.

- When a block gets freed we clear it and mark the freed block as 
cleared,

   when there are buddies which are cleared as well we can merge them.
   Otherwise, we prefer to keep the blocks as separated.

v1: (Christian)
   - Depends on the flag check DRM_BUDDY_CLEARED, enable the block as
 cleared. Else, reset the clear flag for each block in the list.

   - For merging the 2 cleared blocks compare as below,
 drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy)

v2: (Matthew)
   - Add a wrapper drm_buddy_free_list_internal for the freeing of 
blocks

 operation within drm buddy.
   - Write a macro block_incompatible() to allocate the required blocks.
   - Update the xe driver for the drm_buddy_free_list change in 
arguments.


Signed-off-by: Arunpravin Paneer Selvam 


Signed-off-by: Matthew Auld 
Suggested-by: Christian König 


Probably needs a new unit test.

I think we are missing something to forcefully re-merge everything at 
fini()? In theory we can just call the defrag routine. Otherwise we 
might trigger various warnings since the root(s) might still be split.


Also one nit below. Otherwise I think looks good.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c  |   6 +-
  drivers/gpu/drm/drm_buddy.c   | 192 ++
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |   6 +-
  drivers/gpu/drm/tests/drm_buddy_test.c    |  10 +-
  drivers/gpu/drm/xe/xe_ttm_vram_mgr.c  |   4 +-
  include/drm/drm_buddy.h   |  18 +-
  6 files changed, 187 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c

index 8db880244324..c0c851409241 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct 
ttm_resource_manager *man,

  return 0;
    error_free_blocks:
-    drm_buddy_free_list(mm, >blocks);
+    drm_buddy_free_list(mm, >blocks, 0);
  mutex_unlock(>lock);
  error_fini:
  ttm_resource_fini(man, >base);
@@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct 
ttm_resource_manager *man,

    amdgpu_vram_mgr_do_reserve(man);
  -    drm_buddy_free_list(mm, >blocks);
+    drm_buddy_free_list(mm, >blocks, 0);
  mutex_unlock(>lock);
    atomic64_sub(vis_usage, >vis_usage);
@@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device 
*adev)

  kfree(rsv);
    list_for_each_entry_safe(rsv, temp, >reserved_pages, 
blocks) {

-    drm_buddy_free_list(>mm, >allocated);
+    drm_buddy_free_list(>mm, >allocated, 0);
  kfree(rsv);
  }
  if (!adev->gmc.is_app_apu)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index f57e6d74fb0e..33ad0cfbd54c 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -57,6 +57,16 @@ static void list_insert_sorted(struct drm_buddy *mm,
  __list_add(>link, node->link.prev, >link);
  }
  +static void clear_reset(struct drm_buddy_block *block)
+{
+    block->header &= ~DRM_BUDDY_HEADER_CLEAR;
+}
+
+static void mark_cleared(struct drm_buddy_block *block)
+{
+    block->header |= DRM_BUDDY_HEADER_CLEAR;
+}
+
  static void mark_allocated(struct drm_buddy_block *block)
  {
  block->header &= ~DRM_BUDDY_HEADER_STATE;
@@ -223,6 +233,12 @@ static int split_block(struct drm_buddy *mm,
  mark_free(mm, block->left);
  mark_free(mm, block->right);
  +    if (drm_buddy_block_is_clear(block)) {
+    mark_cleared(block->left);
+    mark_cleared(block->right);
+    clear_reset(block);
+    }
+
  mark_split(block);
    return 0;
@@ -273,6 +289,13 @@ static void __drm_buddy_free(struct drm_buddy *mm,
  if (!drm_buddy_block_is_free(buddy))
  break;
  +    if (drm_buddy_block_is_clear(block) !=
+    drm_buddy_block_is_clear(buddy))
+    break;
+
+    if (drm_buddy_block_is_clear(block))
+    mark_cleared(parent);
+
  list_del(>link);
    drm_block_free(mm, block);
@@ -295,26 +318,61 @@ void drm_buddy_free_block(struct drm_buddy *mm,
  {
  BUG_ON(!drm_buddy_block_is_allocated(block));
  mm->avail += drm_buddy_block_size(mm, block);
+    if (drm_buddy_block_is_clear(block))
+    mm->clear_avail += 

[PATCH] drm/mediatek: Add MT8188 Overlay Driver Data

2024-02-21 Thread Justin Green
Add MT8188 overlay driver configuration data. This change consequently
enables 10-bit overlay support on MT8188 devices.

Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and
BA30 overlays are confirmed to work from modetest.

Signed-off-by: Justin Green 
Tested-by: Justin Green 
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 2bffe4245466..696aabe124c2 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -635,6 +635,17 @@ static const struct mtk_disp_ovl_data 
mt8195_ovl_driver_data = {
.supports_clrfmt_ext = true,
 };
 
+static const struct mtk_disp_ovl_data mt8188_ovl_driver_data = {
+   .addr = DISP_REG_OVL_ADDR_MT8173,
+   .gmc_bits = 10,
+   .layer_nr = 4,
+   .fmt_rgb565_is_0 = true,
+   .smi_id_en = true,
+   .formats = mt8195_formats,
+   .num_formats = ARRAY_SIZE(mt8195_formats),
+   .supports_clrfmt_ext = true,
+};
+
 static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
{ .compatible = "mediatek,mt2701-disp-ovl",
  .data = _ovl_driver_data},
@@ -650,6 +661,8 @@ static const struct of_device_id 
mtk_disp_ovl_driver_dt_match[] = {
  .data = _ovl_2l_driver_data},
{ .compatible = "mediatek,mt8195-disp-ovl",
  .data = _ovl_driver_data},
+   { .compatible = "mediatek,mt8188-disp-ovl",
+ .data = _ovl_driver_data},
{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
-- 
2.44.0.rc0.258.g7320e95886-goog



Re: [PATCH] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs

2024-02-21 Thread Steven Price
On 21/02/2024 16:12, Adrián Larumbe wrote:
> Debugfs isn't always available in production builds that try to squeeze
> every single byte out of the kernel image, but we still need a way to
> toggle the timestamp and cycle counter registers so that jobs can be
> profiled for fdinfo's drm engine and cycle calculations.
> 
> Drop the debugfs knob and replace it with a sysfs file that accomplishes
> the same functionality, and document its ABI in a separate file.
> 
> Signed-off-by: Adrián Larumbe 
> ---
>  .../testing/sysfs-driver-panfrost-profiling   | 10 +++
>  Documentation/gpu/panfrost.rst|  9 +++
>  drivers/gpu/drm/panfrost/Makefile |  5 +-
>  drivers/gpu/drm/panfrost/panfrost_debugfs.c   | 21 --
>  drivers/gpu/drm/panfrost/panfrost_debugfs.h   | 14 
>  drivers/gpu/drm/panfrost/panfrost_device.h|  5 +-
>  drivers/gpu/drm/panfrost/panfrost_drv.c   | 14 ++--
>  drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
>  drivers/gpu/drm/panfrost/panfrost_sysfs.c | 74 +++
>  drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 
>  10 files changed, 124 insertions(+), 45 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling
>  delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c
>  delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling 
> b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling
> new file mode 100644
> index ..ce54069714f3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling
> @@ -0,0 +1,10 @@
> +What:
> /sys/bus/.../drivers/panfrost/.../drm/../profiling/status
> +Date:February 2024
> +KernelVersion:   6.8.0
> +Contact: Adrian Larumbe 
> +Description:
> +Get/set drm fdinfo's engine and cycles profiling status.
> +Valid values are:
> + 0: Disable fdinfo job profiling sources. This disables both the 
> GPU's
> +timestamp and cycle counter registers.
> + 1: Enable the above.

Minor point, but if we're going to eventually come up with a generic way
of doing this, then we're going to have to think about backwards
compatibility for this sysfs file. I would expect in this new world '0'
would mean "default behaviour; off unless the new-fangled thing enables
profiling" and '1' means "force on".

In which case perhaps wording like the below would be clearer:

0: Don't enable fdinfo job profiling sources.
1: Enable fdinfo job profiling sources, this enables both the GPU's
   timestamp and cycle counter registers.

Or am I being too picky over the wording ;)

One other small issue below...

> diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst
> index b80e41f4b2c5..be4ac282ef63 100644
> --- a/Documentation/gpu/panfrost.rst
> +++ b/Documentation/gpu/panfrost.rst
> @@ -38,3 +38,12 @@ the currently possible format options:
>  
>  Possible `drm-engine-` key names are: `fragment`, and  `vertex-tiler`.
>  `drm-curfreq-` values convey the current operating frequency for that engine.
> +
> +Users must bear in mind that engine and cycle sampling are disabled by 
> default,
> +because of power saving concerns. `fdinfo` users and benchmark applications 
> which
> +query the fdinfo file must make sure to toggle the job profiling status of 
> the
> +driver by writing into the appropriate sysfs node::
> +
> +echo  > 
> /sys/bus/platform/drivers/panfrost/[a-f0-9]*.gpu/drm/card1/profiling
> +
> +Where `N` is either `0` or `1`, depending on the desired enablement status.
> diff --git a/drivers/gpu/drm/panfrost/Makefile 
> b/drivers/gpu/drm/panfrost/Makefile
> index 2c01c1e7523e..6e718595d8a6 100644
> --- a/drivers/gpu/drm/panfrost/Makefile
> +++ b/drivers/gpu/drm/panfrost/Makefile
> @@ -10,8 +10,7 @@ panfrost-y := \
>   panfrost_job.o \
>   panfrost_mmu.o \
>   panfrost_perfcnt.o \
> - panfrost_dump.o
> -
> -panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o
> + panfrost_dump.o \
> + panfrost_sysfs.o
>  
>  obj-$(CONFIG_DRM_PANFROST) += panfrost.o
> diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.c 
> b/drivers/gpu/drm/panfrost/panfrost_debugfs.c
> deleted file mode 100644
> index 72d4286a6bf7..
> --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.c
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/* Copyright 2023 Collabora ltd. */
> -/* Copyright 2023 Amazon.com, Inc. or its affiliates. */
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include "panfrost_device.h"
> -#include "panfrost_gpu.h"
> -#include "panfrost_debugfs.h"
> -
> -void panfrost_debugfs_init(struct drm_minor *minor)
> -{
> - struct drm_device *dev = 

Re: [PATCH v2 01/10] backlight: Match backlight device against struct fb_info.bl_dev

2024-02-21 Thread Andy Shevchenko
On Wed, Feb 21, 2024 at 5:45 PM Thomas Zimmermann  wrote:
> Am 21.02.24 um 15:34 schrieb Andy Shevchenko:
> > On Wed, Feb 21, 2024 at 10:41:28AM +0100, Thomas Zimmermann wrote:
> >> Framebuffer drivers for devices with dedicated backlight are supposed
> >> to set struct fb_info.bl_dev to the backlight's respective device. Use
> >> the value to match backlight and framebuffer in the backlight core code.

...

> >>  if (!bd->ops)
> >>  goto out;
> >> -if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
> >> +else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info))
> > What's the point of adding redundant 'else'?
> >
> >>  goto out;
> >> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> >> +else if (info->bl_dev && info->bl_dev != bd)
> > Ditto.
>
> They group these tests into one single block of code; signaling that
> these tests serve the same purpose.

Commit message has nothing about this.
Also if needed, it should be a separate change.

> >> +goto out;
> >> +#endif

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 8/9] media: dt-bindings: Add Intel Displayport RX IP

2024-02-21 Thread Paweł Anikiel
On Thu, Feb 15, 2024 at 6:26 PM Conor Dooley  wrote:
>
> Yo,
>
> On Mon, Feb 12, 2024 at 01:13:22PM +, Paweł Anikiel wrote:
> > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > capture and Multi-Stream Transport. The user guide can be found here:
> >
> > https://www.intel.com/programmable/technical-pdfs/683273.pdf
> >
> > Signed-off-by: Paweł Anikiel 
> > ---
> >  .../devicetree/bindings/media/intel,dprx.yaml | 125 ++
> >  1 file changed, 125 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml 
> > b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > new file mode 100644
> > index ..3ed37e0a4a94
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > @@ -0,0 +1,125 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Intel DisplayPort RX IP
> > +
> > +maintainers:
> > +  - Paweł Anikiel 
> > +
> > +properties:
> > +  compatible:
> > +const: intel,dprx
>
> Please version this compatible, given that is it for an FPGA IP.
> I could not find an example of another intel IP that had versioning, but
> there's plenty of xilinx stuff you can get inspiration from.

The IP has had a few different revisions, so I decided to just use the
"IP version" which is 20.0.1 for the version this driver is targeting.

>
> > +  reg:
> > +items:
> > +  - description: core registers
> > +  - description: irq registers
> > +
> > +  interrupts:
> > +maxItems: 1
> > +
> > +  intel,has-mst:
>
> Mostly this looks fine, but this property drew my eye.
> Firstly, I'd probably call this "intel,multi-stream-support" rather than
> "intel,has-mst".

Sure,

>
> > +type: boolean
> > +description: The device supports Multi-Stream Transport
>
> Secondly, there are many many configuration parameters for this IP,
> but you have chosen to document just one.
> Are all other configuration parameters currently in their default
> states or ignored by the driver? If not, please at least document all
> configuration settings that you rely on - for example the max stream
> count or audio packet encoding.

I looked at all the parameters listed in the user guide, and most of
them don't affect the driver. I listed the ones which are required,
and added support for the remaining ones in v2.

>
> > +
> > +  port:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: SST main link
> > +
> > +  ports:
> > +$ref: /schemas/graph.yaml#/properties/ports
> > +
> > +properties:
> > +  port@0:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: MST virtual channel 0 or SST main link
> > +
> > +  port@1:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: MST virtual channel 1
> > +
> > +  port@2:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: MST virtual channel 2
> > +
> > +  port@3:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: MST virtual channel 3
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +allOf:
> > +  - if:
> > +  required:
> > +- intel,has-mst
> > +then:
> > +  required:
> > +- ports
> > +else:
> > +  required:
> > +- port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +dprx@c0062000 {
>
> "dprx" isn't a class of device, please try to use a generic node name
> here.

I couldn't find anything in the DT spec or any other similar examples,
so I chose dp-receiver.


Re: [PATCH v17 0/5] Add RZ/{G2L, G2LC} and RZ/V2L Display Unit support

2024-02-21 Thread Maxime Ripard
On Sun, 18 Feb 2024 16:48:35 +, Biju Das wrote:
> This path series aims to add support for RZ/G2L DU DRM driver.
> 
> RZ/G2L LCD controller composed of Frame compression Processor(FCPVD), Video
> signal processor (VSPD) and Display unit(DU). The output of LCDC is
> connected to Display parallel interface and MIPI link video interface.
> 
> The output from DSI is connected to ADV7535.
> 
> [...]

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime


Re: [PATCH v15 0/5] Add RZ/{G2L,G2LC} and RZ/V2L Display Unit support

2024-02-21 Thread Maxime Ripard
On Tue, 28 Nov 2023 10:51:24 +, Biju Das wrote:
> This path series aims to add support for RZ/G2L DU DRM driver.
> 
> RZ/G2L LCD controller composed of Frame compression Processor(FCPVD), Video
> signal processor (VSPD) and Display unit(DU). The output of LCDC is
> connected to Display parallel interface and MIPI link video interface.
> 
> The output from DSI is connected to ADV7535.
> 
> [...]

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime



Re: [PATCH 7/9] media: dt-bindings: Add Chameleon v3 framebuffer

2024-02-21 Thread Paweł Anikiel
On Thu, Feb 15, 2024 at 6:29 PM Conor Dooley  wrote:
>
> On Mon, Feb 12, 2024 at 01:13:21PM +, Paweł Anikiel wrote:
> > The Chameleon v3 uses the framebuffer IP core to take the video signal
> > from different sources and directly write frames into memory.
> >
> > Signed-off-by: Paweł Anikiel 
> > ---
> >  .../bindings/media/google,chv3-fb.yaml| 77 +++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/media/google,chv3-fb.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/google,chv3-fb.yaml 
> > b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml
> > new file mode 100644
> > index ..ba6643cc7232
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/google,chv3-fb.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Google Chameleon v3 video framebuffer
> > +
> > +maintainers:
> > +  - Paweł Anikiel 
> > +
> > +properties:
> > +  compatible:
> > +const: google,chv3-fb
> > +
> > +  reg:
> > +items:
> > +  - description: core registers
> > +  - description: irq registers
> > +
> > +  interrupts:
> > +maxItems: 1
> > +
> > +  google,legacy-format:
> > +type: boolean
> > +description: The incoming video stream is in 32-bit padded mode.
> > +
> > +  google,no-endpoint:
> > +type: boolean
> > +description:
> > +  The framebuffer isn't connected to a controllable endpoint.
> > +  The video interface still works, but EDID control is unavailable
> > +  and DV timing information only reports the active video width/height.
>
> Why does this need a dedicated property? Is it not sufficient to check
> that there are no endpoints in the devicetree?

Yes, I think it is sufficient. I removed the property and added a
check in the driver in v2.


[PATCH 3/3] arch: Rename fbdev header and source files

2024-02-21 Thread Thomas Zimmermann
The per-architecture fbdev code has no dependencies on fbdev and can
be used for any video-related subsystem. Rename the files to 'video'.
Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE.

Further update all includes statements, includ guards, and Makefiles.
Also update a few strings and comments to refer to video instead of
fbdev.

Signed-off-by: Thomas Zimmermann 
Cc: Vineet Gupta 
Cc: Catalin Marinas 
Cc: Will Deacon 
Cc: Huacai Chen 
Cc: WANG Xuerui 
Cc: Geert Uytterhoeven 
Cc: Thomas Bogendoerfer 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: Michael Ellerman 
Cc: Nicholas Piggin 
Cc: Yoshinori Sato 
Cc: Rich Felker 
Cc: John Paul Adrian Glaubitz 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
 arch/arc/include/asm/fb.h|  8 
 arch/arc/include/asm/video.h |  8 
 arch/arm/include/asm/fb.h|  6 --
 arch/arm/include/asm/video.h |  6 ++
 arch/arm64/include/asm/fb.h  | 10 --
 arch/arm64/include/asm/video.h   | 10 ++
 arch/loongarch/include/asm/{fb.h => video.h} |  8 
 arch/m68k/include/asm/{fb.h => video.h}  |  8 
 arch/mips/include/asm/{fb.h => video.h}  | 12 ++--
 arch/parisc/include/asm/{fb.h => video.h}|  8 
 arch/parisc/video/Makefile   |  2 +-
 arch/parisc/video/{fbdev.c => video-sti.c}   |  2 +-
 arch/powerpc/include/asm/{fb.h => video.h}   |  8 
 arch/powerpc/kernel/pci-common.c |  2 +-
 arch/sh/include/asm/fb.h |  7 ---
 arch/sh/include/asm/video.h  |  7 +++
 arch/sparc/include/asm/{fb.h => video.h} |  8 
 arch/sparc/video/Makefile|  2 +-
 arch/sparc/video/{fbdev.c => video.c}|  4 ++--
 arch/x86/include/asm/{fb.h => video.h}   |  8 
 arch/x86/video/Makefile  |  2 +-
 arch/x86/video/{fbdev.c => video.c}  |  3 ++-
 include/asm-generic/Kbuild   |  2 +-
 include/asm-generic/{fb.h => video.h}|  6 +++---
 include/linux/fb.h   |  2 +-
 25 files changed, 75 insertions(+), 74 deletions(-)
 delete mode 100644 arch/arc/include/asm/fb.h
 create mode 100644 arch/arc/include/asm/video.h
 delete mode 100644 arch/arm/include/asm/fb.h
 create mode 100644 arch/arm/include/asm/video.h
 delete mode 100644 arch/arm64/include/asm/fb.h
 create mode 100644 arch/arm64/include/asm/video.h
 rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
 rename arch/m68k/include/asm/{fb.h => video.h} (86%)
 rename arch/mips/include/asm/{fb.h => video.h} (76%)
 rename arch/parisc/include/asm/{fb.h => video.h} (68%)
 rename arch/parisc/video/{fbdev.c => video-sti.c} (96%)
 rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
 delete mode 100644 arch/sh/include/asm/fb.h
 create mode 100644 arch/sh/include/asm/video.h
 rename arch/sparc/include/asm/{fb.h => video.h} (89%)
 rename arch/sparc/video/{fbdev.c => video.c} (86%)
 rename arch/x86/include/asm/{fb.h => video.h} (77%)
 rename arch/x86/video/{fbdev.c => video.c} (97%)
 rename include/asm-generic/{fb.h => video.h} (96%)

diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h
deleted file mode 100644
index 9c2383d29cbb9..0
--- a/arch/arc/include/asm/fb.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef _ASM_FB_H_
-#define _ASM_FB_H_
-
-#include 
-
-#endif /* _ASM_FB_H_ */
diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h
new file mode 100644
index 0..8ff7263727fe7
--- /dev/null
+++ b/arch/arc/include/asm/video.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include 
+
+#endif /* _ASM_VIDEO_H_ */
diff --git a/arch/arm/include/asm/fb.h b/arch/arm/include/asm/fb.h
deleted file mode 100644
index ce20a43c30339..0
--- a/arch/arm/include/asm/fb.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef _ASM_FB_H_
-#define _ASM_FB_H_
-
-#include 
-
-#endif /* _ASM_FB_H_ */
diff --git a/arch/arm/include/asm/video.h b/arch/arm/include/asm/video.h
new file mode 100644
index 0..f570565366e67
--- /dev/null
+++ b/arch/arm/include/asm/video.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include 
+
+#endif /* _ASM_VIDEO_H_ */
diff --git a/arch/arm64/include/asm/fb.h b/arch/arm64/include/asm/fb.h
deleted file mode 100644
index 1a495d8fb2ce0..0
--- a/arch/arm64/include/asm/fb.h
+++ /dev/null
@@ -1,10 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2012 ARM Ltd.
- */
-#ifndef __ASM_FB_H_
-#define __ASM_FB_H_
-
-#include 
-
-#endif /* __ASM_FB_H_ */
diff --git a/arch/arm64/include/asm/video.h b/arch/arm64/include/asm/video.h
new file mode 100644
index 

[PATCH 1/3] arch: Select fbdev helpers with CONFIG_VIDEO

2024-02-21 Thread Thomas Zimmermann
Various Kconfig options selected the per-architecture helpers for
fbdev. But none of the contained code depends on fbdev. Standardize
on CONFIG_VIDEO, which will allow to add more general helpers for
video functionality.

CONFIG_VIDEO protects each architecture's video/ directory. This
allows for the use of more fine-grained control for each directory's
files, such as the use of CONFIG_STI_CORE on parisc.

Signed-off-by: Thomas Zimmermann 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
 arch/parisc/Makefile  | 2 +-
 arch/sparc/Makefile   | 4 ++--
 arch/sparc/video/Makefile | 2 +-
 arch/x86/Makefile | 2 +-
 arch/x86/video/Makefile   | 3 ++-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
index 316f84f1d15c8..21b8166a68839 100644
--- a/arch/parisc/Makefile
+++ b/arch/parisc/Makefile
@@ -119,7 +119,7 @@ export LIBGCC
 
 libs-y += arch/parisc/lib/ $(LIBGCC)
 
-drivers-y += arch/parisc/video/
+drivers-$(CONFIG_VIDEO) += arch/parisc/video/
 
 boot   := arch/parisc/boot
 
diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile
index 5f60359361312..757451c3ea1df 100644
--- a/arch/sparc/Makefile
+++ b/arch/sparc/Makefile
@@ -59,8 +59,8 @@ endif
 libs-y += arch/sparc/prom/
 libs-y += arch/sparc/lib/
 
-drivers-$(CONFIG_PM) += arch/sparc/power/
-drivers-$(CONFIG_FB) += arch/sparc/video/
+drivers-$(CONFIG_PM)+= arch/sparc/power/
+drivers-$(CONFIG_VIDEO) += arch/sparc/video/
 
 boot := arch/sparc/boot
 
diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile
index 6baddbd58e4db..9dd82880a027a 100644
--- a/arch/sparc/video/Makefile
+++ b/arch/sparc/video/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-obj-$(CONFIG_FB) += fbdev.o
+obj-y  += fbdev.o
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 15a5f4f2ff0aa..c0ea612c62ebe 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -265,7 +265,7 @@ drivers-$(CONFIG_PCI)+= arch/x86/pci/
 # suspend and hibernation support
 drivers-$(CONFIG_PM) += arch/x86/power/
 
-drivers-$(CONFIG_FB_CORE) += arch/x86/video/
+drivers-$(CONFIG_VIDEO) += arch/x86/video/
 
 
 # boot loader support. Several targets are kept for legacy purposes
diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile
index 5ebe48752ffc4..9dd82880a027a 100644
--- a/arch/x86/video/Makefile
+++ b/arch/x86/video/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_FB_CORE)  += fbdev.o
+
+obj-y  += fbdev.o
-- 
2.43.0



[PATCH 2/3] arch: Remove struct fb_info from video helpers

2024-02-21 Thread Thomas Zimmermann
The per-architecture video helpers do not depend on struct fb_info
or anything else from fbdev. Remove it from the interface and replace
fb_is_primary_device() with video_is_primary_device(). The new helper
is similar in functionality, but can operate on non-fbdev devices.

Signed-off-by: Thomas Zimmermann 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
 arch/parisc/include/asm/fb.h |  8 +---
 arch/parisc/video/fbdev.c|  9 +
 arch/sparc/include/asm/fb.h  |  7 ---
 arch/sparc/video/fbdev.c | 17 -
 arch/x86/include/asm/fb.h|  8 +---
 arch/x86/video/fbdev.c   | 18 +++---
 drivers/video/fbdev/core/fbcon.c |  2 +-
 include/asm-generic/fb.h | 11 ++-
 8 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h
index 658a8a7dc5312..ed2a195a3e762 100644
--- a/arch/parisc/include/asm/fb.h
+++ b/arch/parisc/include/asm/fb.h
@@ -2,11 +2,13 @@
 #ifndef _ASM_FB_H_
 #define _ASM_FB_H_
 
-struct fb_info;
+#include 
+
+struct device;
 
 #if defined(CONFIG_STI_CORE)
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
 #endif
 
 #include 
diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index e4f8ac99fc9e0..540fa0c919d59 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -5,12 +5,13 @@
  * Copyright (C) 2001-2002 Thomas Bogendoerfer 
  */
 
-#include 
 #include 
 
 #include 
 
-int fb_is_primary_device(struct fb_info *info)
+#include 
+
+bool video_is_primary_device(struct device *dev)
 {
struct sti_struct *sti;
 
@@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info)
return true;
 
/* return true if it's the default built-in framebuffer driver */
-   return (sti->dev == info->device);
+   return (sti->dev == dev);
 }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 24440c0fda490..07f0325d6921c 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -3,10 +3,11 @@
 #define _SPARC_FB_H_
 
 #include 
+#include 
 
 #include 
 
-struct fb_info;
+struct device;
 
 #ifdef CONFIG_SPARC32
 static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
@@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
 #define pgprot_framebuffer pgprot_framebuffer
 #endif
 
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
 
 static inline void fb_memcpy_fromio(void *to, const volatile void __iomem 
*from, size_t n)
 {
diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c
index bff66dd1909a4..e46f0499c2774 100644
--- a/arch/sparc/video/fbdev.c
+++ b/arch/sparc/video/fbdev.c
@@ -1,26 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include 
-#include 
+#include 
 #include 
 
+#include 
 #include 
 
-int fb_is_primary_device(struct fb_info *info)
+bool video_is_primary_device(struct device *dev)
 {
-   struct device *dev = info->device;
-   struct device_node *node;
+   struct device_node *node = dev->of_node;
 
if (console_set_on_cmdline)
-   return 0;
+   return false;
 
-   node = dev->of_node;
if (node && node == of_console_device)
-   return 1;
+   return true;
 
-   return 0;
+   return false;
 }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
 
 MODULE_DESCRIPTION("Sparc fbdev helpers");
 MODULE_LICENSE("GPL");
diff --git a/arch/x86/include/asm/fb.h b/arch/x86/include/asm/fb.h
index c3b9582de7efd..999db33792869 100644
--- a/arch/x86/include/asm/fb.h
+++ b/arch/x86/include/asm/fb.h
@@ -2,17 +2,19 @@
 #ifndef _ASM_X86_FB_H
 #define _ASM_X86_FB_H
 
+#include 
+
 #include 
 
-struct fb_info;
+struct device;
 
 pgprot_t pgprot_framebuffer(pgprot_t prot,
unsigned long vm_start, unsigned long vm_end,
unsigned long offset);
 #define pgprot_framebuffer pgprot_framebuffer
 
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
 
 #include 
 
diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 1dd6528cc947c..4d87ce8e257fe 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -7,7 +7,6 @@
  *
  */
 
-#include 
 #include 
 

[PATCH 0/3] arch: Remove fbdev dependency from video helpers

2024-02-21 Thread Thomas Zimmermann
Make architecture helpers for display functionality depend on general
video functionality instead of fbdev. This avoid the dependency on
fbdev and makes the functionality available for non-fbdev code.

Patch 1 replaces the variety of Kconfig options that control the
Makefiles with CONFIG_VIDEO. More fine-grained control of the build
can then be done within each video/ directory; see sparc for an
example.

Patch 2 replaces fb_is_primary_device() with video_is_primary_device(),
which has no dependencies on fbdev. The implementation remains identical
on all affected platforms. There's one minor change in fbcon, which is
the only caller of fb_is_primary_device().

Patch 3 renames the source and files from fbdev to video.

Thomas Zimmermann (3):
  arch: Select fbdev helpers with CONFIG_VIDEO
  arch: Remove struct fb_info from video helpers
  arch: Rename fbdev header and source files

 arch/arc/include/asm/fb.h|  8 --
 arch/arc/include/asm/video.h |  8 ++
 arch/arm/include/asm/fb.h|  6 -
 arch/arm/include/asm/video.h |  6 +
 arch/arm64/include/asm/fb.h  | 10 
 arch/arm64/include/asm/video.h   | 10 
 arch/loongarch/include/asm/{fb.h => video.h} |  8 +++---
 arch/m68k/include/asm/{fb.h => video.h}  |  8 +++---
 arch/mips/include/asm/{fb.h => video.h}  | 12 -
 arch/parisc/Makefile |  2 +-
 arch/parisc/include/asm/fb.h | 14 ---
 arch/parisc/include/asm/video.h  | 16 
 arch/parisc/video/Makefile   |  2 +-
 arch/parisc/video/{fbdev.c => video-sti.c}   |  9 ---
 arch/powerpc/include/asm/{fb.h => video.h}   |  8 +++---
 arch/powerpc/kernel/pci-common.c |  2 +-
 arch/sh/include/asm/fb.h |  7 --
 arch/sh/include/asm/video.h  |  7 ++
 arch/sparc/Makefile  |  4 +--
 arch/sparc/include/asm/{fb.h => video.h} | 15 +--
 arch/sparc/video/Makefile|  2 +-
 arch/sparc/video/fbdev.c | 26 
 arch/sparc/video/video.c | 25 +++
 arch/x86/Makefile|  2 +-
 arch/x86/include/asm/fb.h| 19 --
 arch/x86/include/asm/video.h | 21 
 arch/x86/video/Makefile  |  3 ++-
 arch/x86/video/{fbdev.c => video.c}  | 21 +++-
 drivers/video/fbdev/core/fbcon.c |  2 +-
 include/asm-generic/Kbuild   |  2 +-
 include/asm-generic/{fb.h => video.h}| 17 +++--
 include/linux/fb.h   |  2 +-
 32 files changed, 154 insertions(+), 150 deletions(-)
 delete mode 100644 arch/arc/include/asm/fb.h
 create mode 100644 arch/arc/include/asm/video.h
 delete mode 100644 arch/arm/include/asm/fb.h
 create mode 100644 arch/arm/include/asm/video.h
 delete mode 100644 arch/arm64/include/asm/fb.h
 create mode 100644 arch/arm64/include/asm/video.h
 rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
 rename arch/m68k/include/asm/{fb.h => video.h} (86%)
 rename arch/mips/include/asm/{fb.h => video.h} (76%)
 delete mode 100644 arch/parisc/include/asm/fb.h
 create mode 100644 arch/parisc/include/asm/video.h
 rename arch/parisc/video/{fbdev.c => video-sti.c} (78%)
 rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
 delete mode 100644 arch/sh/include/asm/fb.h
 create mode 100644 arch/sh/include/asm/video.h
 rename arch/sparc/include/asm/{fb.h => video.h} (75%)
 delete mode 100644 arch/sparc/video/fbdev.c
 create mode 100644 arch/sparc/video/video.c
 delete mode 100644 arch/x86/include/asm/fb.h
 create mode 100644 arch/x86/include/asm/video.h
 rename arch/x86/video/{fbdev.c => video.c} (66%)
 rename include/asm-generic/{fb.h => video.h} (89%)

-- 
2.43.0



[PATCH] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs

2024-02-21 Thread Adrián Larumbe
Debugfs isn't always available in production builds that try to squeeze
every single byte out of the kernel image, but we still need a way to
toggle the timestamp and cycle counter registers so that jobs can be
profiled for fdinfo's drm engine and cycle calculations.

Drop the debugfs knob and replace it with a sysfs file that accomplishes
the same functionality, and document its ABI in a separate file.

Signed-off-by: Adrián Larumbe 
---
 .../testing/sysfs-driver-panfrost-profiling   | 10 +++
 Documentation/gpu/panfrost.rst|  9 +++
 drivers/gpu/drm/panfrost/Makefile |  5 +-
 drivers/gpu/drm/panfrost/panfrost_debugfs.c   | 21 --
 drivers/gpu/drm/panfrost/panfrost_debugfs.h   | 14 
 drivers/gpu/drm/panfrost/panfrost_device.h|  5 +-
 drivers/gpu/drm/panfrost/panfrost_drv.c   | 14 ++--
 drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
 drivers/gpu/drm/panfrost/panfrost_sysfs.c | 74 +++
 drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 
 10 files changed, 124 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling
 delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c
 delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h

diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling 
b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling
new file mode 100644
index ..ce54069714f3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling
@@ -0,0 +1,10 @@
+What:  /sys/bus/.../drivers/panfrost/.../drm/../profiling/status
+Date:  February 2024
+KernelVersion: 6.8.0
+Contact:   Adrian Larumbe 
+Description:
+Get/set drm fdinfo's engine and cycles profiling status.
+Valid values are:
+   0: Disable fdinfo job profiling sources. This disables both the 
GPU's
+timestamp and cycle counter registers.
+   1: Enable the above.
diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst
index b80e41f4b2c5..be4ac282ef63 100644
--- a/Documentation/gpu/panfrost.rst
+++ b/Documentation/gpu/panfrost.rst
@@ -38,3 +38,12 @@ the currently possible format options:
 
 Possible `drm-engine-` key names are: `fragment`, and  `vertex-tiler`.
 `drm-curfreq-` values convey the current operating frequency for that engine.
+
+Users must bear in mind that engine and cycle sampling are disabled by default,
+because of power saving concerns. `fdinfo` users and benchmark applications 
which
+query the fdinfo file must make sure to toggle the job profiling status of the
+driver by writing into the appropriate sysfs node::
+
+echo  > 
/sys/bus/platform/drivers/panfrost/[a-f0-9]*.gpu/drm/card1/profiling
+
+Where `N` is either `0` or `1`, depending on the desired enablement status.
diff --git a/drivers/gpu/drm/panfrost/Makefile 
b/drivers/gpu/drm/panfrost/Makefile
index 2c01c1e7523e..6e718595d8a6 100644
--- a/drivers/gpu/drm/panfrost/Makefile
+++ b/drivers/gpu/drm/panfrost/Makefile
@@ -10,8 +10,7 @@ panfrost-y := \
panfrost_job.o \
panfrost_mmu.o \
panfrost_perfcnt.o \
-   panfrost_dump.o
-
-panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o
+   panfrost_dump.o \
+   panfrost_sysfs.o
 
 obj-$(CONFIG_DRM_PANFROST) += panfrost.o
diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.c 
b/drivers/gpu/drm/panfrost/panfrost_debugfs.c
deleted file mode 100644
index 72d4286a6bf7..
--- a/drivers/gpu/drm/panfrost/panfrost_debugfs.c
+++ /dev/null
@@ -1,21 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright 2023 Collabora ltd. */
-/* Copyright 2023 Amazon.com, Inc. or its affiliates. */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "panfrost_device.h"
-#include "panfrost_gpu.h"
-#include "panfrost_debugfs.h"
-
-void panfrost_debugfs_init(struct drm_minor *minor)
-{
-   struct drm_device *dev = minor->dev;
-   struct panfrost_device *pfdev = 
platform_get_drvdata(to_platform_device(dev->dev));
-
-   debugfs_create_atomic_t("profile", 0600, minor->debugfs_root, 
>profile_mode);
-}
diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.h 
b/drivers/gpu/drm/panfrost/panfrost_debugfs.h
deleted file mode 100644
index c5af5f35877f..
--- a/drivers/gpu/drm/panfrost/panfrost_debugfs.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright 2023 Collabora ltd.
- * Copyright 2023 Amazon.com, Inc. or its affiliates.
- */
-
-#ifndef PANFROST_DEBUGFS_H
-#define PANFROST_DEBUGFS_H
-
-#ifdef CONFIG_DEBUG_FS
-void panfrost_debugfs_init(struct drm_minor *minor);
-#endif
-
-#endif  /* PANFROST_DEBUGFS_H */
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h 
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 

[PATCH v2 8/9] media: dt-bindings: Add Intel Displayport RX IP

2024-02-21 Thread Paweł Anikiel
The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
capture and Multi-Stream Transport. The user guide can be found here:

https://www.intel.com/programmable/technical-pdfs/683273.pdf

Signed-off-by: Paweł Anikiel 
---
 .../devicetree/bindings/media/intel,dprx.yaml | 160 ++
 1 file changed, 160 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml

diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml 
b/Documentation/devicetree/bindings/media/intel,dprx.yaml
new file mode 100644
index ..31025f2d5dcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
@@ -0,0 +1,160 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel DisplayPort RX IP
+
+maintainers:
+  - Paweł Anikiel 
+
+description: |
+  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
+  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
+  capture and Multi-Stream Transport.
+
+  The IP features a large number of configuration parameters, found at:
+  
https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
+
+  The following parameters have to be enabled:
+- Support DisplayPort sink
+- Enable GPU control
+  The following parameters' values have to be set in the devicetree:
+- RX maximum link rate
+- Maximum lane count
+- Support MST
+- Max stream count (only if Support MST is enabled)
+
+properties:
+  compatible:
+const: intel,dprx-20.0.1
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  intel,max-link-rate:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Max link rate configuration parameter
+
+  intel,max-lane-count:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Max lane count configuration parameter
+
+  intel,multi-stream-support:
+type: boolean
+description: Multi-Stream Transport support configuration parameter
+
+  intel,max-stream-count:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Max stream count configuration parameter
+
+  port:
+$ref: /schemas/graph.yaml#/properties/port
+description: SST main link
+
+  ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+  port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: MST virtual channel 0 or SST main link
+
+  port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description: MST virtual channel 1
+
+  port@2:
+$ref: /schemas/graph.yaml#/properties/port
+description: MST virtual channel 2
+
+  port@3:
+$ref: /schemas/graph.yaml#/properties/port
+description: MST virtual channel 3
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - if:
+  required:
+- intel,multi-stream-support
+then:
+  required:
+- intel,max-stream-count
+- ports
+else:
+  required:
+- port
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+
+dp-receiver@c0062000 {
+compatible = "intel,dprx-20.0.1";
+reg = <0xc0062000 0x800>;
+interrupt-parent = <_mst_irq>;
+interrupts = <0 IRQ_TYPE_EDGE_RISING>;
+intel,max-link-rate = <0x1e>;
+intel,max-lane-count = <4>;
+intel,multi-stream-support;
+intel,max-stream-count = <4>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+dprx_mst_0: endpoint {
+remote-endpoint = <_mst0_0>;
+};
+};
+
+port@1 {
+reg = <1>;
+dprx_mst_1: endpoint {
+remote-endpoint = <_mst1_0>;
+};
+};
+
+port@2 {
+reg = <2>;
+dprx_mst_2: endpoint {
+remote-endpoint = <_mst2_0>;
+};
+};
+
+port@3 {
+reg = <3>;
+dprx_mst_3: endpoint {
+remote-endpoint = <_mst3_0>;
+};
+};
+};
+};
+
+  - |
+dp-receiver@c0064000 {
+compatible = "intel,dprx-20.0.1";
+reg = <0xc0064000 0x800>;
+interrupt-parent = <_sst_irq>;
+interrupts = <0 IRQ_TYPE_EDGE_RISING>;
+intel,max-link-rate = <0x1e>;
+intel,max-lane-count = <4>;
+
+port {
+dprx_sst_0: endpoint {
+remote-endpoint = <_sst_0>;
+};
+};
+};
-- 
2.44.0.rc0.258.g7320e95886-goog



[PATCH v2 4/9] lib: Move DisplayPort CRC functions to common lib

2024-02-21 Thread Paweł Anikiel
The CRC functions found in drivers/gpu/drm/display/drm_dp_mst_topology.c
may be useful for other non-DRM code that deals with DisplayPort, e.g.
v4l2 drivers for DP receivers. Move these functions to /lib.

Signed-off-by: Paweł Anikiel 
---
 drivers/gpu/drm/display/Kconfig   |  1 +
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 76 ++
 include/linux/crc-dp.h| 10 +++
 lib/Kconfig   |  8 ++
 lib/Makefile  |  1 +
 lib/crc-dp.c  | 78 +++
 6 files changed, 103 insertions(+), 71 deletions(-)
 create mode 100644 include/linux/crc-dp.h
 create mode 100644 lib/crc-dp.c

diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 09712b88a5b8..c615f50152f2 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -14,6 +14,7 @@ config DRM_DISPLAY_HELPER
 config DRM_DISPLAY_DP_HELPER
bool
depends on DRM_DISPLAY_HELPER
+   select CRC_DP
help
  DRM display helpers for DisplayPort.
 
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c 
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index f7c6b60629c2..ada1f90fa808 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -22,6 +22,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -195,73 +196,6 @@ drm_dp_mst_rad_to_str(const u8 rad[8], u8 lct, char *out, 
size_t len)
 }
 
 /* sideband msg handling */
-static u8 drm_dp_msg_header_crc4(const uint8_t *data, size_t num_nibbles)
-{
-   u8 bitmask = 0x80;
-   u8 bitshift = 7;
-   u8 array_index = 0;
-   int number_of_bits = num_nibbles * 4;
-   u8 remainder = 0;
-
-   while (number_of_bits != 0) {
-   number_of_bits--;
-   remainder <<= 1;
-   remainder |= (data[array_index] & bitmask) >> bitshift;
-   bitmask >>= 1;
-   bitshift--;
-   if (bitmask == 0) {
-   bitmask = 0x80;
-   bitshift = 7;
-   array_index++;
-   }
-   if ((remainder & 0x10) == 0x10)
-   remainder ^= 0x13;
-   }
-
-   number_of_bits = 4;
-   while (number_of_bits != 0) {
-   number_of_bits--;
-   remainder <<= 1;
-   if ((remainder & 0x10) != 0)
-   remainder ^= 0x13;
-   }
-
-   return remainder;
-}
-
-static u8 drm_dp_msg_data_crc4(const uint8_t *data, u8 number_of_bytes)
-{
-   u8 bitmask = 0x80;
-   u8 bitshift = 7;
-   u8 array_index = 0;
-   int number_of_bits = number_of_bytes * 8;
-   u16 remainder = 0;
-
-   while (number_of_bits != 0) {
-   number_of_bits--;
-   remainder <<= 1;
-   remainder |= (data[array_index] & bitmask) >> bitshift;
-   bitmask >>= 1;
-   bitshift--;
-   if (bitmask == 0) {
-   bitmask = 0x80;
-   bitshift = 7;
-   array_index++;
-   }
-   if ((remainder & 0x100) == 0x100)
-   remainder ^= 0xd5;
-   }
-
-   number_of_bits = 8;
-   while (number_of_bits != 0) {
-   number_of_bits--;
-   remainder <<= 1;
-   if ((remainder & 0x100) != 0)
-   remainder ^= 0xd5;
-   }
-
-   return remainder & 0xff;
-}
 static inline u8 drm_dp_calc_sb_hdr_size(struct drm_dp_sideband_msg_hdr *hdr)
 {
u8 size = 3;
@@ -284,7 +218,7 @@ static void drm_dp_encode_sideband_msg_hdr(struct 
drm_dp_sideband_msg_hdr *hdr,
(hdr->msg_len & 0x3f);
buf[idx++] = (hdr->somt << 7) | (hdr->eomt << 6) | (hdr->seqno << 4);
 
-   crc4 = drm_dp_msg_header_crc4(buf, (idx * 2) - 1);
+   crc4 = crc_dp_msg_header(buf, (idx * 2) - 1);
buf[idx - 1] |= (crc4 & 0xf);
 
*len = idx;
@@ -305,7 +239,7 @@ static bool drm_dp_decode_sideband_msg_hdr(const struct 
drm_dp_mst_topology_mgr
len += ((buf[0] & 0xf0) >> 4) / 2;
if (len > buflen)
return false;
-   crc4 = drm_dp_msg_header_crc4(buf, (len * 2) - 1);
+   crc4 = crc_dp_msg_header(buf, (len * 2) - 1);
 
if ((crc4 & 0xf) != (buf[len - 1] & 0xf)) {
drm_dbg_kms(mgr->dev, "crc4 mismatch 0x%x 0x%x\n", crc4, 
buf[len - 1]);
@@ -725,7 +659,7 @@ static void drm_dp_crc_sideband_chunk_req(u8 *msg, u8 len)
 {
u8 crc4;
 
-   crc4 = drm_dp_msg_data_crc4(msg, len);
+   crc4 = crc_dp_msg_data(msg, len);
msg[len] = crc4;
 }
 
@@ -782,7 +716,7 @@ static bool drm_dp_sideband_append_payload(struct 
drm_dp_sideband_msg_rx *msg,
 
if (msg->curchunk_idx >= msg->curchunk_len) {
/* do CRC */
-   

[PATCH v2 9/9] ARM: dts: chameleonv3: Add video device nodes

2024-02-21 Thread Paweł Anikiel
Add device nodes for the video system present on the Chameleon v3.
It consists of six framebuffers and two Intel Displayport receivers.

Signed-off-by: Paweł Anikiel 
---
 .../socfpga/socfpga_arria10_chameleonv3.dts   | 152 ++
 1 file changed, 152 insertions(+)

diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts 
b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts
index 422d00cd4c74..2f48f30cb538 100644
--- a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts
+++ b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts
@@ -10,6 +10,158 @@ / {
compatible = "google,chameleon-v3", "enclustra,mercury-aa1",
 "altr,socfpga-arria10", "altr,socfpga";
 
+   soc {
+   fb0: video@c0060500 {
+   compatible = "google,chv3-fb";
+   reg = <0xc0060500 0x100>,
+ <0xc0060f20 0x10>;
+   interrupts = ;
+   google,legacy-format;
+   };
+
+   fb_mst0: video@c0060600 {
+   compatible = "google,chv3-fb";
+   reg = <0xc0060600 0x100>,
+ <0xc0060f30 0x10>;
+   interrupts = ;
+
+   port {
+   fb_mst0_0: endpoint {
+   remote-endpoint = <_mst_0>;
+   };
+   };
+   };
+
+   fb_mst1: video@c0060700 {
+   compatible = "google,chv3-fb";
+   reg = <0xc0060700 0x100>,
+ <0xc0060f40 0x10>;
+   interrupts = ;
+
+   port {
+   fb_mst1_0: endpoint {
+   remote-endpoint = <_mst_1>;
+   };
+   };
+   };
+
+   fb_mst2: video@c0060800 {
+   compatible = "google,chv3-fb";
+   reg = <0xc0060800 0x100>,
+ <0xc0060f50 0x10>;
+   interrupts = ;
+
+   port {
+   fb_mst2_0: endpoint {
+   remote-endpoint = <_mst_2>;
+   };
+   };
+   };
+
+   fb_mst3: video@c0060900 {
+   compatible = "google,chv3-fb";
+   reg = <0xc0060900 0x100>,
+ <0xc0060f60 0x10>;
+   interrupts = ;
+
+   port {
+   fb_mst3_0: endpoint {
+   remote-endpoint = <_mst_3>;
+   };
+   };
+   };
+
+   fb_sst: video@c0060a00 {
+   compatible = "google,chv3-fb";
+   reg = <0xc0060a00 0x100>,
+ <0xc0060f70 0x10>;
+   interrupts = ;
+
+   port {
+   fb_sst_0: endpoint {
+   remote-endpoint = <_sst_0>;
+   };
+   };
+   };
+
+   dprx_mst_irq: intc@c0060f80 {
+   compatible = "altr,pio-1.0";
+   reg = <0xc0060f80 0x10>;
+   interrupts = ;
+   altr,interrupt-type = ;
+   #interrupt-cells = <2>;
+   interrupt-controller;
+   };
+
+   dprx_sst_irq: intc@c0060fe0 {
+   compatible = "altr,pio-1.0";
+   reg = <0xc0060fe0 0x10>;
+   interrupts = ;
+   altr,interrupt-type = ;
+   #interrupt-cells = <2>;
+   interrupt-controller;
+   };
+
+   dprx_mst: dp-receiver@c0062000 {
+   compatible = "intel,dprx-20.0.1";
+   reg = <0xc0062000 0x800>;
+   interrupt-parent = <_mst_irq>;
+   interrupts = <0 IRQ_TYPE_EDGE_RISING>;
+   intel,max-link-rate = <0x1e>;
+   intel,max-lane-count = <4>;
+   intel,multi-stream-support;
+   intel,max-stream-count = <4>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   dprx_mst_0: endpoint {
+   remote-endpoint = <_mst0_0>;
+

[PATCH v2 7/9] media: dt-bindings: Add Chameleon v3 framebuffer

2024-02-21 Thread Paweł Anikiel
The Chameleon v3 uses the framebuffer IP core to take the video signal
from different sources and directly write frames into memory.

Signed-off-by: Paweł Anikiel 
---
 .../bindings/media/google,chv3-fb.yaml| 67 +++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/google,chv3-fb.yaml

diff --git a/Documentation/devicetree/bindings/media/google,chv3-fb.yaml 
b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml
new file mode 100644
index ..7961c0ab66ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/google,chv3-fb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Chameleon v3 video framebuffer
+
+maintainers:
+  - Paweł Anikiel 
+
+properties:
+  compatible:
+const: google,chv3-fb
+
+  reg:
+items:
+  - description: core registers
+  - description: irq registers
+
+  interrupts:
+maxItems: 1
+
+  google,legacy-format:
+type: boolean
+description: The incoming video stream is in 32-bit padded mode.
+
+  port:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+  Connection to the video receiver - optional. If this isn't present,
+  the video interface still works on its own, but EDID control is
+  unavailable and DV timing information only reports the active
+  video width/height.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+
+video@c0060500 {
+compatible = "google,chv3-fb";
+reg = <0xc0060500 0x100>,
+  <0xc0060f20 0x10>;
+interrupts = ;
+google,legacy-format;
+};
+
+  - |
+video@c0060600 {
+compatible = "google,chv3-fb";
+reg = <0xc0060600 0x100>,
+  <0xc0060f30 0x10>;
+interrupts = ;
+
+port {
+fb_mst0_0: endpoint {
+remote-endpoint = <_mst_0>;
+};
+};
+};
-- 
2.44.0.rc0.258.g7320e95886-goog



[PATCH v2 5/9] drm/display: Add mask definitions for DP_PAYLOAD_ALLOCATE_* registers

2024-02-21 Thread Paweł Anikiel
Each of these registers contains a single value, but not the entire
8 bits:

DP_PAYLOAD_ALLOCATE_SET - Bit 7 Reserved
DP_PAYLOAD_ALLOCATE_START_TIME_SLOT - Bits 7:6 Reserved
DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT - Bits 7:6 Reserved

Add definitions to properly mask off values read from these registers.

Signed-off-by: Paweł Anikiel 
---
 include/drm/display/drm_dp.h | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 3731828825bd..9dee30190f14 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -733,8 +733,13 @@
 # define DP_PANEL_REPLAY_SU_ENABLE  (1 << 6)
 
 #define DP_PAYLOAD_ALLOCATE_SET0x1c0
-#define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1
-#define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2
+# define DP_PAYLOAD_ALLOCATE_SET_MASK  0x7f
+
+#define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT0x1c1
+# define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT_MASK  0x3f
+
+#define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT0x1c2
+# define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT_MASK  0x3f
 
 /* Link/Sink Device Status */
 #define DP_SINK_COUNT  0x200
-- 
2.44.0.rc0.258.g7320e95886-goog



[PATCH v2 6/9] media: intel: Add Displayport RX IP driver

2024-02-21 Thread Paweł Anikiel
Add driver for the Intel DisplayPort RX FPGA IP

Signed-off-by: Paweł Anikiel 
---
 drivers/media/platform/intel/Kconfig  |   12 +
 drivers/media/platform/intel/Makefile |1 +
 drivers/media/platform/intel/intel-dprx.c | 2176 +
 3 files changed, 2189 insertions(+)
 create mode 100644 drivers/media/platform/intel/intel-dprx.c

diff --git a/drivers/media/platform/intel/Kconfig 
b/drivers/media/platform/intel/Kconfig
index 724e80a9086d..eafcd47cce68 100644
--- a/drivers/media/platform/intel/Kconfig
+++ b/drivers/media/platform/intel/Kconfig
@@ -12,3 +12,15 @@ config VIDEO_PXA27x
select V4L2_FWNODE
help
  This is a v4l2 driver for the PXA27x Quick Capture Interface
+
+config VIDEO_INTEL_DPRX
+   tristate "Intel DisplayPort RX IP driver"
+   depends on V4L_PLATFORM_DRIVERS
+   depends on VIDEO_DEV
+   select V4L2_FWNODE
+   select CRC_DP
+   help
+ v4l2 subdev driver for Intel Displayport receiver FPGA IP.
+ It is a part of the DisplayPort Intel FPGA IP Core.
+ It implements a DisplayPort 1.4 receiver capable of HBR3
+ video capture and Multi-Stream Transport.
diff --git a/drivers/media/platform/intel/Makefile 
b/drivers/media/platform/intel/Makefile
index 7e8889cbd2df..f571399f5aa8 100644
--- a/drivers/media/platform/intel/Makefile
+++ b/drivers/media/platform/intel/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_VIDEO_PXA27x) += pxa_camera.o
+obj-$(CONFIG_VIDEO_INTEL_DPRX) += intel-dprx.o
diff --git a/drivers/media/platform/intel/intel-dprx.c 
b/drivers/media/platform/intel/intel-dprx.c
new file mode 100644
index ..d0c60e29e51d
--- /dev/null
+++ b/drivers/media/platform/intel/intel-dprx.c
@@ -0,0 +1,2176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2023-2024 Google LLC.
+ * Author: Paweł Anikiel 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DPRX_MAX_EDID_BLOCKS   4
+
+/* DPRX registers */
+
+#define DPRX_RX_CONTROL0x000
+#define DPRX_RX_CONTROL_LINK_RATE_SHIFT16
+#define DPRX_RX_CONTROL_LINK_RATE_MASK 0xff
+#define DPRX_RX_CONTROL_RECONFIG_LINKRATE  13
+#define DPRX_RX_CONTROL_TP_SHIFT   8
+#define DPRX_RX_CONTROL_TP_MASK0x7
+#define DPRX_RX_CONTROL_SCRAMBLER_DISABLE  7
+#define DPRX_RX_CONTROL_CHANNEL_CODING_SHIFT   5
+#define DPRX_RX_CONTROL_CHANNEL_CODING_8B10B   0x1
+#define DPRX_RX_CONTROL_LANE_COUNT_SHIFT   0
+#define DPRX_RX_CONTROL_LANE_COUNT_MASK0x1f
+
+#define DPRX_RX_STATUS 0x001
+#define DPRX_RX_STATUS_INTERLANE_ALIGN 8
+#define DPRX_RX_STATUS_SYM_LOCK_SHIFT  4
+#define DPRX_RX_STATUS_SYM_LOCK(i) (4 + i)
+#define DPRX_RX_STATUS_CR_LOCK_SHIFT   0
+#define DPRX_RX_STATUS_CR_LOCK(i)  (0 + i)
+
+#define DPRX_MSA_HTOTAL(i) (0x022 + 0x20 * (i))
+#define DPRX_MSA_VTOTAL(i) (0x023 + 0x20 * (i))
+#define DPRX_MSA_HSP(i)(0x024 + 0x20 * (i))
+#define DPRX_MSA_HSW(i)(0x025 + 0x20 * (i))
+#define DPRX_MSA_HSTART(i) (0x026 + 0x20 * (i))
+#define DPRX_MSA_VSTART(i) (0x027 + 0x20 * (i))
+#define DPRX_MSA_VSP(i)(0x028 + 0x20 * (i))
+#define DPRX_MSA_VSW(i)(0x029 + 0x20 * (i))
+#define DPRX_MSA_HWIDTH(i) (0x02a + 0x20 * (i))
+#define DPRX_MSA_VHEIGHT(i)(0x02b + 0x20 * (i))
+#define DPRX_VBID(i)   (0x02f + 0x20 * (i))
+#define DPRX_VBID_MSA_LOCK 7
+
+#define DPRX_MST_CONTROL1  0x0a0
+#define DPRX_MST_CONTROL1_VCPTAB_UPD_FORCE 31
+#define DPRX_MST_CONTROL1_VCPTAB_UPD_REQ   30
+#define DPRX_MST_CONTROL1_VCP_ID_SHIFT(i)  (4 + 4 * (i))
+#define DPRX_MST_CONTROL1_VCP_IDS_SHIFT4
+#define DPRX_MST_CONTROL1_VCP_IDS_MASK 0x
+#define DPRX_MST_CONTROL1_MST_EN   0
+
+#define DPRX_MST_STATUS1   0x0a1
+#define DPRX_MST_STATUS1_VCPTAB_ACT_ACK30
+
+#define DPRX_MST_VCPTAB(i) (0x0a2 + i)
+
+#define DPRX_AUX_CONTROL   0x100
+#define DPRX_AUX_CONTROL_IRQ_EN8
+#define DPRX_AUX_CONTROL_TX_STROBE 7
+#define DPRX_AUX_CONTROL_LENGTH_SHIFT  0
+#define DPRX_AUX_CONTROL_LENGTH_MASK   0x1f
+
+#define DPRX_AUX_STATUS0x101
+#define DPRX_AUX_STATUS_MSG_READY  31
+#define DPRX_AUX_STATUS_READY_TO_TX30
+
+#define DPRX_AUX_COMMAND   0x102
+
+#define DPRX_AUX_HPD   0x119
+#define DPRX_AUX_HPD_IRQ   12

  1   2   3   >