[PATCH 00/10] drm/xe & drm/i915: drvdata usage changes

[PATCH 00/10] drm/xe & drm/i915: drvdata usage changes

[Jani Nikula]

Fix and cleanup drvdata usage, and switch to storing struct drm_device
instead of driver specific pointers in drvdata for the benefit of
display code.

BR,
Jani.

Jani Nikula (10):
  drm/xe: use pdev_to_xe_device() instead of pci_get_drvdata() directly
  drm/xe: add kdev_to_xe_device() helper and use it
  drm/xe/tests: fix drvdata usage
  drm/i915: use pdev_to_i915() instead of pci_get_drvdata() directly
  drm/i915 & drm/xe: save struct drm_device to drvdata
  drm/i915: support struct device and pci_dev in to_intel_display()
  drm/i915/audio: migrate away from kdev_to_i915()
  drm/i915/hdcp: migrate away from kdev_to_i915() in bind/unbind
  drm/i915/hdcp: migrate away from kdev_to_i915() in GSC messaging
  drm/xe/display: remove unused compat kdev_to_i915() and pdev_to_i915()

 drivers/gpu/drm/i915/display/intel_audio.c    | 34 ++++++----
 .../drm/i915/display/intel_display_types.h    |  6 ++
 drivers/gpu/drm/i915/display/intel_hdcp.c     | 10 +--
 .../drm/i915/display/intel_hdcp_gsc_message.c | 67 +++++++++++++------
 drivers/gpu/drm/i915/i915_driver.c            |  2 +-
 drivers/gpu/drm/i915/i915_drv.h               |  8 ++-
 drivers/gpu/drm/i915/i915_pci.c               |  4 +-
 .../gpu/drm/i915/selftests/mock_gem_device.c  |  2 +-
 .../gpu/drm/xe/compat-i915-headers/i915_drv.h |  6 --
 drivers/gpu/drm/xe/tests/xe_pci.c             |  6 +-
 drivers/gpu/drm/xe/xe_device.h                | 11 ++-
 drivers/gpu/drm/xe/xe_gsc_proxy.c             |  9 +--
 drivers/gpu/drm/xe/xe_pci.c                   |  4 +-
 13 files changed, 105 insertions(+), 64 deletions(-)

-- 
2.39.2

[PATCH 01/10] drm/xe: use pdev_to_xe_device() instead of pci_get_drvdata() directly

[Jani Nikula]

We have a helper for converting pci device to xe device, use it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index f818aa69f3ca..7bb811b4a057 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -752,7 +752,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
 {
 	struct xe_device *xe;
 
-	xe = pci_get_drvdata(pdev);
+	xe = pdev_to_xe_device(pdev);
 	if (!xe) /* driver load aborted, nothing to cleanup */
 		return;
 
-- 
2.39.2

Re: [PATCH 01/10] drm/xe: use pdev_to_xe_device() instead of pci_get_drvdata() directly

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:02-03:00)
>We have a helper for converting pci device to xe device, use it.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/xe/xe_pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index f818aa69f3ca..7bb811b4a057 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -752,7 +752,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
> {
>         struct xe_device *xe;
> 
>-        xe = pci_get_drvdata(pdev);
>+        xe = pdev_to_xe_device(pdev);
>         if (!xe) /* driver load aborted, nothing to cleanup */
>                 return;
> 
>-- 
>2.39.2
>

[PATCH 02/10] drm/xe: add kdev_to_xe_device() helper and use it

[Jani Nikula]

There are enough users for kernel device to xe device conversion, add a
helper for it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/xe/xe_device.h    | 5 +++++
 drivers/gpu/drm/xe/xe_gsc_proxy.c | 9 ++-------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
index db6cc8d0d6b8..2c96f1b2aafd 100644
--- a/drivers/gpu/drm/xe/xe_device.h
+++ b/drivers/gpu/drm/xe/xe_device.h
@@ -15,6 +15,11 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
 	return container_of(dev, struct xe_device, drm);
 }
 
+static inline struct xe_device *kdev_to_xe_device(struct device *kdev)
+{
+	return dev_get_drvdata(kdev);
+}
+
 static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
 {
 	return pci_get_drvdata(pdev);
diff --git a/drivers/gpu/drm/xe/xe_gsc_proxy.c b/drivers/gpu/drm/xe/xe_gsc_proxy.c
index aa812a2bc3ed..28e6a7a1d282 100644
--- a/drivers/gpu/drm/xe/xe_gsc_proxy.c
+++ b/drivers/gpu/drm/xe/xe_gsc_proxy.c
@@ -62,11 +62,6 @@ gsc_to_gt(struct xe_gsc *gsc)
 	return container_of(gsc, struct xe_gt, uc.gsc);
 }
 
