On Mon, 02 Oct 2023, Rodrigo Vivi <[email protected]> wrote:
> On Mon, Oct 02, 2023 at 10:41:14AM +0300, Jani Nikula wrote:
>> On Fri, 29 Sep 2023, Rodrigo Vivi <[email protected]> wrote:
>> > The goal is to have this function ready for Xe to use
>> > directly. So, let's use the available macro.
>>
>> Seesm wrong to use DISPLAY_INFO() as an lvalue
>
> to be really honestly I don't like that either.
> I barely like macros, specially used like this.
>
>> and I'm not sure why
>> this wouldn't work as-is.
>
> I should probably had collected some logs and added to the
> commit message. But the thing was that without this assignment,
> (xe)->info.display was NULL and the memcpy below was exploding
> with NULL dereference.
Aww crap. That's because both DISPLAY_INFO() and DISPLAY_RUNTIME_INFO()
in xe are completely bogus.
They should be
#define DISPLAY_INFO(i915) ((i915)->display.info.__device_info)
#define DISPLAY_RUNTIME_INFO(i915) (&(i915)->display.info.__runtime_info)
instead of
#define DISPLAY_INFO(xe) ((xe)->info.display)
#define DISPLAY_RUNTIME_INFO(xe) (&(xe)->info.display_runtime)
and these should be removed from struct xe_device info member:
const struct intel_display_device_info *display;
struct intel_display_runtime_info display_runtime;
BR,
Jani.
>
>>
>> But *shrug*.
>>
>> Reviewed-by: Jani Nikula <[email protected]>
>
> thanks, pushed as is.
>
>>
>> for merging to i915. (xe should come as a backport with cherry-pick -x.)
>
> and sent the proper backported cherry-pick to intel-xe ml.
>
>>
>> BR,
>> Jani
>>
>>
>> >
>> > Cc: Jani Nikula <[email protected]>
>> > Signed-off-by: Rodrigo Vivi <[email protected]>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_display_device.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
>> > b/drivers/gpu/drm/i915/display/intel_display_device.c
>> > index a6a18eae7ae8..ce55b968e658 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_device.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
>> > @@ -926,7 +926,7 @@ void intel_display_device_probe(struct
>> > drm_i915_private *i915)
>> > else
>> > info = probe_display(i915);
>> >
>> > - i915->display.info.__device_info = info;
>> > + DISPLAY_INFO(i915) = info;
>> >
>> > memcpy(DISPLAY_RUNTIME_INFO(i915),
>> > &DISPLAY_INFO(i915)->__runtime_defaults,
>>
>> --
>> Jani Nikula, Intel
--
Jani Nikula, Intel
