On Tue, 09 Dec 2025, "Borah, Chaitanya Kumar" <[email protected]> 
wrote:
> On 12/4/2025 8:02 PM, Jani Nikula wrote:
>> intel_display_driver_probe_nogem() is too high of an abstraction level
>> to be looping and initializing individual CRTCs. Move this to
>> intel_crtc.c and repurpose intel_crtc_init() to initialize all
>> CRTCs. Make the original a static __intel_crtc_init() for ininitializing
>> a single CRTC.
>> 
>
> typo: s/ininitializing/initializing
>
> Other than that, LGTM
>
> Reviewed-by: Chaitanya Kumar Borah <[email protected]>

Thanks, fixed the typo while pushing.

BR,
Jani.

>
>> Signed-off-by: Jani Nikula <[email protected]>
>> 
>> ---
>> 
>> This is prep for doing [1] in a nicer way, without divulging the details
>> at the high level.
>> 
>> [1] https://lore.kernel.org/r/[email protected]
>> ---
>>   drivers/gpu/drm/i915/display/intel_crtc.c     | 19 ++++++++++++++++++-
>>   drivers/gpu/drm/i915/display/intel_crtc.h     |  2 +-
>>   .../drm/i915/display/intel_display_driver.c   | 13 +++----------
>>   3 files changed, 22 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c 
>> b/drivers/gpu/drm/i915/display/intel_crtc.c
>> index 5e1e02c8d9d4..778ebc5095c3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
>> @@ -308,7 +308,7 @@ static const struct drm_crtc_funcs i8xx_crtc_funcs = {
>>      .get_vblank_timestamp = intel_crtc_get_vblank_timestamp,
>>   };
>>   
>> -int intel_crtc_init(struct intel_display *display, enum pipe pipe)
>> +static int __intel_crtc_init(struct intel_display *display, enum pipe pipe)
>>   {
>>      struct intel_plane *primary, *cursor;
>>      const struct drm_crtc_funcs *funcs;
>> @@ -406,6 +406,23 @@ int intel_crtc_init(struct intel_display *display, enum 
>> pipe pipe)
>>      return ret;
>>   }
>>   
>> +int intel_crtc_init(struct intel_display *display)
>> +{
>> +    enum pipe pipe;
>> +    int ret;
>> +
>> +    drm_dbg_kms(display->drm, "%d display pipe%s available.\n",
>> +                INTEL_NUM_PIPES(display), 
>> str_plural(INTEL_NUM_PIPES(display)));
>> +
>> +    for_each_pipe(display, pipe) {
>> +            ret = __intel_crtc_init(display, pipe);
>> +            if (ret)
>> +                    return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   int intel_crtc_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void 
>> *data,
>>                                         struct drm_file *file)
>>   {
>> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h 
>> b/drivers/gpu/drm/i915/display/intel_crtc.h
>> index 07917e8a9ae3..12507b51ee77 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crtc.h
>> +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
>> @@ -37,7 +37,7 @@ void intel_crtc_arm_vblank_event(struct intel_crtc_state 
>> *crtc_state);
>>   void intel_crtc_prepare_vblank_event(struct intel_crtc_state *crtc_state,
>>                                   struct drm_pending_vblank_event **event);
>>   u32 intel_crtc_max_vblank_count(const struct intel_crtc_state *crtc_state);
>> -int intel_crtc_init(struct intel_display *display, enum pipe pipe);
>> +int intel_crtc_init(struct intel_display *display);
>>   int intel_crtc_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void 
>> *data,
>>                                         struct drm_file *file_priv);
>>   struct intel_crtc_state *intel_crtc_state_alloc(struct intel_crtc *crtc);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c 
>> b/drivers/gpu/drm/i915/display/intel_display_driver.c
>> index 7e000ba3e08b..e282b533d5b6 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
>> @@ -452,7 +452,6 @@ bool intel_display_driver_check_access(struct 
>> intel_display *display)
>>   /* part #2: call after irq install, but before gem init */
>>   int intel_display_driver_probe_nogem(struct intel_display *display)
>>   {
>> -    enum pipe pipe;
>>      int ret;
>>   
>>      if (!HAS_DISPLAY(display))
>> @@ -466,15 +465,9 @@ int intel_display_driver_probe_nogem(struct 
>> intel_display *display)
>>   
>>      intel_gmbus_setup(display);
>>   
>> -    drm_dbg_kms(display->drm, "%d display pipe%s available.\n",
>> -                INTEL_NUM_PIPES(display),
>> -                INTEL_NUM_PIPES(display) > 1 ? "s" : "");
>> -
>> -    for_each_pipe(display, pipe) {
>> -            ret = intel_crtc_init(display, pipe);
>> -            if (ret)
>> -                    goto err_mode_config;
>> -    }
>> +    ret = intel_crtc_init(display);
>> +    if (ret)
>> +            goto err_mode_config;
>>   
>>      intel_plane_possible_crtcs_init(display);
>>      intel_dpll_init(display);
>

-- 
Jani Nikula, Intel

Reply via email to