-static inline struct xe_device *kdev_to_xe(struct device *kdev)
-{
-	return dev_get_drvdata(kdev);
-}
-
 bool xe_gsc_proxy_init_done(struct xe_gsc *gsc)
 {
 	struct xe_gt *gt = gsc_to_gt(gsc);
@@ -345,7 +340,7 @@ void xe_gsc_proxy_irq_handler(struct xe_gsc *gsc, u32 iir)
 static int xe_gsc_proxy_component_bind(struct device *xe_kdev,
 				       struct device *mei_kdev, void *data)
 {
-	struct xe_device *xe = kdev_to_xe(xe_kdev);
+	struct xe_device *xe = kdev_to_xe_device(xe_kdev);
 	struct xe_gt *gt = xe->tiles[0].media_gt;
 	struct xe_gsc *gsc = &gt->uc.gsc;
 
@@ -360,7 +355,7 @@ static int xe_gsc_proxy_component_bind(struct device *xe_kdev,
 static void xe_gsc_proxy_component_unbind(struct device *xe_kdev,
 					  struct device *mei_kdev, void *data)
 {
-	struct xe_device *xe = kdev_to_xe(xe_kdev);
+	struct xe_device *xe = kdev_to_xe_device(xe_kdev);
 	struct xe_gt *gt = xe->tiles[0].media_gt;
 	struct xe_gsc *gsc = &gt->uc.gsc;
 
-- 
2.39.2

Re: [PATCH 02/10] drm/xe: add kdev_to_xe_device() helper and use it

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:03-03:00)
>There are enough users for kernel device to xe device conversion, add a
>helper for it.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/xe/xe_device.h    | 5 +++++
> drivers/gpu/drm/xe/xe_gsc_proxy.c | 9 ++-------
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>index db6cc8d0d6b8..2c96f1b2aafd 100644
>--- a/drivers/gpu/drm/xe/xe_device.h
>+++ b/drivers/gpu/drm/xe/xe_device.h
>@@ -15,6 +15,11 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
>         return container_of(dev, struct xe_device, drm);
> }
> 
>+static inline struct xe_device *kdev_to_xe_device(struct device *kdev)

Nitpick: Although there are some places that do it differently, it seems
it is very common to use "dev" to refer to the generic struct device, so
I would s/kdev/dev/ here.

In any case:

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>+{
>+        return dev_get_drvdata(kdev);
>+}
>+
> static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
> {
>         return pci_get_drvdata(pdev);
>diff --git a/drivers/gpu/drm/xe/xe_gsc_proxy.c b/drivers/gpu/drm/xe/xe_gsc_proxy.c
>index aa812a2bc3ed..28e6a7a1d282 100644
>--- a/drivers/gpu/drm/xe/xe_gsc_proxy.c
>+++ b/drivers/gpu/drm/xe/xe_gsc_proxy.c
>@@ -62,11 +62,6 @@ gsc_to_gt(struct xe_gsc *gsc)
>         return container_of(gsc, struct xe_gt, uc.gsc);
> }
> 
>-static inline struct xe_device *kdev_to_xe(struct device *kdev)
>-{
>-        return dev_get_drvdata(kdev);
>-}
>-
> bool xe_gsc_proxy_init_done(struct xe_gsc *gsc)
> {
>         struct xe_gt *gt = gsc_to_gt(gsc);
>@@ -345,7 +340,7 @@ void xe_gsc_proxy_irq_handler(struct xe_gsc *gsc, u32 iir)
> static int xe_gsc_proxy_component_bind(struct device *xe_kdev,
>                                        struct device *mei_kdev, void *data)
> {
>-        struct xe_device *xe = kdev_to_xe(xe_kdev);
>+        struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>         struct xe_gt *gt = xe->tiles[0].media_gt;
>         struct xe_gsc *gsc = &gt->uc.gsc;
> 
>@@ -360,7 +355,7 @@ static int xe_gsc_proxy_component_bind(struct device *xe_kdev,
> static void xe_gsc_proxy_component_unbind(struct device *xe_kdev,
>                                           struct device *mei_kdev, void *data)
> {
>-        struct xe_device *xe = kdev_to_xe(xe_kdev);
>+        struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>         struct xe_gt *gt = xe->tiles[0].media_gt;
>         struct xe_gsc *gsc = &gt->uc.gsc;
> 
>-- 
>2.39.2
>

Re: [PATCH 02/10] drm/xe: add kdev_to_xe_device() helper and use it

[Jani Nikula]

On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Jani Nikula (2024-07-29 11:30:03-03:00)
>>There are enough users for kernel device to xe device conversion, add a
>>helper for it.
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> drivers/gpu/drm/xe/xe_device.h    | 5 +++++
>> drivers/gpu/drm/xe/xe_gsc_proxy.c | 9 ++-------
>> 2 files changed, 7 insertions(+), 7 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>>index db6cc8d0d6b8..2c96f1b2aafd 100644
>>--- a/drivers/gpu/drm/xe/xe_device.h
>>+++ b/drivers/gpu/drm/xe/xe_device.h
>>@@ -15,6 +15,11 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
>>         return container_of(dev, struct xe_device, drm);
>> }
>> 
>>+static inline struct xe_device *kdev_to_xe_device(struct device *kdev)
>
> Nitpick: Although there are some places that do it differently, it seems
> it is very common to use "dev" to refer to the generic struct device, so
> I would s/kdev/dev/ here.

I think kdev is often used to distinguish from struct drm_device *dev.

> In any case:
>
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

Thanks,
Jani.

>
>>+{
>>+        return dev_get_drvdata(kdev);
>>+}
>>+
>> static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
>> {
>>         return pci_get_drvdata(pdev);
>>diff --git a/drivers/gpu/drm/xe/xe_gsc_proxy.c b/drivers/gpu/drm/xe/xe_gsc_proxy.c
>>index aa812a2bc3ed..28e6a7a1d282 100644
>>--- a/drivers/gpu/drm/xe/xe_gsc_proxy.c
>>+++ b/drivers/gpu/drm/xe/xe_gsc_proxy.c
>>@@ -62,11 +62,6 @@ gsc_to_gt(struct xe_gsc *gsc)
>>         return container_of(gsc, struct xe_gt, uc.gsc);
>> }
>> 
>>-static inline struct xe_device *kdev_to_xe(struct device *kdev)
>>-{
>>-        return dev_get_drvdata(kdev);
>>-}
>>-
>> bool xe_gsc_proxy_init_done(struct xe_gsc *gsc)
>> {
>>         struct xe_gt *gt = gsc_to_gt(gsc);
>>@@ -345,7 +340,7 @@ void xe_gsc_proxy_irq_handler(struct xe_gsc *gsc, u32 iir)
>> static int xe_gsc_proxy_component_bind(struct device *xe_kdev,
>>                                        struct device *mei_kdev, void *data)
>> {
>>-        struct xe_device *xe = kdev_to_xe(xe_kdev);
>>+        struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>>         struct xe_gt *gt = xe->tiles[0].media_gt;
>>         struct xe_gsc *gsc = &gt->uc.gsc;
>> 
>>@@ -360,7 +355,7 @@ static int xe_gsc_proxy_component_bind(struct device *xe_kdev,
>> static void xe_gsc_proxy_component_unbind(struct device *xe_kdev,
>>                                           struct device *mei_kdev, void *data)
>> {
>>-        struct xe_device *xe = kdev_to_xe(xe_kdev);
>>+        struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>>         struct xe_gt *gt = xe->tiles[0].media_gt;
>>         struct xe_gsc *gsc = &gt->uc.gsc;
>> 
>>-- 
>>2.39.2
>>

-- 
Jani Nikula, Intel

[PATCH 03/10] drm/xe/tests: fix drvdata usage

[Jani Nikula]

The test code seems to assume struct drm_device * is stored in
drvdata. This is (currently) not the case. Use the proper helper to get
at the xe device.

This has not been an issue, because struct drm_device is embedded in
struct xe_device at offset 0.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
index 577ee7d14381..2046789f62bd 100644
--- a/drivers/gpu/drm/xe/tests/xe_pci.c
+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
@@ -20,15 +20,15 @@ struct kunit_test_data {
 static int dev_to_xe_device_fn(struct device *dev, void *__data)
 
 {
-	struct drm_device *drm = dev_get_drvdata(dev);
+	struct xe_device *xe = kdev_to_xe_device(dev);
 	struct kunit_test_data *data = __data;
 	int ret = 0;
 	int idx;
 
 	data->ndevs++;
 
-	if (drm_dev_enter(drm, &idx))
-		ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev)));
+	if (drm_dev_enter(&xe->drm, &idx))
+		ret = data->xe_fn(xe);
 	drm_dev_exit(idx);
 
 	return ret;
-- 
2.39.2

Re: [PATCH 03/10] drm/xe/tests: fix drvdata usage

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:04-03:00)
>The test code seems to assume struct drm_device * is stored in
>drvdata. This is (currently) not the case. Use the proper helper to get
>at the xe device.
>
>This has not been an issue, because struct drm_device is embedded in
>struct xe_device at offset 0.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>

The fix looks correct, so:

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

I noticed that xe_call_for_each_device() stopped being used as of commit
57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could
also have a patch removing it and dev_to_xe_device_fn().

--
Gustavo Sousa

>---
> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>index 577ee7d14381..2046789f62bd 100644
>--- a/drivers/gpu/drm/xe/tests/xe_pci.c
>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>@@ -20,15 +20,15 @@ struct kunit_test_data {
> static int dev_to_xe_device_fn(struct device *dev, void *__data)
> 
> {
>-        struct drm_device *drm = dev_get_drvdata(dev);
>+        struct xe_device *xe = kdev_to_xe_device(dev);
>         struct kunit_test_data *data = __data;
>         int ret = 0;
>         int idx;
> 
>         data->ndevs++;
> 
>-        if (drm_dev_enter(drm, &idx))
>-                ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev)));
>+        if (drm_dev_enter(&xe->drm, &idx))
>+                ret = data->xe_fn(xe);
>         drm_dev_exit(idx);
> 
>         return ret;
>-- 
>2.39.2
>

Re: [PATCH 03/10] drm/xe/tests: fix drvdata usage

[Jani Nikula]

On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Jani Nikula (2024-07-29 11:30:04-03:00)
>>The test code seems to assume struct drm_device * is stored in
>>drvdata. This is (currently) not the case. Use the proper helper to get
>>at the xe device.
>>
>>This has not been an issue, because struct drm_device is embedded in
>>struct xe_device at offset 0.
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> The fix looks correct, so:
>
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>
> I noticed that xe_call_for_each_device() stopped being used as of commit
> 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could
> also have a patch removing it and dev_to_xe_device_fn().

Cc: people involved with that commit.

Do you want xe_call_for_each_device() removed or retained?

BR,
Jani.


>
> --
> Gustavo Sousa
>
>>---
>> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>>index 577ee7d14381..2046789f62bd 100644
>>--- a/drivers/gpu/drm/xe/tests/xe_pci.c
>>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>>@@ -20,15 +20,15 @@ struct kunit_test_data {
>> static int dev_to_xe_device_fn(struct device *dev, void *__data)
>> 
>> {
>>-        struct drm_device *drm = dev_get_drvdata(dev);
>>+        struct xe_device *xe = kdev_to_xe_device(dev);
>>         struct kunit_test_data *data = __data;
>>         int ret = 0;
>>         int idx;
>> 
>>         data->ndevs++;
>> 
>>-        if (drm_dev_enter(drm, &idx))
>>-                ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev)));
>>+        if (drm_dev_enter(&xe->drm, &idx))
>>+                ret = data->xe_fn(xe);
>>         drm_dev_exit(idx);
>> 
>>         return ret;
>>-- 
>>2.39.2
>>

-- 
Jani Nikula, Intel

Re: [PATCH 03/10] drm/xe/tests: fix drvdata usage

[Jani Nikula]

On Tue, 06 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> Quoting Jani Nikula (2024-07-29 11:30:04-03:00)
>>>The test code seems to assume struct drm_device * is stored in
>>>drvdata. This is (currently) not the case. Use the proper helper to get
>>>at the xe device.
>>>
>>>This has not been an issue, because struct drm_device is embedded in
>>>struct xe_device at offset 0.
>>>
>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> The fix looks correct, so:
>>
>> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>
>> I noticed that xe_call_for_each_device() stopped being used as of commit
>> 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could
>> also have a patch removing it and dev_to_xe_device_fn().
>
> Cc: people involved with that commit.

And now actually Cc. *facepalm*

>
> Do you want xe_call_for_each_device() removed or retained?
>
> BR,
> Jani.
>
>
>>
>> --
>> Gustavo Sousa
>>
>>>---
>>> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>>>index 577ee7d14381..2046789f62bd 100644
>>>--- a/drivers/gpu/drm/xe/tests/xe_pci.c
>>>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>>>@@ -20,15 +20,15 @@ struct kunit_test_data {
>>> static int dev_to_xe_device_fn(struct device *dev, void *__data)
>>> 
>>> {
>>>-        struct drm_device *drm = dev_get_drvdata(dev);
>>>+        struct xe_device *xe = kdev_to_xe_device(dev);
>>>         struct kunit_test_data *data = __data;
>>>         int ret = 0;
>>>         int idx;
>>> 
>>>         data->ndevs++;
>>> 
>>>-        if (drm_dev_enter(drm, &idx))
>>>-                ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev)));
>>>+        if (drm_dev_enter(&xe->drm, &idx))
>>>+                ret = data->xe_fn(xe);
>>>         drm_dev_exit(idx);
>>> 
>>>         return ret;
>>>-- 
>>>2.39.2
>>>

-- 
Jani Nikula, Intel

Re: [PATCH 03/10] drm/xe/tests: fix drvdata usage

[Lucas De Marchi]

On Tue, Aug 06, 2024 at 03:14:02PM GMT, Jani Nikula wrote:
>On Tue, 06 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>>> Quoting Jani Nikula (2024-07-29 11:30:04-03:00)
>>>>The test code seems to assume struct drm_device * is stored in
>>>>drvdata. This is (currently) not the case. Use the proper helper to get
>>>>at the xe device.
>>>>
>>>>This has not been an issue, because struct drm_device is embedded in
>>>>struct xe_device at offset 0.
>>>>
>>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> The fix looks correct, so:
>>>
>>> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>>
>>> I noticed that xe_call_for_each_device() stopped being used as of commit
>>> 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could
>>> also have a patch removing it and dev_to_xe_device_fn().
>>
>> Cc: people involved with that commit.
>
>And now actually Cc. *facepalm*
>
>>
>> Do you want xe_call_for_each_device() removed or retained?

removed... it should had been removed as part of that series as it only
reflects the previous approach. My bad for not catching it during
review. 

struct kunit_test_data and dev_to_xe_device_fn() should be gone too. 

Lucas De Marchi

>>
>> BR,
>> Jani.
>>
>>
>>>
>>> --
>>> Gustavo Sousa
>>>
>>>>---
>>>> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>>>>index 577ee7d14381..2046789f62bd 100644
>>>>--- a/drivers/gpu/drm/xe/tests/xe_pci.c
>>>>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>>>>@@ -20,15 +20,15 @@ struct kunit_test_data {
>>>> static int dev_to_xe_device_fn(struct device *dev, void *__data)
>>>>
>>>> {
>>>>-        struct drm_device *drm = dev_get_drvdata(dev);
>>>>+        struct xe_device *xe = kdev_to_xe_device(dev);
>>>>         struct kunit_test_data *data = __data;
>>>>         int ret = 0;
>>>>         int idx;
>>>>
>>>>         data->ndevs++;
>>>>
>>>>-        if (drm_dev_enter(drm, &idx))
>>>>-                ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev)));
>>>>+        if (drm_dev_enter(&xe->drm, &idx))
>>>>+                ret = data->xe_fn(xe);
>>>>         drm_dev_exit(idx);
>>>>
>>>>         return ret;
>>>>--
>>>>2.39.2
>>>>
>
>-- 
>Jani Nikula, Intel

[PATCH 04/10] drm/i915: use pdev_to_i915() instead of pci_get_drvdata() directly

[Jani Nikula]

We have a helper for converting pci device to i915 device, use it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index ce4dfd65fafa..b2e1fd22520b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -880,7 +880,7 @@ static void i915_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_i915_private *i915;
 
-	i915 = pci_get_drvdata(pdev);
+	i915 = pdev_to_i915(pdev);
 	if (!i915) /* driver load aborted, nothing to cleanup */
 		return;
 
@@ -1025,7 +1025,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 static void i915_pci_shutdown(struct pci_dev *pdev)
 {
-	struct drm_i915_private *i915 = pci_get_drvdata(pdev);
+	struct drm_i915_private *i915 = pdev_to_i915(pdev);
 
 	i915_driver_shutdown(i915);
 }
-- 
2.39.2

Re: [PATCH 04/10] drm/i915: use pdev_to_i915() instead of pci_get_drvdata() directly

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:05-03:00)
>We have a helper for converting pci device to i915 device, use it.

Any reason why i915_inject_probe_failure(pci_get_drvdata(pdev)) was not
converted in i915_pci_probe()?

--
Gustavo Sousa

>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/i915_pci.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>index ce4dfd65fafa..b2e1fd22520b 100644
>--- a/drivers/gpu/drm/i915/i915_pci.c
>+++ b/drivers/gpu/drm/i915/i915_pci.c
>@@ -880,7 +880,7 @@ static void i915_pci_remove(struct pci_dev *pdev)
> {
>         struct drm_i915_private *i915;
> 
>-        i915 = pci_get_drvdata(pdev);
>+        i915 = pdev_to_i915(pdev);
>         if (!i915) /* driver load aborted, nothing to cleanup */
>                 return;
> 
>@@ -1025,7 +1025,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> 
> static void i915_pci_shutdown(struct pci_dev *pdev)
> {
>-        struct drm_i915_private *i915 = pci_get_drvdata(pdev);
>+        struct drm_i915_private *i915 = pdev_to_i915(pdev);
> 
>         i915_driver_shutdown(i915);
> }
>-- 
>2.39.2
>

Re: [PATCH 04/10] drm/i915: use pdev_to_i915() instead of pci_get_drvdata() directly

[Gustavo Sousa]

Quoting Gustavo Sousa (2024-08-01 14:03:33-03:00)
>Quoting Jani Nikula (2024-07-29 11:30:05-03:00)
>>We have a helper for converting pci device to i915 device, use it.
>
>Any reason why i915_inject_probe_failure(pci_get_drvdata(pdev)) was not
>converted in i915_pci_probe()?

Hm... And after the next patch, that pci_get_drvdata(pdev) call seems to
be wrong (although it would work because of drm being at offset 0).

--
Gustavo Sousa

>
>--
>Gustavo Sousa
>
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> drivers/gpu/drm/i915/i915_pci.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>>index ce4dfd65fafa..b2e1fd22520b 100644
>>--- a/drivers/gpu/drm/i915/i915_pci.c
>>+++ b/drivers/gpu/drm/i915/i915_pci.c
>>@@ -880,7 +880,7 @@ static void i915_pci_remove(struct pci_dev *pdev)
>> {
>>         struct drm_i915_private *i915;
>> 
>>-        i915 = pci_get_drvdata(pdev);
>>+        i915 = pdev_to_i915(pdev);
>>         if (!i915) /* driver load aborted, nothing to cleanup */
>>                 return;
>> 
>>@@ -1025,7 +1025,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> 
>> static void i915_pci_shutdown(struct pci_dev *pdev)
>> {
>>-        struct drm_i915_private *i915 = pci_get_drvdata(pdev);
>>+        struct drm_i915_private *i915 = pdev_to_i915(pdev);
>> 
>>         i915_driver_shutdown(i915);
>> }
>>-- 
>>2.39.2
>>

[PATCH 05/10] drm/i915 & drm/xe: save struct drm_device to drvdata

[Jani Nikula]

In the future, the display code shall not have any idea about struct
xe_device or struct drm_i915_private, but will need to get at the struct
drm_device via drvdata. Store the struct drm_device pointer to drvdata
instead of the driver specific pointer.

Even though struct drm_device is embedded in both struct xe_device and
struct drm_i915_private at offset 0, take care to check for NULL before
using container_of() to allow for other offsets.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c                | 2 +-
 drivers/gpu/drm/i915/i915_drv.h                   | 8 ++++++--
 drivers/gpu/drm/i915/selftests/mock_gem_device.c  | 2 +-
 drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 4 +++-
 drivers/gpu/drm/xe/xe_device.h                    | 8 ++++++--
 drivers/gpu/drm/xe/xe_pci.c                       | 2 +-
 6 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index fb8e9c2fcea5..176c13c2e191 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -723,7 +723,7 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (IS_ERR(i915))
 		return i915;
 
-	pci_set_drvdata(pdev, i915);
+	pci_set_drvdata(pdev, &i915->drm);
 
 	/* Device parameters start as a copy of module parameters. */
 	i915_params_copy(&i915->params, &i915_modparams);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d7723dd11c80..f6edaef73db5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -365,12 +365,16 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
 
 static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
 {
-	return dev_get_drvdata(kdev);
+	struct drm_device *drm = dev_get_drvdata(kdev);
+
+	return drm ? to_i915(drm) : NULL;
 }
 
 static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 {
-	return pci_get_drvdata(pdev);
+	struct drm_device *drm = pci_get_drvdata(pdev);
+
+	return drm ? to_i915(drm) : NULL;
 }
 
 static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 0bd29846873b..91794ca17a58 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -172,7 +172,7 @@ struct drm_i915_private *mock_gem_device(void)
 		return NULL;
 	}
 
-	pci_set_drvdata(pdev, i915);
+	pci_set_drvdata(pdev, &i915->drm);
 
 	/* Device parameters start as a copy of module parameters. */
 	i915_params_copy(&i915->params, &i915_modparams);
diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
index 2feedddf1e40..766fba88a3c8 100644
--- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
+++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
@@ -23,7 +23,9 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
 
 static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
 {
-	return dev_get_drvdata(kdev);
+	struct drm_device *drm = dev_get_drvdata(kdev);
+
+	return drm ? to_i915(drm) : NULL;
 }
 
 #define IS_PLATFORM(xe, x) ((xe)->info.platform == x)
diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
index 2c96f1b2aafd..022876eebfd5 100644
--- a/drivers/gpu/drm/xe/xe_device.h
+++ b/drivers/gpu/drm/xe/xe_device.h
@@ -17,12 +17,16 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
 
 static inline struct xe_device *kdev_to_xe_device(struct device *kdev)
 {
-	return dev_get_drvdata(kdev);
+	struct drm_device *drm = dev_get_drvdata(kdev);
+
+	return drm ? to_xe_device(drm) : NULL;
 }
 
 static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
 {
-	return pci_get_drvdata(pdev);
+	struct drm_device *drm = pci_get_drvdata(pdev);
+
+	return drm ? to_xe_device(drm) : NULL;
 }
 
 static inline struct xe_device *xe_device_const_cast(const struct xe_device *xe)
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 7bb811b4a057..f861b8cf931a 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -800,7 +800,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (IS_ERR(xe))
 		return PTR_ERR(xe);
 
-	pci_set_drvdata(pdev, xe);
+	pci_set_drvdata(pdev, &xe->drm);
 
 	xe_pm_assert_unbounded_bridge(xe);
 	subplatform_desc = find_subplatform(xe, desc);
-- 
2.39.2

Re: [PATCH 05/10] drm/i915 & drm/xe: save struct drm_device to drvdata

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:06-03:00)
>In the future, the display code shall not have any idea about struct
>xe_device or struct drm_i915_private, but will need to get at the struct
>drm_device via drvdata. Store the struct drm_device pointer to drvdata
>instead of the driver specific pointer.
>
>Even though struct drm_device is embedded in both struct xe_device and
>struct drm_i915_private at offset 0, take care to check for NULL before
>using container_of() to allow for other offsets.

I think the second half of this paragraph could be rephrased. One might
think that the text is suggesting that checking for NULL has something
to do with allowing other offsets.

I would jump directly to mention using container_of() and would assume
that it is implied that NULL check is necessary before using it. :-)

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/i915_driver.c                | 2 +-
> drivers/gpu/drm/i915/i915_drv.h                   | 8 ++++++--
> drivers/gpu/drm/i915/selftests/mock_gem_device.c  | 2 +-
> drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 4 +++-
> drivers/gpu/drm/xe/xe_device.h                    | 8 ++++++--
> drivers/gpu/drm/xe/xe_pci.c                       | 2 +-
> 6 files changed, 18 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>index fb8e9c2fcea5..176c13c2e191 100644
>--- a/drivers/gpu/drm/i915/i915_driver.c
>+++ b/drivers/gpu/drm/i915/i915_driver.c
>@@ -723,7 +723,7 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
>         if (IS_ERR(i915))
>                 return i915;
> 
>-        pci_set_drvdata(pdev, i915);
>+        pci_set_drvdata(pdev, &i915->drm);
> 
>         /* Device parameters start as a copy of module parameters. */
>         i915_params_copy(&i915->params, &i915_modparams);
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index d7723dd11c80..f6edaef73db5 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -365,12 +365,16 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> 
> static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
> {
>-        return dev_get_drvdata(kdev);
>+        struct drm_device *drm = dev_get_drvdata(kdev);
>+
>+        return drm ? to_i915(drm) : NULL;
> }
> 
> static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
> {
>-        return pci_get_drvdata(pdev);
>+        struct drm_device *drm = pci_get_drvdata(pdev);
>+
>+        return drm ? to_i915(drm) : NULL;
> }
> 
> static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
>diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>index 0bd29846873b..91794ca17a58 100644
>--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>@@ -172,7 +172,7 @@ struct drm_i915_private *mock_gem_device(void)
>                 return NULL;
>         }
> 
>-        pci_set_drvdata(pdev, i915);
>+        pci_set_drvdata(pdev, &i915->drm);
> 
>         /* Device parameters start as a copy of module parameters. */
>         i915_params_copy(&i915->params, &i915_modparams);
>diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>index 2feedddf1e40..766fba88a3c8 100644
>--- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>+++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>@@ -23,7 +23,9 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> 
> static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
> {
>-        return dev_get_drvdata(kdev);
>+        struct drm_device *drm = dev_get_drvdata(kdev);
>+
>+        return drm ? to_i915(drm) : NULL;
> }
> 
> #define IS_PLATFORM(xe, x) ((xe)->info.platform == x)
>diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>index 2c96f1b2aafd..022876eebfd5 100644
>--- a/drivers/gpu/drm/xe/xe_device.h
>+++ b/drivers/gpu/drm/xe/xe_device.h
>@@ -17,12 +17,16 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
> 
> static inline struct xe_device *kdev_to_xe_device(struct device *kdev)
> {
>-        return dev_get_drvdata(kdev);
>+        struct drm_device *drm = dev_get_drvdata(kdev);
>+
>+        return drm ? to_xe_device(drm) : NULL;
> }
> 
> static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
> {
>-        return pci_get_drvdata(pdev);
>+        struct drm_device *drm = pci_get_drvdata(pdev);
>+
>+        return drm ? to_xe_device(drm) : NULL;
> }
> 
> static inline struct xe_device *xe_device_const_cast(const struct xe_device *xe)
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index 7bb811b4a057..f861b8cf931a 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -800,7 +800,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         if (IS_ERR(xe))
>                 return PTR_ERR(xe);
> 
>-        pci_set_drvdata(pdev, xe);
>+        pci_set_drvdata(pdev, &xe->drm);
> 
>         xe_pm_assert_unbounded_bridge(xe);
>         subplatform_desc = find_subplatform(xe, desc);
>-- 
>2.39.2
>

