On 2019-01-10 21:16, Sam Ravnborg wrote: > Hi Peter. > > (Hijacking this thread as I lost the orginal mails)
Assuming you wanted to reply to this patch? https://patchwork.kernel.org/patch/10753571/ >>> I found an unfortunate issue while recoding plane handling to use >>> drm_atomic_helper_check_plane_state(). The driver rotates clockwise, >>> which is not correct. I simply fixed it (patch 1/4), but maybe that >>> will cause regressions for unsuspecting users who simply assumed >>> that the clockwise rotation was correct? I don't know what to do >>> about that? Adding an option to get the old broken behavior seems >>> useless, wouldn't it be just as easy to just fix whatever app to >>> rotate the other way instead of adding an option somewhere? >> >> Hm, rotation support has been added before the standard rotation >> property was created, and at that time I assumed rotation was clockwise >> (which apparently was an unwise choice). Anyway, I don't have a >> solution for this problem, so I'll let Nicolas decide if it's >> acceptable to change the rotation behavior. >> >>> I have only tested this series on sama5d3, but I did check the docs >>> for various other chips (sama5d2, sama5d4, sam9n12, sam9g15, sam9g35 >>> and sam9x35) supported by the driver (relevant to patch 4/4). > > I wonder if, when this code path is anyway touched, could benefit > from drm_rect_rotate(). > > It is obviously not a simple replacement, but could it be used then > I hope the resulting code is simpler. What are you referring to that goes beyond what is done in patch 3/4 in this series? After setting up the strides, the only use of src_[xywh] are to calculate the scaling factors, and for that the position is irrelevant. I.e. src_x and src_y are not used. Sure, in some theoretical sense it might be good if src_[xy] are also transformed into the rotated coordinate system along with src_[wh], but it seems a bit backwards to switch over to struct drm_rect when the interesting properties are the width and height, not the coordinates of the corners. No strong feelings on the issue though... Cheers, Peter
