[Intel-gfx] [PATCH 5/6] drm/i915: Clean up intel_fbdev_init_bios() a bit
Rodrigo Vivi
rodrigo.vivi at intel.com
Wed Jun 9 11:46:28 UTC 2021
On Wed, Jun 09, 2021 at 11:56:31AM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Sort out the mess with the local variables in
> intel_fbdev_init_bios(). Get rid of all aliasing pointers,
> use standard naming/types, and introduc a few more locals
^ typo
> in the loops to avoid the hard to read long struct walks.
>
> While at we also polish the debugs a bit to use the
> canonical [CRTC:%d:%s] style.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_fbdev.c | 94 +++++++++++++---------
> 1 file changed, 56 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 4af40229f5ec..df05d285f0bd 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -335,32 +335,43 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> * fbcon), so we just find the biggest and use that.
> */
> static bool intel_fbdev_init_bios(struct drm_device *dev,
> - struct intel_fbdev *ifbdev)
> + struct intel_fbdev *ifbdev)
> {
> struct drm_i915_private *i915 = to_i915(dev);
> struct intel_framebuffer *fb = NULL;
> - struct drm_crtc *crtc;
> - struct intel_crtc *intel_crtc;
> + struct intel_crtc *crtc;
> unsigned int max_size = 0;
>
> /* Find the largest fb */
> - for_each_crtc(dev, crtc) {
> + for_each_intel_crtc(dev, crtc) {
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> + struct intel_plane *plane =
> + to_intel_plane(crtc->base.primary);
> + struct intel_plane_state *plane_state =
> + to_intel_plane_state(plane->base.state);
> struct drm_i915_gem_object *obj =
> - intel_fb_obj(crtc->primary->state->fb);
> - intel_crtc = to_intel_crtc(crtc);
> + intel_fb_obj(plane_state->uapi.fb);
oh, here we have again that plane_state uapi change that I don't understand
and I'm not seeing any mention in any commit msg..
>
> - if (!crtc->state->active || !obj) {
> + if (!crtc_state->uapi.active) {
> drm_dbg_kms(&i915->drm,
> - "pipe %c not active or no fb, skipping\n",
> - pipe_name(intel_crtc->pipe));
> + "[CRTC:%d:%s] not active, skipping\n",
> + crtc->base.base.id, crtc->base.name);
> + continue;
> + }
> +
> + if (!obj) {
> + drm_dbg_kms(&i915->drm,
> + "[PLANE:%d:%s] no fb, skipping\n",
> + plane->base.base.id, plane->base.name);
> continue;
> }
>
> if (obj->base.size > max_size) {
> drm_dbg_kms(&i915->drm,
> - "found possible fb from plane %c\n",
> - pipe_name(intel_crtc->pipe));
> - fb = to_intel_framebuffer(crtc->primary->state->fb);
> + "found possible fb from [PLANE:%d:%s]\n",
> + plane->base.base.id, plane->base.name);
> + fb = to_intel_framebuffer(plane_state->uapi.fb);
> max_size = obj->base.size;
> }
> }
> @@ -372,60 +383,62 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> }
>
> /* Now make sure all the pipes will fit into it */
> - for_each_crtc(dev, crtc) {
> + for_each_intel_crtc(dev, crtc) {
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> + struct intel_plane *plane =
> + to_intel_plane(crtc->base.primary);
> unsigned int cur_size;
>
> - intel_crtc = to_intel_crtc(crtc);
> -
> - if (!crtc->state->active) {
> + if (!crtc_state->uapi.active) {
> drm_dbg_kms(&i915->drm,
> - "pipe %c not active, skipping\n",
> - pipe_name(intel_crtc->pipe));
> + "[CRTC:%d:%s] not active, skipping\n",
> + crtc->base.base.id, crtc->base.name);
> continue;
> }
>
> - drm_dbg_kms(&i915->drm, "checking plane %c for BIOS fb\n",
> - pipe_name(intel_crtc->pipe));
> + drm_dbg_kms(&i915->drm, "checking [PLANE:%d:%s] for BIOS fb\n",
> + plane->base.base.id, plane->base.name);
>
> /*
> * See if the plane fb we found above will fit on this
> * pipe. Note we need to use the selected fb's pitch and bpp
> * rather than the current pipe's, since they differ.
> */
> - cur_size = crtc->state->adjusted_mode.crtc_hdisplay;
> + cur_size = crtc_state->uapi.adjusted_mode.crtc_hdisplay;
> cur_size = cur_size * fb->base.format->cpp[0];
> if (fb->base.pitches[0] < cur_size) {
> drm_dbg_kms(&i915->drm,
> - "fb not wide enough for plane %c (%d vs %d)\n",
> - pipe_name(intel_crtc->pipe),
> + "fb not wide enough for [PLANE:%d:%s] (%d vs %d)\n",
> + plane->base.base.id, plane->base.name,
> cur_size, fb->base.pitches[0]);
> fb = NULL;
> break;
> }
>
> - cur_size = crtc->state->adjusted_mode.crtc_vdisplay;
> + cur_size = crtc_state->uapi.adjusted_mode.crtc_vdisplay;
> cur_size = intel_fb_align_height(&fb->base, 0, cur_size);
> cur_size *= fb->base.pitches[0];
> drm_dbg_kms(&i915->drm,
> - "pipe %c area: %dx%d, bpp: %d, size: %d\n",
> - pipe_name(intel_crtc->pipe),
> - crtc->state->adjusted_mode.crtc_hdisplay,
> - crtc->state->adjusted_mode.crtc_vdisplay,
> + "[CRTC:%d:%s] area: %dx%d, bpp: %d, size: %d\n",
> + crtc->base.base.id, crtc->base.name,
> + crtc_state->uapi.adjusted_mode.crtc_hdisplay,
> + crtc_state->uapi.adjusted_mode.crtc_vdisplay,
> fb->base.format->cpp[0] * 8,
> cur_size);
>
> if (cur_size > max_size) {
> drm_dbg_kms(&i915->drm,
> - "fb not big enough for plane %c (%d vs %d)\n",
> - pipe_name(intel_crtc->pipe),
> + "fb not big enough for [PLANE:%d:%s] (%d vs %d)\n",
> + plane->base.base.id, plane->base.name,
> cur_size, max_size);
> fb = NULL;
> break;
> }
>
> drm_dbg_kms(&i915->drm,
> - "fb big enough for plane %c (%d >= %d)\n",
> - pipe_name(intel_crtc->pipe),
> + "fb big enough [PLANE:%d:%s] (%d >= %d)\n",
> + plane->base.base.id, plane->base.name,
> max_size, cur_size);
> }
>
> @@ -441,15 +454,20 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> drm_framebuffer_get(&ifbdev->fb->base);
>
> /* Final pass to check if any active pipes don't have fbs */
> - for_each_crtc(dev, crtc) {
> - intel_crtc = to_intel_crtc(crtc);
> + for_each_intel_crtc(dev, crtc) {
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> + struct intel_plane *plane =
> + to_intel_plane(crtc->base.primary);
> + struct intel_plane_state *plane_state =
> + to_intel_plane_state(plane->base.state);
>
> - if (!crtc->state->active)
> + if (!crtc_state->uapi.active)
> continue;
>
> - drm_WARN(dev, !crtc->primary->state->fb,
> - "re-used BIOS config but lost an fb on crtc %d\n",
> - crtc->base.id);
> + drm_WARN(dev, !plane_state->uapi.fb,
> + "re-used BIOS config but lost an fb on [PLANE:%d:%s]\n",
> + plane->base.base.id, plane->base.name);
> }
>
>
> --
> 2.31.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list