Re: [PATCH 05/10] drm/i915 & drm/xe: save struct drm_device to drvdata

[Jani Nikula]

On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Jani Nikula (2024-07-29 11:30:06-03:00)
>>In the future, the display code shall not have any idea about struct
>>xe_device or struct drm_i915_private, but will need to get at the struct
>>drm_device via drvdata. Store the struct drm_device pointer to drvdata
>>instead of the driver specific pointer.
>>
>>Even though struct drm_device is embedded in both struct xe_device and
>>struct drm_i915_private at offset 0, take care to check for NULL before
>>using container_of() to allow for other offsets.
>
> I think the second half of this paragraph could be rephrased. One might
> think that the text is suggesting that checking for NULL has something
> to do with allowing other offsets.
>
> I would jump directly to mention using container_of() and would assume
> that it is implied that NULL check is necessary before using it. :-)

But I *am* suggesting the NULL check before container_of() has
everything to do with allowing other offsets!

container_of() will return a NULL pointer for a NULL pointer when the
offset is 0, but will return a negative garbage pointer otherwise.

BR,
Jani.


>
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> drivers/gpu/drm/i915/i915_driver.c                | 2 +-
>> drivers/gpu/drm/i915/i915_drv.h                   | 8 ++++++--
>> drivers/gpu/drm/i915/selftests/mock_gem_device.c  | 2 +-
>> drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 4 +++-
>> drivers/gpu/drm/xe/xe_device.h                    | 8 ++++++--
>> drivers/gpu/drm/xe/xe_pci.c                       | 2 +-
>> 6 files changed, 18 insertions(+), 8 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>>index fb8e9c2fcea5..176c13c2e191 100644
>>--- a/drivers/gpu/drm/i915/i915_driver.c
>>+++ b/drivers/gpu/drm/i915/i915_driver.c
>>@@ -723,7 +723,7 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
>>         if (IS_ERR(i915))
>>                 return i915;
>> 
>>-        pci_set_drvdata(pdev, i915);
>>+        pci_set_drvdata(pdev, &i915->drm);
>> 
>>         /* Device parameters start as a copy of module parameters. */
>>         i915_params_copy(&i915->params, &i915_modparams);
>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>index d7723dd11c80..f6edaef73db5 100644
>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>@@ -365,12 +365,16 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>> 
>> static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
>> {
>>-        return dev_get_drvdata(kdev);
>>+        struct drm_device *drm = dev_get_drvdata(kdev);
>>+
>>+        return drm ? to_i915(drm) : NULL;
>> }
>> 
>> static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>> {
>>-        return pci_get_drvdata(pdev);
>>+        struct drm_device *drm = pci_get_drvdata(pdev);
>>+
>>+        return drm ? to_i915(drm) : NULL;
>> }
>> 
>> static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
>>diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>>index 0bd29846873b..91794ca17a58 100644
>>--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>>+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>>@@ -172,7 +172,7 @@ struct drm_i915_private *mock_gem_device(void)
>>                 return NULL;
>>         }
>> 
>>-        pci_set_drvdata(pdev, i915);
>>+        pci_set_drvdata(pdev, &i915->drm);
>> 
>>         /* Device parameters start as a copy of module parameters. */
>>         i915_params_copy(&i915->params, &i915_modparams);
>>diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>index 2feedddf1e40..766fba88a3c8 100644
>>--- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>+++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>@@ -23,7 +23,9 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>> 
>> static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
>> {
>>-        return dev_get_drvdata(kdev);
>>+        struct drm_device *drm = dev_get_drvdata(kdev);
>>+
>>+        return drm ? to_i915(drm) : NULL;
>> }
>> 
>> #define IS_PLATFORM(xe, x) ((xe)->info.platform == x)
>>diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>>index 2c96f1b2aafd..022876eebfd5 100644
>>--- a/drivers/gpu/drm/xe/xe_device.h
>>+++ b/drivers/gpu/drm/xe/xe_device.h
>>@@ -17,12 +17,16 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
>> 
>> static inline struct xe_device *kdev_to_xe_device(struct device *kdev)
>> {
>>-        return dev_get_drvdata(kdev);
>>+        struct drm_device *drm = dev_get_drvdata(kdev);
>>+
>>+        return drm ? to_xe_device(drm) : NULL;
>> }
>> 
>> static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
>> {
>>-        return pci_get_drvdata(pdev);
>>+        struct drm_device *drm = pci_get_drvdata(pdev);
>>+
>>+        return drm ? to_xe_device(drm) : NULL;
>> }
>> 
>> static inline struct xe_device *xe_device_const_cast(const struct xe_device *xe)
>>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>>index 7bb811b4a057..f861b8cf931a 100644
>>--- a/drivers/gpu/drm/xe/xe_pci.c
>>+++ b/drivers/gpu/drm/xe/xe_pci.c
>>@@ -800,7 +800,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>         if (IS_ERR(xe))
>>                 return PTR_ERR(xe);
>> 
>>-        pci_set_drvdata(pdev, xe);
>>+        pci_set_drvdata(pdev, &xe->drm);
>> 
>>         xe_pm_assert_unbounded_bridge(xe);
>>         subplatform_desc = find_subplatform(xe, desc);
>>-- 
>>2.39.2
>>

-- 
Jani Nikula, Intel

Re: [PATCH 05/10] drm/i915 & drm/xe: save struct drm_device to drvdata

[Gustavo Sousa]

Quoting Jani Nikula (2024-08-02 05:18:39-03:00)
>On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> Quoting Jani Nikula (2024-07-29 11:30:06-03:00)
>>>In the future, the display code shall not have any idea about struct
>>>xe_device or struct drm_i915_private, but will need to get at the struct
>>>drm_device via drvdata. Store the struct drm_device pointer to drvdata
>>>instead of the driver specific pointer.
>>>
>>>Even though struct drm_device is embedded in both struct xe_device and
>>>struct drm_i915_private at offset 0, take care to check for NULL before
>>>using container_of() to allow for other offsets.
>>
>> I think the second half of this paragraph could be rephrased. One might
>> think that the text is suggesting that checking for NULL has something
>> to do with allowing other offsets.
>>
>> I would jump directly to mention using container_of() and would assume
>> that it is implied that NULL check is necessary before using it. :-)
>
>But I *am* suggesting the NULL check before container_of() has
>everything to do with allowing other offsets!
>
>container_of() will return a NULL pointer for a NULL pointer when the
>offset is 0, but will return a negative garbage pointer otherwise.

Okay, I think I got a better understanding of what you mean.

That said, the following makes more sense *to me*:

- The use of container_of() is what actually allows the use of non-zero
  offsets.

- container_of() shouldn't be used with NULL pointers, it just doesn't
  make sense. So, if we receive a NULL pointer, don't bother using
  container_of() and simply return NULL.

--
Gustavo Sousa

>
>BR,
>Jani.
>
>
>>
>> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>
>>>
>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>---
>>> drivers/gpu/drm/i915/i915_driver.c                | 2 +-
>>> drivers/gpu/drm/i915/i915_drv.h                   | 8 ++++++--
>>> drivers/gpu/drm/i915/selftests/mock_gem_device.c  | 2 +-
>>> drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 4 +++-
>>> drivers/gpu/drm/xe/xe_device.h                    | 8 ++++++--
>>> drivers/gpu/drm/xe/xe_pci.c                       | 2 +-
>>> 6 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>>>index fb8e9c2fcea5..176c13c2e191 100644
>>>--- a/drivers/gpu/drm/i915/i915_driver.c
>>>+++ b/drivers/gpu/drm/i915/i915_driver.c
>>>@@ -723,7 +723,7 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>         if (IS_ERR(i915))
>>>                 return i915;
>>> 
>>>-        pci_set_drvdata(pdev, i915);
>>>+        pci_set_drvdata(pdev, &i915->drm);
>>> 
>>>         /* Device parameters start as a copy of module parameters. */
>>>         i915_params_copy(&i915->params, &i915_modparams);
>>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>index d7723dd11c80..f6edaef73db5 100644
>>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>>@@ -365,12 +365,16 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>>> 
>>> static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
>>> {
>>>-        return dev_get_drvdata(kdev);
>>>+        struct drm_device *drm = dev_get_drvdata(kdev);
>>>+
>>>+        return drm ? to_i915(drm) : NULL;
>>> }
>>> 
>>> static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>>> {
>>>-        return pci_get_drvdata(pdev);
>>>+        struct drm_device *drm = pci_get_drvdata(pdev);
>>>+
>>>+        return drm ? to_i915(drm) : NULL;
>>> }
>>> 
>>> static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
>>>diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>>>index 0bd29846873b..91794ca17a58 100644
>>>--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>>>+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
>>>@@ -172,7 +172,7 @@ struct drm_i915_private *mock_gem_device(void)
>>>                 return NULL;
>>>         }
>>> 
>>>-        pci_set_drvdata(pdev, i915);
>>>+        pci_set_drvdata(pdev, &i915->drm);
>>> 
>>>         /* Device parameters start as a copy of module parameters. */
>>>         i915_params_copy(&i915->params, &i915_modparams);
>>>diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>>index 2feedddf1e40..766fba88a3c8 100644
>>>--- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>>+++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>>@@ -23,7 +23,9 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>>> 
>>> static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
>>> {
>>>-        return dev_get_drvdata(kdev);
>>>+        struct drm_device *drm = dev_get_drvdata(kdev);
>>>+
>>>+        return drm ? to_i915(drm) : NULL;
>>> }
>>> 
>>> #define IS_PLATFORM(xe, x) ((xe)->info.platform == x)
>>>diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>>>index 2c96f1b2aafd..022876eebfd5 100644
>>>--- a/drivers/gpu/drm/xe/xe_device.h
>>>+++ b/drivers/gpu/drm/xe/xe_device.h
>>>@@ -17,12 +17,16 @@ static inline struct xe_device *to_xe_device(const struct drm_device *dev)
>>> 
>>> static inline struct xe_device *kdev_to_xe_device(struct device *kdev)
>>> {
>>>-        return dev_get_drvdata(kdev);
>>>+        struct drm_device *drm = dev_get_drvdata(kdev);
>>>+
>>>+        return drm ? to_xe_device(drm) : NULL;
>>> }
>>> 
>>> static inline struct xe_device *pdev_to_xe_device(struct pci_dev *pdev)
>>> {
>>>-        return pci_get_drvdata(pdev);
>>>+        struct drm_device *drm = pci_get_drvdata(pdev);
>>>+
>>>+        return drm ? to_xe_device(drm) : NULL;
>>> }
>>> 
>>> static inline struct xe_device *xe_device_const_cast(const struct xe_device *xe)
>>>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>>>index 7bb811b4a057..f861b8cf931a 100644
>>>--- a/drivers/gpu/drm/xe/xe_pci.c
>>>+++ b/drivers/gpu/drm/xe/xe_pci.c
>>>@@ -800,7 +800,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>         if (IS_ERR(xe))
>>>                 return PTR_ERR(xe);
>>> 
>>>-        pci_set_drvdata(pdev, xe);
>>>+        pci_set_drvdata(pdev, &xe->drm);
>>> 
>>>         xe_pm_assert_unbounded_bridge(xe);
>>>         subplatform_desc = find_subplatform(xe, desc);
>>>-- 
>>>2.39.2
>>>
>
>-- 
>Jani Nikula, Intel

[PATCH 06/10] drm/i915: support struct device and pci_dev in to_intel_display()

[Jani Nikula]

Now that both xe and i915 store struct drm_device in drvdata, we can
trivially support struct device and struct pci_dev in
to_intel_display().

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index a9d2acdc51a4..ce9c2f9ff5b0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -2208,6 +2208,10 @@ static inline int to_bpp_x16(int bpp)
  */
 #define __drm_device_to_intel_display(p) \
 	(&to_i915(p)->display)
+#define __device_to_intel_display(p)				\
+	__drm_device_to_intel_display(dev_get_drvdata(p))
+#define __pci_dev_to_intel_display(p)				\
+	__drm_device_to_intel_display(pci_get_drvdata(p))
 #define __intel_connector_to_intel_display(p)		\
 	__drm_device_to_intel_display((p)->base.dev)
 #define __intel_crtc_to_intel_display(p)		\
@@ -2231,6 +2235,8 @@ static inline int to_bpp_x16(int bpp)
 #define to_intel_display(p)				\
 	_Generic(*p,					\
 		 __assoc(drm_device, p),		\
+		 __assoc(device, p),			\
+		 __assoc(pci_dev, p),			\
 		 __assoc(intel_connector, p),		\
 		 __assoc(intel_crtc, p),		\
 		 __assoc(intel_crtc_state, p),		\
-- 
2.39.2

Re: [PATCH 06/10] drm/i915: support struct device and pci_dev in to_intel_display()

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:07-03:00)
>Now that both xe and i915 store struct drm_device in drvdata, we can
>trivially support struct device and struct pci_dev in
>to_intel_display().
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/i915/display/intel_display_types.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>index a9d2acdc51a4..ce9c2f9ff5b0 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_types.h
>+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>@@ -2208,6 +2208,10 @@ static inline int to_bpp_x16(int bpp)
>  */
> #define __drm_device_to_intel_display(p) \
>         (&to_i915(p)->display)
>+#define __device_to_intel_display(p)                                \
>+        __drm_device_to_intel_display(dev_get_drvdata(p))
>+#define __pci_dev_to_intel_display(p)                                \
>+        __drm_device_to_intel_display(pci_get_drvdata(p))
> #define __intel_connector_to_intel_display(p)                \
>         __drm_device_to_intel_display((p)->base.dev)
> #define __intel_crtc_to_intel_display(p)                \
>@@ -2231,6 +2235,8 @@ static inline int to_bpp_x16(int bpp)
> #define to_intel_display(p)                                \
>         _Generic(*p,                                        \
>                  __assoc(drm_device, p),                \
>+                 __assoc(device, p),                        \
>+                 __assoc(pci_dev, p),                        \
>                  __assoc(intel_connector, p),                \
>                  __assoc(intel_crtc, p),                \
>                  __assoc(intel_crtc_state, p),                \
>-- 
>2.39.2
>

[PATCH 07/10] drm/i915/audio: migrate away from kdev_to_i915()

[Jani Nikula]

Use to_intel_display() instead of kdev_to_i915() in the audio component
API hooks. Avoid further drive-by changes at this point, and just
convert the display pointer to i915, and leave the struct intel_display
conversion for later.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 34 +++++++++++++---------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index b9bafec06fb8..df2879538738 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -979,7 +979,8 @@ static void glk_force_audio_cdclk(struct drm_i915_private *i915,
 
 static unsigned long i915_audio_component_get_power(struct device *kdev)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_display *display = to_intel_display(kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	intel_wakeref_t ret;
 
 	/* Catch potential impedance mismatches before they occur! */
@@ -1011,7 +1012,8 @@ static unsigned long i915_audio_component_get_power(struct device *kdev)
 static void i915_audio_component_put_power(struct device *kdev,
 					   unsigned long cookie)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_display *display = to_intel_display(kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 
 	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
 	if (--i915->display.audio.power_refcount == 0)
@@ -1024,7 +1026,8 @@ static void i915_audio_component_put_power(struct device *kdev,
 static void i915_audio_component_codec_wake_override(struct device *kdev,
 						     bool enable)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_display *display = to_intel_display(kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	unsigned long cookie;
 
 	if (DISPLAY_VER(i915) < 9)
@@ -1052,7 +1055,8 @@ static void i915_audio_component_codec_wake_override(struct device *kdev,
 /* Get CDCLK in kHz  */
 static int i915_audio_component_get_cdclk_freq(struct device *kdev)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_display *display = to_intel_display(kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 
 	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_DDI(i915)))
 		return -ENODEV;
@@ -1111,7 +1115,8 @@ static struct intel_audio_state *find_audio_state(struct drm_i915_private *i915,
 static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 						int cpu_transcoder, int rate)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_display *display = to_intel_display(kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct i915_audio_component *acomp = i915->display.audio.component;
 	const struct intel_audio_state *audio_state;
 	struct intel_encoder *encoder;
@@ -1153,7 +1158,8 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
 					int cpu_transcoder, bool *enabled,
 					unsigned char *buf, int max_bytes)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_display *display = to_intel_display(kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	const struct intel_audio_state *audio_state;
 	int ret = 0;
 
@@ -1188,24 +1194,25 @@ static const struct drm_audio_component_ops i915_audio_component_ops = {
 	.get_eld	= i915_audio_component_get_eld,
 };
 
-static int i915_audio_component_bind(struct device *i915_kdev,
+static int i915_audio_component_bind(struct device *drv_kdev,
 				     struct device *hda_kdev, void *data)
 {
+	struct intel_display *display = to_intel_display(drv_kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct i915_audio_component *acomp = data;
-	struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
 	int i;
 
 	if (drm_WARN_ON(&i915->drm, acomp->base.ops || acomp->base.dev))
 		return -EEXIST;
 
 	if (drm_WARN_ON(&i915->drm,
-			!device_link_add(hda_kdev, i915_kdev,
+			!device_link_add(hda_kdev, drv_kdev,
 					 DL_FLAG_STATELESS)))
 		return -ENOMEM;
 
 	drm_modeset_lock_all(&i915->drm);
 	acomp->base.ops = &i915_audio_component_ops;
-	acomp->base.dev = i915_kdev;
+	acomp->base.dev = drv_kdev;
 	BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
 	for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
 		acomp->aud_sample_rate[i] = 0;
@@ -1215,11 +1222,12 @@ static int i915_audio_component_bind(struct device *i915_kdev,
 	return 0;
 }
 
-static void i915_audio_component_unbind(struct device *i915_kdev,
+static void i915_audio_component_unbind(struct device *drv_kdev,
 					struct device *hda_kdev, void *data)
 {
+	struct intel_display *display = to_intel_display(drv_kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct i915_audio_component *acomp = data;
-	struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
 
 	drm_modeset_lock_all(&i915->drm);
 	acomp->base.ops = NULL;
@@ -1227,7 +1235,7 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
 	i915->display.audio.component = NULL;
 	drm_modeset_unlock_all(&i915->drm);
 
-	device_link_remove(hda_kdev, i915_kdev);
+	device_link_remove(hda_kdev, drv_kdev);
 
 	if (i915->display.audio.power_refcount)
 		drm_err(&i915->drm, "audio power refcount %d after unbind\n",
-- 
2.39.2

Re: [PATCH 07/10] drm/i915/audio: migrate away from kdev_to_i915()

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:08-03:00)
>Use to_intel_display() instead of kdev_to_i915() in the audio component
>API hooks. Avoid further drive-by changes at this point, and just
>convert the display pointer to i915, and leave the struct intel_display
>conversion for later.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/i915/display/intel_audio.c | 34 +++++++++++++---------
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
>index b9bafec06fb8..df2879538738 100644
>--- a/drivers/gpu/drm/i915/display/intel_audio.c
>+++ b/drivers/gpu/drm/i915/display/intel_audio.c
>@@ -979,7 +979,8 @@ static void glk_force_audio_cdclk(struct drm_i915_private *i915,
> 
> static unsigned long i915_audio_component_get_power(struct device *kdev)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(kdev);
>+        struct intel_display *display = to_intel_display(kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
>         intel_wakeref_t ret;
> 
>         /* Catch potential impedance mismatches before they occur! */
>@@ -1011,7 +1012,8 @@ static unsigned long i915_audio_component_get_power(struct device *kdev)
> static void i915_audio_component_put_power(struct device *kdev,
>                                            unsigned long cookie)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(kdev);
>+        struct intel_display *display = to_intel_display(kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
> 
>         /* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
>         if (--i915->display.audio.power_refcount == 0)
>@@ -1024,7 +1026,8 @@ static void i915_audio_component_put_power(struct device *kdev,
> static void i915_audio_component_codec_wake_override(struct device *kdev,
>                                                      bool enable)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(kdev);
>+        struct intel_display *display = to_intel_display(kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
>         unsigned long cookie;
> 
>         if (DISPLAY_VER(i915) < 9)
>@@ -1052,7 +1055,8 @@ static void i915_audio_component_codec_wake_override(struct device *kdev,
> /* Get CDCLK in kHz  */
> static int i915_audio_component_get_cdclk_freq(struct device *kdev)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(kdev);
>+        struct intel_display *display = to_intel_display(kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
> 
>         if (drm_WARN_ON_ONCE(&i915->drm, !HAS_DDI(i915)))
>                 return -ENODEV;
>@@ -1111,7 +1115,8 @@ static struct intel_audio_state *find_audio_state(struct drm_i915_private *i915,
> static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>                                                 int cpu_transcoder, int rate)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(kdev);
>+        struct intel_display *display = to_intel_display(kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
>         struct i915_audio_component *acomp = i915->display.audio.component;
>         const struct intel_audio_state *audio_state;
>         struct intel_encoder *encoder;
>@@ -1153,7 +1158,8 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
>                                         int cpu_transcoder, bool *enabled,
>                                         unsigned char *buf, int max_bytes)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(kdev);
>+        struct intel_display *display = to_intel_display(kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
>         const struct intel_audio_state *audio_state;
>         int ret = 0;
> 
>@@ -1188,24 +1194,25 @@ static const struct drm_audio_component_ops i915_audio_component_ops = {
>         .get_eld        = i915_audio_component_get_eld,
> };
> 
>-static int i915_audio_component_bind(struct device *i915_kdev,
>+static int i915_audio_component_bind(struct device *drv_kdev,
>                                      struct device *hda_kdev, void *data)
> {
>+        struct intel_display *display = to_intel_display(drv_kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
>         struct i915_audio_component *acomp = data;
>-        struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
>         int i;
> 
>         if (drm_WARN_ON(&i915->drm, acomp->base.ops || acomp->base.dev))
>                 return -EEXIST;
> 
>         if (drm_WARN_ON(&i915->drm,
>-                        !device_link_add(hda_kdev, i915_kdev,
>+                        !device_link_add(hda_kdev, drv_kdev,
>                                          DL_FLAG_STATELESS)))
>                 return -ENOMEM;
> 
>         drm_modeset_lock_all(&i915->drm);
>         acomp->base.ops = &i915_audio_component_ops;
>-        acomp->base.dev = i915_kdev;
>+        acomp->base.dev = drv_kdev;
>         BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
>         for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
>                 acomp->aud_sample_rate[i] = 0;
>@@ -1215,11 +1222,12 @@ static int i915_audio_component_bind(struct device *i915_kdev,
>         return 0;
> }
> 
>-static void i915_audio_component_unbind(struct device *i915_kdev,
>+static void i915_audio_component_unbind(struct device *drv_kdev,
>                                         struct device *hda_kdev, void *data)
> {
>+        struct intel_display *display = to_intel_display(drv_kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
>         struct i915_audio_component *acomp = data;
>-        struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
> 
>         drm_modeset_lock_all(&i915->drm);
>         acomp->base.ops = NULL;
>@@ -1227,7 +1235,7 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
>         i915->display.audio.component = NULL;
>         drm_modeset_unlock_all(&i915->drm);
> 
>-        device_link_remove(hda_kdev, i915_kdev);
>+        device_link_remove(hda_kdev, drv_kdev);
> 
>         if (i915->display.audio.power_refcount)
>                 drm_err(&i915->drm, "audio power refcount %d after unbind\n",
>-- 
>2.39.2
>

[PATCH 08/10] drm/i915/hdcp: migrate away from kdev_to_i915() in bind/unbind

[Jani Nikula]

Use to_intel_display() instead of kdev_to_i915() in the HDCP component
API hooks. Avoid further drive-by changes at this point, and just
convert the display pointer to i915, and leave the struct intel_display
conversion for later.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 05402ae6b569..42f8f9d41de6 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2181,10 +2181,11 @@ static void intel_hdcp_check_work(struct work_struct *work)
 				   DRM_HDCP_CHECK_PERIOD_MS);
 }
 
-static int i915_hdcp_component_bind(struct device *i915_kdev,
+static int i915_hdcp_component_bind(struct device *drv_kdev,
 				    struct device *mei_kdev, void *data)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
+	struct intel_display *display = to_intel_display(drv_kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 
 	drm_dbg(&i915->drm, "I915 HDCP comp bind\n");
 	mutex_lock(&i915->display.hdcp.hdcp_mutex);
@@ -2195,10 +2196,11 @@ static int i915_hdcp_component_bind(struct device *i915_kdev,
 	return 0;
 }
 
-static void i915_hdcp_component_unbind(struct device *i915_kdev,
+static void i915_hdcp_component_unbind(struct device *drv_kdev,
 				       struct device *mei_kdev, void *data)
 {
-	struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
+	struct intel_display *display = to_intel_display(drv_kdev);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 
 	drm_dbg(&i915->drm, "I915 HDCP comp unbind\n");
 	mutex_lock(&i915->display.hdcp.hdcp_mutex);
-- 
2.39.2

Re: [PATCH 08/10] drm/i915/hdcp: migrate away from kdev_to_i915() in bind/unbind

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:09-03:00)
>Use to_intel_display() instead of kdev_to_i915() in the HDCP component
>API hooks. Avoid further drive-by changes at this point, and just
>convert the display pointer to i915, and leave the struct intel_display
>conversion for later.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/i915/display/intel_hdcp.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
>index 05402ae6b569..42f8f9d41de6 100644
>--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>@@ -2181,10 +2181,11 @@ static void intel_hdcp_check_work(struct work_struct *work)
>                                    DRM_HDCP_CHECK_PERIOD_MS);
> }
> 
>-static int i915_hdcp_component_bind(struct device *i915_kdev,
>+static int i915_hdcp_component_bind(struct device *drv_kdev,
>                                     struct device *mei_kdev, void *data)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
>+        struct intel_display *display = to_intel_display(drv_kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
> 
>         drm_dbg(&i915->drm, "I915 HDCP comp bind\n");
>         mutex_lock(&i915->display.hdcp.hdcp_mutex);
>@@ -2195,10 +2196,11 @@ static int i915_hdcp_component_bind(struct device *i915_kdev,
>         return 0;
> }
> 
>-static void i915_hdcp_component_unbind(struct device *i915_kdev,
>+static void i915_hdcp_component_unbind(struct device *drv_kdev,
>                                        struct device *mei_kdev, void *data)
> {
>-        struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
>+        struct intel_display *display = to_intel_display(drv_kdev);
>+        struct drm_i915_private *i915 = to_i915(display->drm);
> 
>         drm_dbg(&i915->drm, "I915 HDCP comp unbind\n");
>         mutex_lock(&i915->display.hdcp.hdcp_mutex);
>-- 
>2.39.2
>

[PATCH 09/10] drm/i915/hdcp: migrate away from kdev_to_i915() in GSC messaging

[Jani Nikula]

Use to_intel_display() instead of kdev_to_i915() in the HDCP component
API hooks. Avoid further drive-by changes at this point, and just
convert the display pointer to i915, and leave the struct intel_display
conversion for later.

The NULL error checking in the hooks make this a bit cumbersome. I'm not
actually sure they're really required, but don't go down that rabbit
hole just now.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_hdcp_gsc_message.c | 67 +++++++++++++------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
index 6548e71b4c49..35bdb532bbb3 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
@@ -7,6 +7,7 @@
 #include <drm/intel/i915_hdcp_interface.h>
 
 #include "i915_drv.h"
+#include "intel_display_types.h"
 #include "intel_hdcp_gsc_message.h"
 
 int
@@ -15,17 +16,19 @@ intel_hdcp_gsc_initiate_session(struct device *dev, struct hdcp_port_data *data,
 {
 	struct wired_cmd_initiate_hdcp2_session_in session_init_in = {};
 	struct wired_cmd_initiate_hdcp2_session_out session_init_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !ake_data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	session_init_in.header.api_version = HDCP_API_VERSION;
 	session_init_in.header.command_id = WIRED_INITIATE_HDCP2_SESSION;
@@ -72,17 +75,19 @@ intel_hdcp_gsc_verify_receiver_cert_prepare_km(struct device *dev,
 {
 	struct wired_cmd_verify_receiver_cert_in verify_rxcert_in = {};
 	struct wired_cmd_verify_receiver_cert_out verify_rxcert_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !rx_cert || !km_stored || !ek_pub_km || !msg_sz)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	verify_rxcert_in.header.api_version = HDCP_API_VERSION;
 	verify_rxcert_in.header.command_id = WIRED_VERIFY_RECEIVER_CERT;
@@ -135,17 +140,19 @@ intel_hdcp_gsc_verify_hprime(struct device *dev, struct hdcp_port_data *data,
 {
 	struct wired_cmd_ake_send_hprime_in send_hprime_in = {};
 	struct wired_cmd_ake_send_hprime_out send_hprime_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !rx_hprime)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	send_hprime_in.header.api_version = HDCP_API_VERSION;
 	send_hprime_in.header.command_id = WIRED_AKE_SEND_HPRIME;
@@ -183,17 +190,19 @@ intel_hdcp_gsc_store_pairing_info(struct device *dev, struct hdcp_port_data *dat
 {
 	struct wired_cmd_ake_send_pairing_info_in pairing_info_in = {};
 	struct wired_cmd_ake_send_pairing_info_out pairing_info_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !pairing_info)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	pairing_info_in.header.api_version = HDCP_API_VERSION;
 	pairing_info_in.header.command_id = WIRED_AKE_SEND_PAIRING_INFO;
@@ -234,17 +243,19 @@ intel_hdcp_gsc_initiate_locality_check(struct device *dev,
 {
 	struct wired_cmd_init_locality_check_in lc_init_in = {};
 	struct wired_cmd_init_locality_check_out lc_init_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !lc_init_data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	lc_init_in.header.api_version = HDCP_API_VERSION;
 	lc_init_in.header.command_id = WIRED_INIT_LOCALITY_CHECK;
@@ -280,17 +291,19 @@ intel_hdcp_gsc_verify_lprime(struct device *dev, struct hdcp_port_data *data,
 {
 	struct wired_cmd_validate_locality_in verify_lprime_in = {};
 	struct wired_cmd_validate_locality_out verify_lprime_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !rx_lprime)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	verify_lprime_in.header.api_version = HDCP_API_VERSION;
 	verify_lprime_in.header.command_id = WIRED_VALIDATE_LOCALITY;
@@ -330,17 +343,19 @@ int intel_hdcp_gsc_get_session_key(struct device *dev,
 {
 	struct wired_cmd_get_session_key_in get_skey_in = {};
 	struct wired_cmd_get_session_key_out get_skey_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data || !ske_data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	get_skey_in.header.api_version = HDCP_API_VERSION;
 	get_skey_in.header.command_id = WIRED_GET_SESSION_KEY;
@@ -382,17 +397,19 @@ intel_hdcp_gsc_repeater_check_flow_prepare_ack(struct device *dev,
 {
 	struct wired_cmd_verify_repeater_in verify_repeater_in = {};
 	struct wired_cmd_verify_repeater_out verify_repeater_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !rep_topology || !rep_send_ack || !data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	verify_repeater_in.header.api_version = HDCP_API_VERSION;
 	verify_repeater_in.header.command_id = WIRED_VERIFY_REPEATER;
@@ -442,6 +459,7 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
 {
 	struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
 	struct wired_cmd_repeater_auth_stream_req_out verify_mprime_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 	size_t cmd_size;
@@ -449,11 +467,12 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
 	if (!dev || !stream_ready || !data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	cmd_size = struct_size(verify_mprime_in, streams, data->k);
 	if (cmd_size == SIZE_MAX)
@@ -504,17 +523,19 @@ int intel_hdcp_gsc_enable_authentication(struct device *dev,
 {
 	struct wired_cmd_enable_auth_in enable_auth_in = {};
 	struct wired_cmd_enable_auth_out enable_auth_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	enable_auth_in.header.api_version = HDCP_API_VERSION;
 	enable_auth_in.header.command_id = WIRED_ENABLE_AUTH;
@@ -549,17 +570,19 @@ intel_hdcp_gsc_close_session(struct device *dev, struct hdcp_port_data *data)
 {
 	struct wired_cmd_close_session_in session_close_in = {};
 	struct wired_cmd_close_session_out session_close_out = {};
+	struct intel_display *display;
 	struct drm_i915_private *i915;
 	ssize_t byte;
 
 	if (!dev || !data)
 		return -EINVAL;
 
-	i915 = kdev_to_i915(dev);
-	if (!i915) {
+	display = to_intel_display(dev);
+	if (!display) {
 		dev_err(dev, "DRM not initialized, aborting HDCP.\n");
 		return -ENODEV;
 	}
+	i915 = to_i915(display->drm);
 
 	session_close_in.header.api_version = HDCP_API_VERSION;
 	session_close_in.header.command_id = WIRED_CLOSE_SESSION;
-- 
2.39.2

Re: [PATCH 09/10] drm/i915/hdcp: migrate away from kdev_to_i915() in GSC messaging

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:10-03:00)
>Use to_intel_display() instead of kdev_to_i915() in the HDCP component
>API hooks. Avoid further drive-by changes at this point, and just
>convert the display pointer to i915, and leave the struct intel_display
>conversion for later.
>
>The NULL error checking in the hooks make this a bit cumbersome. I'm not
>actually sure they're really required, but don't go down that rabbit
>hole just now.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> .../drm/i915/display/intel_hdcp_gsc_message.c | 67 +++++++++++++------
> 1 file changed, 45 insertions(+), 22 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>index 6548e71b4c49..35bdb532bbb3 100644
>--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>@@ -7,6 +7,7 @@
> #include <drm/intel/i915_hdcp_interface.h>
> 
> #include "i915_drv.h"
>+#include "intel_display_types.h"
> #include "intel_hdcp_gsc_message.h"
> 
> int
>@@ -15,17 +16,19 @@ intel_hdcp_gsc_initiate_session(struct device *dev, struct hdcp_port_data *data,
> {
>         struct wired_cmd_initiate_hdcp2_session_in session_init_in = {};
>         struct wired_cmd_initiate_hdcp2_session_out session_init_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !ake_data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {

Hm... I'm afraid that wouldn't really work if drvdata was NULL, would
it?

With a NULL drvdata, I believe we would probably get a non-zero offset
here.

--
Gustavo Sousa

>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         session_init_in.header.api_version = HDCP_API_VERSION;
>         session_init_in.header.command_id = WIRED_INITIATE_HDCP2_SESSION;
>@@ -72,17 +75,19 @@ intel_hdcp_gsc_verify_receiver_cert_prepare_km(struct device *dev,
> {
>         struct wired_cmd_verify_receiver_cert_in verify_rxcert_in = {};
>         struct wired_cmd_verify_receiver_cert_out verify_rxcert_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !rx_cert || !km_stored || !ek_pub_km || !msg_sz)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         verify_rxcert_in.header.api_version = HDCP_API_VERSION;
>         verify_rxcert_in.header.command_id = WIRED_VERIFY_RECEIVER_CERT;
>@@ -135,17 +140,19 @@ intel_hdcp_gsc_verify_hprime(struct device *dev, struct hdcp_port_data *data,
> {
>         struct wired_cmd_ake_send_hprime_in send_hprime_in = {};
>         struct wired_cmd_ake_send_hprime_out send_hprime_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !rx_hprime)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         send_hprime_in.header.api_version = HDCP_API_VERSION;
>         send_hprime_in.header.command_id = WIRED_AKE_SEND_HPRIME;
>@@ -183,17 +190,19 @@ intel_hdcp_gsc_store_pairing_info(struct device *dev, struct hdcp_port_data *dat
> {
>         struct wired_cmd_ake_send_pairing_info_in pairing_info_in = {};
>         struct wired_cmd_ake_send_pairing_info_out pairing_info_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !pairing_info)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         pairing_info_in.header.api_version = HDCP_API_VERSION;
>         pairing_info_in.header.command_id = WIRED_AKE_SEND_PAIRING_INFO;
>@@ -234,17 +243,19 @@ intel_hdcp_gsc_initiate_locality_check(struct device *dev,
> {
>         struct wired_cmd_init_locality_check_in lc_init_in = {};
>         struct wired_cmd_init_locality_check_out lc_init_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !lc_init_data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         lc_init_in.header.api_version = HDCP_API_VERSION;
>         lc_init_in.header.command_id = WIRED_INIT_LOCALITY_CHECK;
>@@ -280,17 +291,19 @@ intel_hdcp_gsc_verify_lprime(struct device *dev, struct hdcp_port_data *data,
> {
>         struct wired_cmd_validate_locality_in verify_lprime_in = {};
>         struct wired_cmd_validate_locality_out verify_lprime_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !rx_lprime)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         verify_lprime_in.header.api_version = HDCP_API_VERSION;
>         verify_lprime_in.header.command_id = WIRED_VALIDATE_LOCALITY;
>@@ -330,17 +343,19 @@ int intel_hdcp_gsc_get_session_key(struct device *dev,
> {
>         struct wired_cmd_get_session_key_in get_skey_in = {};
>         struct wired_cmd_get_session_key_out get_skey_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data || !ske_data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         get_skey_in.header.api_version = HDCP_API_VERSION;
>         get_skey_in.header.command_id = WIRED_GET_SESSION_KEY;
>@@ -382,17 +397,19 @@ intel_hdcp_gsc_repeater_check_flow_prepare_ack(struct device *dev,
> {
>         struct wired_cmd_verify_repeater_in verify_repeater_in = {};
>         struct wired_cmd_verify_repeater_out verify_repeater_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !rep_topology || !rep_send_ack || !data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         verify_repeater_in.header.api_version = HDCP_API_VERSION;
>         verify_repeater_in.header.command_id = WIRED_VERIFY_REPEATER;
>@@ -442,6 +459,7 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
> {
>         struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
>         struct wired_cmd_repeater_auth_stream_req_out verify_mprime_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
>         size_t cmd_size;
>@@ -449,11 +467,12 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>         if (!dev || !stream_ready || !data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         cmd_size = struct_size(verify_mprime_in, streams, data->k);
>         if (cmd_size == SIZE_MAX)
>@@ -504,17 +523,19 @@ int intel_hdcp_gsc_enable_authentication(struct device *dev,
> {
>         struct wired_cmd_enable_auth_in enable_auth_in = {};
>         struct wired_cmd_enable_auth_out enable_auth_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         enable_auth_in.header.api_version = HDCP_API_VERSION;
>         enable_auth_in.header.command_id = WIRED_ENABLE_AUTH;
>@@ -549,17 +570,19 @@ intel_hdcp_gsc_close_session(struct device *dev, struct hdcp_port_data *data)
> {
>         struct wired_cmd_close_session_in session_close_in = {};
>         struct wired_cmd_close_session_out session_close_out = {};
>+        struct intel_display *display;
>         struct drm_i915_private *i915;
>         ssize_t byte;
> 
>         if (!dev || !data)
>                 return -EINVAL;
> 
>-        i915 = kdev_to_i915(dev);
>-        if (!i915) {
>+        display = to_intel_display(dev);
>+        if (!display) {
>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>                 return -ENODEV;
>         }
>+        i915 = to_i915(display->drm);
> 
>         session_close_in.header.api_version = HDCP_API_VERSION;
>         session_close_in.header.command_id = WIRED_CLOSE_SESSION;
>-- 
>2.39.2
>

Re: [PATCH 09/10] drm/i915/hdcp: migrate away from kdev_to_i915() in GSC messaging

[Jani Nikula]

On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Jani Nikula (2024-07-29 11:30:10-03:00)
>>Use to_intel_display() instead of kdev_to_i915() in the HDCP component
>>API hooks. Avoid further drive-by changes at this point, and just
>>convert the display pointer to i915, and leave the struct intel_display
>>conversion for later.
>>
>>The NULL error checking in the hooks make this a bit cumbersome. I'm not
>>actually sure they're really required, but don't go down that rabbit
>>hole just now.
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> .../drm/i915/display/intel_hdcp_gsc_message.c | 67 +++++++++++++------
>> 1 file changed, 45 insertions(+), 22 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>index 6548e71b4c49..35bdb532bbb3 100644
>>--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>@@ -7,6 +7,7 @@
>> #include <drm/intel/i915_hdcp_interface.h>
>> 
>> #include "i915_drv.h"
>>+#include "intel_display_types.h"
>> #include "intel_hdcp_gsc_message.h"
>> 
>> int
>>@@ -15,17 +16,19 @@ intel_hdcp_gsc_initiate_session(struct device *dev, struct hdcp_port_data *data,
>> {
>>         struct wired_cmd_initiate_hdcp2_session_in session_init_in = {};
>>         struct wired_cmd_initiate_hdcp2_session_out session_init_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !ake_data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>
> Hm... I'm afraid that wouldn't really work if drvdata was NULL, would
> it?
>
> With a NULL drvdata, I believe we would probably get a non-zero offset
> here.

Right, good catch. Not sure I want to add that check to everything in
to_intel_display() macro, because in most cases the passed in value
can't be NULL. Driver data is a bit different, though I don't think it
should really be NULL either.

BR,
Jani.



>
> --
> Gustavo Sousa
>
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         session_init_in.header.api_version = HDCP_API_VERSION;
>>         session_init_in.header.command_id = WIRED_INITIATE_HDCP2_SESSION;
>>@@ -72,17 +75,19 @@ intel_hdcp_gsc_verify_receiver_cert_prepare_km(struct device *dev,
>> {
>>         struct wired_cmd_verify_receiver_cert_in verify_rxcert_in = {};
>>         struct wired_cmd_verify_receiver_cert_out verify_rxcert_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !rx_cert || !km_stored || !ek_pub_km || !msg_sz)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         verify_rxcert_in.header.api_version = HDCP_API_VERSION;
>>         verify_rxcert_in.header.command_id = WIRED_VERIFY_RECEIVER_CERT;
>>@@ -135,17 +140,19 @@ intel_hdcp_gsc_verify_hprime(struct device *dev, struct hdcp_port_data *data,
>> {
>>         struct wired_cmd_ake_send_hprime_in send_hprime_in = {};
>>         struct wired_cmd_ake_send_hprime_out send_hprime_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !rx_hprime)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         send_hprime_in.header.api_version = HDCP_API_VERSION;
>>         send_hprime_in.header.command_id = WIRED_AKE_SEND_HPRIME;
>>@@ -183,17 +190,19 @@ intel_hdcp_gsc_store_pairing_info(struct device *dev, struct hdcp_port_data *dat
>> {
>>         struct wired_cmd_ake_send_pairing_info_in pairing_info_in = {};
>>         struct wired_cmd_ake_send_pairing_info_out pairing_info_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !pairing_info)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         pairing_info_in.header.api_version = HDCP_API_VERSION;
>>         pairing_info_in.header.command_id = WIRED_AKE_SEND_PAIRING_INFO;
>>@@ -234,17 +243,19 @@ intel_hdcp_gsc_initiate_locality_check(struct device *dev,
>> {
>>         struct wired_cmd_init_locality_check_in lc_init_in = {};
>>         struct wired_cmd_init_locality_check_out lc_init_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !lc_init_data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         lc_init_in.header.api_version = HDCP_API_VERSION;
>>         lc_init_in.header.command_id = WIRED_INIT_LOCALITY_CHECK;
>>@@ -280,17 +291,19 @@ intel_hdcp_gsc_verify_lprime(struct device *dev, struct hdcp_port_data *data,
>> {
>>         struct wired_cmd_validate_locality_in verify_lprime_in = {};
>>         struct wired_cmd_validate_locality_out verify_lprime_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !rx_lprime)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         verify_lprime_in.header.api_version = HDCP_API_VERSION;
>>         verify_lprime_in.header.command_id = WIRED_VALIDATE_LOCALITY;
>>@@ -330,17 +343,19 @@ int intel_hdcp_gsc_get_session_key(struct device *dev,
>> {
>>         struct wired_cmd_get_session_key_in get_skey_in = {};
>>         struct wired_cmd_get_session_key_out get_skey_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data || !ske_data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         get_skey_in.header.api_version = HDCP_API_VERSION;
>>         get_skey_in.header.command_id = WIRED_GET_SESSION_KEY;
>>@@ -382,17 +397,19 @@ intel_hdcp_gsc_repeater_check_flow_prepare_ack(struct device *dev,
>> {
>>         struct wired_cmd_verify_repeater_in verify_repeater_in = {};
>>         struct wired_cmd_verify_repeater_out verify_repeater_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !rep_topology || !rep_send_ack || !data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         verify_repeater_in.header.api_version = HDCP_API_VERSION;
>>         verify_repeater_in.header.command_id = WIRED_VERIFY_REPEATER;
>>@@ -442,6 +459,7 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>> {
>>         struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
>>         struct wired_cmd_repeater_auth_stream_req_out verify_mprime_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>>         size_t cmd_size;
>>@@ -449,11 +467,12 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>>         if (!dev || !stream_ready || !data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         cmd_size = struct_size(verify_mprime_in, streams, data->k);
>>         if (cmd_size == SIZE_MAX)
>>@@ -504,17 +523,19 @@ int intel_hdcp_gsc_enable_authentication(struct device *dev,
>> {
>>         struct wired_cmd_enable_auth_in enable_auth_in = {};
>>         struct wired_cmd_enable_auth_out enable_auth_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         enable_auth_in.header.api_version = HDCP_API_VERSION;
>>         enable_auth_in.header.command_id = WIRED_ENABLE_AUTH;
>>@@ -549,17 +570,19 @@ intel_hdcp_gsc_close_session(struct device *dev, struct hdcp_port_data *data)
>> {
>>         struct wired_cmd_close_session_in session_close_in = {};
>>         struct wired_cmd_close_session_out session_close_out = {};
>>+        struct intel_display *display;
>>         struct drm_i915_private *i915;
>>         ssize_t byte;
>> 
>>         if (!dev || !data)
>>                 return -EINVAL;
>> 
>>-        i915 = kdev_to_i915(dev);
>>-        if (!i915) {
>>+        display = to_intel_display(dev);
>>+        if (!display) {
>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>                 return -ENODEV;
>>         }
>>+        i915 = to_i915(display->drm);
>> 
>>         session_close_in.header.api_version = HDCP_API_VERSION;
>>         session_close_in.header.command_id = WIRED_CLOSE_SESSION;
>>-- 
>>2.39.2
>>

-- 
Jani Nikula, Intel

Re: [PATCH 09/10] drm/i915/hdcp: migrate away from kdev_to_i915() in GSC messaging

[Jani Nikula]

On Tue, 06 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> Quoting Jani Nikula (2024-07-29 11:30:10-03:00)
>>>Use to_intel_display() instead of kdev_to_i915() in the HDCP component
>>>API hooks. Avoid further drive-by changes at this point, and just
>>>convert the display pointer to i915, and leave the struct intel_display
>>>conversion for later.
>>>
>>>The NULL error checking in the hooks make this a bit cumbersome. I'm not
>>>actually sure they're really required, but don't go down that rabbit
>>>hole just now.
>>>
>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>---
>>> .../drm/i915/display/intel_hdcp_gsc_message.c | 67 +++++++++++++------
>>> 1 file changed, 45 insertions(+), 22 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>>index 6548e71b4c49..35bdb532bbb3 100644
>>>--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>>+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>>@@ -7,6 +7,7 @@
>>> #include <drm/intel/i915_hdcp_interface.h>
>>> 
>>> #include "i915_drv.h"
>>>+#include "intel_display_types.h"
>>> #include "intel_hdcp_gsc_message.h"
>>> 
>>> int
>>>@@ -15,17 +16,19 @@ intel_hdcp_gsc_initiate_session(struct device *dev, struct hdcp_port_data *data,
>>> {
>>>         struct wired_cmd_initiate_hdcp2_session_in session_init_in = {};
>>>         struct wired_cmd_initiate_hdcp2_session_out session_init_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !ake_data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>
>> Hm... I'm afraid that wouldn't really work if drvdata was NULL, would
>> it?
>>
>> With a NULL drvdata, I believe we would probably get a non-zero offset
>> here.
>
> Right, good catch. Not sure I want to add that check to everything in
> to_intel_display() macro, because in most cases the passed in value
> can't be NULL. Driver data is a bit different, though I don't think it
> should really be NULL either.

Decided to fix this in __drm_device_to_intel_display() like this:

 #define __drm_device_to_intel_display(p) \
-       (&to_i915(p)->display)
+       ((p) ? &to_i915(p)->display : NULL)

That'll have to change anyway when we can no longer use struct
drm_i915_private when going from drm_device to intel_display.

BR,
Jani.

>
> BR,
> Jani.
>
>
>
>>
>> --
>> Gustavo Sousa
>>
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         session_init_in.header.api_version = HDCP_API_VERSION;
>>>         session_init_in.header.command_id = WIRED_INITIATE_HDCP2_SESSION;
>>>@@ -72,17 +75,19 @@ intel_hdcp_gsc_verify_receiver_cert_prepare_km(struct device *dev,
>>> {
>>>         struct wired_cmd_verify_receiver_cert_in verify_rxcert_in = {};
>>>         struct wired_cmd_verify_receiver_cert_out verify_rxcert_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !rx_cert || !km_stored || !ek_pub_km || !msg_sz)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         verify_rxcert_in.header.api_version = HDCP_API_VERSION;
>>>         verify_rxcert_in.header.command_id = WIRED_VERIFY_RECEIVER_CERT;
>>>@@ -135,17 +140,19 @@ intel_hdcp_gsc_verify_hprime(struct device *dev, struct hdcp_port_data *data,
>>> {
>>>         struct wired_cmd_ake_send_hprime_in send_hprime_in = {};
>>>         struct wired_cmd_ake_send_hprime_out send_hprime_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !rx_hprime)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         send_hprime_in.header.api_version = HDCP_API_VERSION;
>>>         send_hprime_in.header.command_id = WIRED_AKE_SEND_HPRIME;
>>>@@ -183,17 +190,19 @@ intel_hdcp_gsc_store_pairing_info(struct device *dev, struct hdcp_port_data *dat
>>> {
>>>         struct wired_cmd_ake_send_pairing_info_in pairing_info_in = {};
>>>         struct wired_cmd_ake_send_pairing_info_out pairing_info_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !pairing_info)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         pairing_info_in.header.api_version = HDCP_API_VERSION;
>>>         pairing_info_in.header.command_id = WIRED_AKE_SEND_PAIRING_INFO;
>>>@@ -234,17 +243,19 @@ intel_hdcp_gsc_initiate_locality_check(struct device *dev,
>>> {
>>>         struct wired_cmd_init_locality_check_in lc_init_in = {};
>>>         struct wired_cmd_init_locality_check_out lc_init_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !lc_init_data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         lc_init_in.header.api_version = HDCP_API_VERSION;
>>>         lc_init_in.header.command_id = WIRED_INIT_LOCALITY_CHECK;
>>>@@ -280,17 +291,19 @@ intel_hdcp_gsc_verify_lprime(struct device *dev, struct hdcp_port_data *data,
>>> {
>>>         struct wired_cmd_validate_locality_in verify_lprime_in = {};
>>>         struct wired_cmd_validate_locality_out verify_lprime_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !rx_lprime)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         verify_lprime_in.header.api_version = HDCP_API_VERSION;
>>>         verify_lprime_in.header.command_id = WIRED_VALIDATE_LOCALITY;
>>>@@ -330,17 +343,19 @@ int intel_hdcp_gsc_get_session_key(struct device *dev,
>>> {
>>>         struct wired_cmd_get_session_key_in get_skey_in = {};
>>>         struct wired_cmd_get_session_key_out get_skey_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data || !ske_data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         get_skey_in.header.api_version = HDCP_API_VERSION;
>>>         get_skey_in.header.command_id = WIRED_GET_SESSION_KEY;
>>>@@ -382,17 +397,19 @@ intel_hdcp_gsc_repeater_check_flow_prepare_ack(struct device *dev,
>>> {
>>>         struct wired_cmd_verify_repeater_in verify_repeater_in = {};
>>>         struct wired_cmd_verify_repeater_out verify_repeater_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !rep_topology || !rep_send_ack || !data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         verify_repeater_in.header.api_version = HDCP_API_VERSION;
>>>         verify_repeater_in.header.command_id = WIRED_VERIFY_REPEATER;
>>>@@ -442,6 +459,7 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>>> {
>>>         struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
>>>         struct wired_cmd_repeater_auth_stream_req_out verify_mprime_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>>         size_t cmd_size;
>>>@@ -449,11 +467,12 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>>>         if (!dev || !stream_ready || !data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         cmd_size = struct_size(verify_mprime_in, streams, data->k);
>>>         if (cmd_size == SIZE_MAX)
>>>@@ -504,17 +523,19 @@ int intel_hdcp_gsc_enable_authentication(struct device *dev,
>>> {
>>>         struct wired_cmd_enable_auth_in enable_auth_in = {};
>>>         struct wired_cmd_enable_auth_out enable_auth_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         enable_auth_in.header.api_version = HDCP_API_VERSION;
>>>         enable_auth_in.header.command_id = WIRED_ENABLE_AUTH;
>>>@@ -549,17 +570,19 @@ intel_hdcp_gsc_close_session(struct device *dev, struct hdcp_port_data *data)
>>> {
>>>         struct wired_cmd_close_session_in session_close_in = {};
>>>         struct wired_cmd_close_session_out session_close_out = {};
>>>+        struct intel_display *display;
>>>         struct drm_i915_private *i915;
>>>         ssize_t byte;
>>> 
>>>         if (!dev || !data)
>>>                 return -EINVAL;
>>> 
>>>-        i915 = kdev_to_i915(dev);
>>>-        if (!i915) {
>>>+        display = to_intel_display(dev);
>>>+        if (!display) {
>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>                 return -ENODEV;
>>>         }
>>>+        i915 = to_i915(display->drm);
>>> 
>>>         session_close_in.header.api_version = HDCP_API_VERSION;
>>>         session_close_in.header.command_id = WIRED_CLOSE_SESSION;
>>>-- 
>>>2.39.2
>>>

-- 
Jani Nikula, Intel

Re: [PATCH 09/10] drm/i915/hdcp: migrate away from kdev_to_i915() in GSC messaging

[Gustavo Sousa]

Quoting Jani Nikula (2024-08-06 11:14:00-03:00)
>On Tue, 06 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>>> Quoting Jani Nikula (2024-07-29 11:30:10-03:00)
>>>>Use to_intel_display() instead of kdev_to_i915() in the HDCP component
>>>>API hooks. Avoid further drive-by changes at this point, and just
>>>>convert the display pointer to i915, and leave the struct intel_display
>>>>conversion for later.
>>>>
>>>>The NULL error checking in the hooks make this a bit cumbersome. I'm not
>>>>actually sure they're really required, but don't go down that rabbit
>>>>hole just now.
>>>>
>>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>---
>>>> .../drm/i915/display/intel_hdcp_gsc_message.c | 67 +++++++++++++------
>>>> 1 file changed, 45 insertions(+), 22 deletions(-)
>>>>
>>>>diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>>>index 6548e71b4c49..35bdb532bbb3 100644
>>>>--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>>>+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc_message.c
>>>>@@ -7,6 +7,7 @@
>>>> #include <drm/intel/i915_hdcp_interface.h>
>>>> 
>>>> #include "i915_drv.h"
>>>>+#include "intel_display_types.h"
>>>> #include "intel_hdcp_gsc_message.h"
>>>> 
>>>> int
>>>>@@ -15,17 +16,19 @@ intel_hdcp_gsc_initiate_session(struct device *dev, struct hdcp_port_data *data,
>>>> {
>>>>         struct wired_cmd_initiate_hdcp2_session_in session_init_in = {};
>>>>         struct wired_cmd_initiate_hdcp2_session_out session_init_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !ake_data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>
>>> Hm... I'm afraid that wouldn't really work if drvdata was NULL, would
>>> it?
>>>
>>> With a NULL drvdata, I believe we would probably get a non-zero offset
>>> here.
>>
>> Right, good catch. Not sure I want to add that check to everything in
>> to_intel_display() macro, because in most cases the passed in value
>> can't be NULL. Driver data is a bit different, though I don't think it
>> should really be NULL either.
>
>Decided to fix this in __drm_device_to_intel_display() like this:
>
> #define __drm_device_to_intel_display(p) \
>-       (&to_i915(p)->display)
>+       ((p) ? &to_i915(p)->display : NULL)
>
>That'll have to change anyway when we can no longer use struct
>drm_i915_private when going from drm_device to intel_display.

Yep, seems like a reasonable approach.

--
Gustavo Sousa

>
>BR,
>Jani.
>
>>
>> BR,
>> Jani.
>>
>>
>>
>>>
>>> --
>>> Gustavo Sousa
>>>
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         session_init_in.header.api_version = HDCP_API_VERSION;
>>>>         session_init_in.header.command_id = WIRED_INITIATE_HDCP2_SESSION;
>>>>@@ -72,17 +75,19 @@ intel_hdcp_gsc_verify_receiver_cert_prepare_km(struct device *dev,
>>>> {
>>>>         struct wired_cmd_verify_receiver_cert_in verify_rxcert_in = {};
>>>>         struct wired_cmd_verify_receiver_cert_out verify_rxcert_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !rx_cert || !km_stored || !ek_pub_km || !msg_sz)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         verify_rxcert_in.header.api_version = HDCP_API_VERSION;
>>>>         verify_rxcert_in.header.command_id = WIRED_VERIFY_RECEIVER_CERT;
>>>>@@ -135,17 +140,19 @@ intel_hdcp_gsc_verify_hprime(struct device *dev, struct hdcp_port_data *data,
>>>> {
>>>>         struct wired_cmd_ake_send_hprime_in send_hprime_in = {};
>>>>         struct wired_cmd_ake_send_hprime_out send_hprime_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !rx_hprime)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         send_hprime_in.header.api_version = HDCP_API_VERSION;
>>>>         send_hprime_in.header.command_id = WIRED_AKE_SEND_HPRIME;
>>>>@@ -183,17 +190,19 @@ intel_hdcp_gsc_store_pairing_info(struct device *dev, struct hdcp_port_data *dat
>>>> {
>>>>         struct wired_cmd_ake_send_pairing_info_in pairing_info_in = {};
>>>>         struct wired_cmd_ake_send_pairing_info_out pairing_info_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !pairing_info)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         pairing_info_in.header.api_version = HDCP_API_VERSION;
>>>>         pairing_info_in.header.command_id = WIRED_AKE_SEND_PAIRING_INFO;
>>>>@@ -234,17 +243,19 @@ intel_hdcp_gsc_initiate_locality_check(struct device *dev,
>>>> {
>>>>         struct wired_cmd_init_locality_check_in lc_init_in = {};
>>>>         struct wired_cmd_init_locality_check_out lc_init_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !lc_init_data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         lc_init_in.header.api_version = HDCP_API_VERSION;
>>>>         lc_init_in.header.command_id = WIRED_INIT_LOCALITY_CHECK;
>>>>@@ -280,17 +291,19 @@ intel_hdcp_gsc_verify_lprime(struct device *dev, struct hdcp_port_data *data,
>>>> {
>>>>         struct wired_cmd_validate_locality_in verify_lprime_in = {};
>>>>         struct wired_cmd_validate_locality_out verify_lprime_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !rx_lprime)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         verify_lprime_in.header.api_version = HDCP_API_VERSION;
>>>>         verify_lprime_in.header.command_id = WIRED_VALIDATE_LOCALITY;
>>>>@@ -330,17 +343,19 @@ int intel_hdcp_gsc_get_session_key(struct device *dev,
>>>> {
>>>>         struct wired_cmd_get_session_key_in get_skey_in = {};
>>>>         struct wired_cmd_get_session_key_out get_skey_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data || !ske_data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         get_skey_in.header.api_version = HDCP_API_VERSION;
>>>>         get_skey_in.header.command_id = WIRED_GET_SESSION_KEY;
>>>>@@ -382,17 +397,19 @@ intel_hdcp_gsc_repeater_check_flow_prepare_ack(struct device *dev,
>>>> {
>>>>         struct wired_cmd_verify_repeater_in verify_repeater_in = {};
>>>>         struct wired_cmd_verify_repeater_out verify_repeater_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !rep_topology || !rep_send_ack || !data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         verify_repeater_in.header.api_version = HDCP_API_VERSION;
>>>>         verify_repeater_in.header.command_id = WIRED_VERIFY_REPEATER;
>>>>@@ -442,6 +459,7 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>>>> {
>>>>         struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
>>>>         struct wired_cmd_repeater_auth_stream_req_out verify_mprime_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>>         size_t cmd_size;
>>>>@@ -449,11 +467,12 @@ int intel_hdcp_gsc_verify_mprime(struct device *dev,
>>>>         if (!dev || !stream_ready || !data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         cmd_size = struct_size(verify_mprime_in, streams, data->k);
>>>>         if (cmd_size == SIZE_MAX)
>>>>@@ -504,17 +523,19 @@ int intel_hdcp_gsc_enable_authentication(struct device *dev,
>>>> {
>>>>         struct wired_cmd_enable_auth_in enable_auth_in = {};
>>>>         struct wired_cmd_enable_auth_out enable_auth_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         enable_auth_in.header.api_version = HDCP_API_VERSION;
>>>>         enable_auth_in.header.command_id = WIRED_ENABLE_AUTH;
>>>>@@ -549,17 +570,19 @@ intel_hdcp_gsc_close_session(struct device *dev, struct hdcp_port_data *data)
>>>> {
>>>>         struct wired_cmd_close_session_in session_close_in = {};
>>>>         struct wired_cmd_close_session_out session_close_out = {};
>>>>+        struct intel_display *display;
>>>>         struct drm_i915_private *i915;
>>>>         ssize_t byte;
>>>> 
>>>>         if (!dev || !data)
>>>>                 return -EINVAL;
>>>> 
>>>>-        i915 = kdev_to_i915(dev);
>>>>-        if (!i915) {
>>>>+        display = to_intel_display(dev);
>>>>+        if (!display) {
>>>>                 dev_err(dev, "DRM not initialized, aborting HDCP.\n");
>>>>                 return -ENODEV;
>>>>         }
>>>>+        i915 = to_i915(display->drm);
>>>> 
>>>>         session_close_in.header.api_version = HDCP_API_VERSION;
>>>>         session_close_in.header.command_id = WIRED_CLOSE_SESSION;
>>>>-- 
>>>>2.39.2
>>>>
>
>-- 
>Jani Nikula, Intel

[PATCH 10/10] drm/xe/display: remove unused compat kdev_to_i915() and pdev_to_i915()

[Jani Nikula]

The display code no longer uses kdev_to_i915() or pdev_to_i915()
helpers. Remove them.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
index 766fba88a3c8..e1d6ce829a0b 100644
--- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
+++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
@@ -21,13 +21,6 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
 	return container_of(dev, struct drm_i915_private, drm);
 }
 
-static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
-{
-	struct drm_device *drm = dev_get_drvdata(kdev);
-
-	return drm ? to_i915(drm) : NULL;
-}
-
 #define IS_PLATFORM(xe, x) ((xe)->info.platform == x)
 #define INTEL_INFO(dev_priv)	(&((dev_priv)->info))
 #define IS_I830(dev_priv)	(dev_priv && 0)
@@ -117,7 +110,6 @@ struct i915_sched_attr {
 };
 #define i915_gem_fence_wait_priority(fence, attr) do { (void) attr; } while (0)
 
-#define pdev_to_i915 pdev_to_xe_device
 #define RUNTIME_INFO(xe)		(&(xe)->info.i915_runtime)
 
 #define FORCEWAKE_ALL XE_FORCEWAKE_ALL
-- 
2.39.2

Re: [PATCH 10/10] drm/xe/display: remove unused compat kdev_to_i915() and pdev_to_i915()

[Gustavo Sousa]

Quoting Jani Nikula (2024-07-29 11:30:11-03:00)
>The display code no longer uses kdev_to_i915() or pdev_to_i915()
>helpers. Remove them.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 8 --------
> 1 file changed, 8 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>index 766fba88a3c8..e1d6ce829a0b 100644
>--- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>+++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>@@ -21,13 +21,6 @@ static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>         return container_of(dev, struct drm_i915_private, drm);
> }
> 
>-static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
>-{
>-        struct drm_device *drm = dev_get_drvdata(kdev);
>-
>-        return drm ? to_i915(drm) : NULL;
>-}
>-
> #define IS_PLATFORM(xe, x) ((xe)->info.platform == x)
> #define INTEL_INFO(dev_priv)        (&((dev_priv)->info))
> #define IS_I830(dev_priv)        (dev_priv && 0)
>@@ -117,7 +110,6 @@ struct i915_sched_attr {
> };
> #define i915_gem_fence_wait_priority(fence, attr) do { (void) attr; } while (0)
> 
>-#define pdev_to_i915 pdev_to_xe_device
> #define RUNTIME_INFO(xe)                (&(xe)->info.i915_runtime)
> 
> #define FORCEWAKE_ALL XE_FORCEWAKE_ALL
>-- 
>2.39.2
>

✗ Fi.CI.SPARSE: warning for drm/xe & drm/i915: drvdata usage changes

[Patchwork]

== Series Details ==

Series: drm/xe & drm/i915: drvdata usage changes
URL   : https://patchwork.freedesktop.org/series/136621/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for drm/xe & drm/i915: drvdata usage changes

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3661 bytes --]

== Series Details ==

Series: drm/xe & drm/i915: drvdata usage changes
URL   : https://patchwork.freedesktop.org/series/136621/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15147 -> Patchwork_136621v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_136621v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_136621v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/index.html

Participating hosts (41 -> 39)
------------------------------

  Additional (2): fi-glk-j4005 bat-kbl-2 
  Missing    (4): fi-kbl-7567u bat-jsl-1 fi-snb-2520m fi-kbl-8809g 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_136621v1:

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-glk-j4005:       NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/fi-glk-j4005/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_136621v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@fbdev@info:
    - bat-kbl-2:          NOTRUN -> [SKIP][2] ([i915#1849])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/bat-kbl-2/igt@fbdev@info.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-kbl-2:          NOTRUN -> [SKIP][3] +39 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/bat-kbl-2/igt@gem_lmem_swapping@parallel-random-engines.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_mocs:
    - bat-arlh-2:         [INCOMPLETE][4] -> [PASS][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15147/bat-arlh-2/igt@i915_selftest@live@gt_mocs.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/bat-arlh-2/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@hangcheck:
    - bat-arls-1:         [DMESG-WARN][6] ([i915#11349] / [i915#11378]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15147/bat-arls-1/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/bat-arls-1/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-2:         [DMESG-FAIL][8] -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15147/bat-arlh-2/igt@i915_selftest@live@workarounds.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/bat-arlh-2/igt@i915_selftest@live@workarounds.html

  
  [i915#11349]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11349
  [i915#11378]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11378
  [i915#1849]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1849


Build changes
-------------

  * Linux: CI_DRM_15147 -> Patchwork_136621v1

  CI-20190529: 20190529
  CI_DRM_15147: ec9c8ca911f54dec90f736f284b7128668deeb0d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7940: 2a73158fa69a2b8e20d5a0bdf773ee194bfe13c2 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_136621v1: ec9c8ca911f54dec90f736f284b7128668deeb0d @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_136621v1/index.html

[-- Attachment #2: Type: text/html, Size: 4430 bytes --]